Skip to content

feat: keep unused tools and skills, and detach any resource kind - #946

Open
ctawiah wants to merge 2 commits into
ctawiah/plan-referenced-attachments-1e8afrom
ctawiah/keep-unused-attachments-1e8a
Open

ctawiah wants to merge 2 commits into
ctawiah/plan-referenced-attachments-1e8afrom
ctawiah/keep-unused-attachments-1e8a

Conversation

@ctawiah

@ctawiah ctawiah commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Context

ldcli sync prompts keeps the files of prompt variations in step with LaunchDarkly. A variation can use tools and skills, and their files live next to the variation files. Phase 1 treats a tool or skill file as a part of the variations that use it:

  • After a sync, sync prompts lists each tool and skill file that no variation uses. It deletes the file with --yes, or after the user answers yes.
  • After every apply, sync removes the sync.lock and manifest rows of unused tools and skills.
  • sync prompts detach drops the rows of the tools and skills that only the detached variations used.
  • detach asks for confirmation only with --archive.

The rest of this stack syncs a tool or skill on its own, also when no variation uses it. #941 already keeps the rows of unused tools and skills. So the deletion above now removes a file that sync still tracks, and the next sync of tools restores it.

What changes

  • Sync never deletes a tool or skill file, or its rows, because no variation uses it. cleanupOrphanedAttachments, OrphanedAttachments, and RemoveUnusedAttachments are removed.
  • detach works for every resource kind. sync prompts detach still lists only variations. Later PRs in this stack add sync tools detach and sync skills detach.
  • Detach refuses a tool or skill that a local variation uses: tool demo/search is used by variation demo/support/default. Remove it from that variation first. If the same detach also removes that variation, the tool is free.
  • sync prompts detach keeps the tools and skills of the variation, with their files and rows. They keep syncing on their own.
  • Every detach lists what it removes and asks once. It marks a resource whose file differs from its sync.lock row. --yes skips the question, and also covers --archive. Without a terminal and without --yes, detach fails with the confirmation error.
  • Detach deletes the variation, tool, and skill files in one transaction. If one delete fails, every file stays, and the baseline comes back.
  • --archive archives only variations. LaunchDarkly keeps a detached tool or skill.
  • attach of a tool that already has a file keeps the local content, as it already does for a tool that another variation uses.

A detach in a terminal after a local edit:

Resources to detach:
  variation demo/support/default (local changes will be lost)

Detach these resources? [y/N]

Design notes

  • detach.Options.Kind is the kind that the picker lists. loadResources returns every kind, so a selection by flags works for any kind.
  • The used-by check compiles the local variations. A compile error does not block a variation detach, because Phase 1 detach works in a broken workspace. It does block a tool or skill detach, because the check cannot run.
  • The mark compares the raw local fingerprint with the lock row. A file that leaves out its model config version can show a false mark. A model config read in detach is not worth the extra request.
  • attach now reads a tool file through CompileAttachments. So one read covers a tool that a variation uses and a tool that no variation uses.
  • Store.DeleteResources deletes the files of every kind in one transaction, and replaces DeleteAttachments. The delete row of a sync uses it too.
  • This changes Phase 1 scripts: sync prompts detach <selector> --no-input must now add --yes.
  • This PR must ship in the same ldcli release as feat: apply tool and skill rows in a sync #941. feat: apply tool and skill rows in a sync #941 keeps the rows of unused tools and skills, but sync prompts still offers to delete their files. This PR removes that offer.

Review focus

  • Is there a path left that deletes a tool or skill file only because no variation uses it?
  • Is the used-by refusal right when the same detach also removes the variation?
  • Is the new confirmation of sync prompts detach acceptable for Phase 1 scripts?

Verification

  • go test ./... and golangci-lint run (v2.9.0) pass.
  • TestPromptDeletesUnreferencedLocalToolWithYes becomes TestPromptKeepsUnreferencedLocalToolWithYes. The file, the lock row, and the manifest row stay.
  • TestPromptDetachKeepsToolsOfTheVariation checks that sync prompts detach --yes keeps the tool file and its rows.
  • The detach tests cover tool and skill detach, and the used-by refusal and its exception. They also cover one question with and without --archive, the mark, and the error without a terminal.
  • TestRunKeepsEveryFileWhenOneDeleteFails uses a tool file that cannot be deleted, and checks that the variation file and the baseline stay. It fails without c6078016.
  • TestRunArchiveWithAToolArchivesOnlyTheVariation archives the variation, detaches both resources, and makes no archive request for the tool. It fails without c6078016.
  • I built ldcli and ran these flows against a stand-in for the LaunchDarkly API:
    • sync prompts --yes keeps a standalone tool file.
    • Detach without --yes and without a terminal fails.
    • The question shows the mark.
    • detach --yes --no-input keeps the tool files.

Related changes

This stack builds on #937 to #941, which are merged.

  1. fix: exclude expanded tools from prompt diffs #957: Exclude expanded tools from prompt diffs (ctawiah/fix-prompt-tool-diff-5b1c)
  2. fix: plan a tool or skill row when a variation newly references it #945: Plan a tool or skill row when a variation newly references it (ctawiah/plan-referenced-attachments-1e8a)
  3. This PR: keep unused tools and skills, and detach any resource kind (ctawiah/keep-unused-attachments-1e8a)
  4. feat: add tools and skills from LaunchDarkly #947: Add tools and skills from LaunchDarkly (ctawiah/add-attachments-1e8a)
  5. feat: route sync, add, and detach by scope #948: Route sync, add, and detach by scope (ctawiah/route-scopes-1e8a)
  6. feat: add the sync all, sync tools, and sync skills commands #949: Add the sync all, sync tools, and sync skills commands (ctawiah/scope-commands-1e8a)
  7. feat: create a tool or a skill from the content of an add #950: Create a tool or a skill from the content of an add (ctawiah/create-attachments-1e8a)
  8. feat: add the create flags of sync tools add and sync skills add #951: Add the create flags of sync tools add and sync skills add (ctawiah/create-attachment-flags-1e8a)
  9. refactor: share the selection of a new variation, and check any kind before an add #952: Share the selection of a new variation, and check any kind before an add (ctawiah/share-variation-selection-1e8a)
  10. feat: create a prompt variation from the content of an add #953: Create a prompt variation from the content of an add (ctawiah/create-variations-1e8a)
  11. feat: add the create flags of sync prompts add #954: Add the create flags of sync prompts add (ctawiah/create-variation-flags-1e8a)

Note

Overview
Tools and skills are first-class sync resources: sync no longer offers to delete unreferenced tool/skill files or prune their sync.lock rows, and detach can stop syncing variations, tools, or skills (prompt detach still targets variations only via Kind).

Detach now lists selected resources, warns when local files differ from the lock, and asks once before deleting (with optional --archive for variations only). --yes is required for non-interactive runs, including scripts that used detach … --no-input without --yes. Detaching a variation leaves its tools/skills on disk and in the lock; detaching a tool/skill is blocked while a local variation still references it unless that variation is detached in the same command. File removal uses transactional DeleteResources, and manifest updates only remove explicitly selected IDs.

Attach preserves local edits for tools that already have a standalone file, not only when another variation references them.

Reviewed by Cursor Bugbot for commit c607801. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 9b6a6ed. Configure here.

Comment thread internal/sync/detach/detach.go
Comment thread internal/sync/detach/detach.go
@cursor
cursor Bot force-pushed the ctawiah/keep-unused-attachments-1e8a branch 2 times, most recently from c607801 to f89bdfc Compare October 23, 2026 14:40
Sync no longer deletes tool and skill files that no variation uses, and
it keeps their sync.lock and remote manifest entries. A detach of a
variation also keeps its tools and skills. They sync on their own.

Detach now takes resources of every kind. It refuses a tool or a skill
that a local variation still uses. It lists every resource, marks the
resources with local changes, and asks once. --yes skips the question,
also with --archive. Without a terminal, detach needs --yes.

Attach keeps the local content of a tool or skill file that no variation
uses yet.
A detach with --archive tried to archive each selected resource as a
variation. A tool or skill in the selection failed after the question,
and variations before it were already archived while the local detach
never ran. The archive now skips tools and skills, because LaunchDarkly
keeps them.

Detach deleted the variation files and the tool and skill files in two
transactions. When the second one failed, the variation files were gone
while the baseline came back, so local edits were lost. Store now has
DeleteResources, which deletes the files of every kind in one
transaction, and it replaces DeleteAttachments.
@cursor
cursor Bot force-pushed the ctawiah/keep-unused-attachments-1e8a branch from f89bdfc to bd1b7d8 Compare October 23, 2026 14:44

// Detach deletes the variation, tool, and skill files in one transaction. When
// one file cannot be deleted, every file stays, and so does the baseline.
func TestRunKeepsEveryFileWhenOneDeleteFails(t *testing.T) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the all or nothing behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants