prompt(skill+memory): enforce sequential destructive pair + final-text honesty - #12
Merged
Merged
Conversation
…t honesty PR-A (#11) physically blocked the silent data loss with a same-name destructive guard; this PR closes the prompt-side root causes from the 2026-05-26 dogfood so the model stops emitting the destructive pair in the first place. skillConsolidator workflow (`src/agents/bundled/skillConsolidator.ts`): - Step 2: "A single skill is never a merge candidate" — explicit guard against today's degenerate case (model treated size=1 group as X-merges-into-X and SkillDelete'd the same name). - Step 3 split: 3.a SkillWrite is a standalone tool_use; wait for `is_error:false`. 3.b deletes only run after that confirmation, and **never** delete the surviving name. 3.c on `is_error:true`, abort the group; do not retry blindly. Bundling 3.a and 3.b in the same assistant turn is forbidden. - Output discipline: final text must enumerate SkillWrite and SkillDelete success counts plus any aborted groups. Forbids "nothing to merge" if any destructive tool fired — that exact phrase is what the model used in the dogfood while it had just deleted the user's only skill. memoryCurator (`src/agents/bundled/memoryCurator.ts`): mirrors the same sequencing for MemoryWriteAt + MemoryDelete (both in cross-role promotion and within-directory cleanup), plus the same final-text honesty rule. Snapshot test hashes (`role-prompt.snapshot.test.ts` + `prompt/prompt-for-role.test.ts`) updated for memoryCurator (22c073df → c37344ca) and skillConsolidator (3e45d127 → c2700894); deliberate semantic change, not a structural refactor — see [feedback_prompt_changes]. New semantic contract test (`curator-prompt-contract.test.ts`, 13 cases) anchors the key sequencing phrases so a future rewrite that forgets the ordering rules fails loudly. These are intent guards on top of the snapshot's byte-level pin. Test plan: - pnpm test: 1724/1724 PASS (1711 from main + 13 new contract cases) - pnpm typecheck clean - pnpm build clean - Re-confirmed the cleanup-line regex matches by running just that one test before adding the rest Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the bundled skillConsolidator and memoryCurator role prompts to prevent destructive “write+delete” pairs from executing out of order (or after a failed write), and tightens final-text reporting so the agent acknowledges destructive operations.
Changes:
- Update
skillConsolidatorprompt to require sequentialSkillWrite→ (only on success)SkillDelete, forbid same-name delete, and improve “nothing to merge” honesty rules. - Update
memoryCuratorprompt to require sequentialMemoryWriteAt→ (only on success)MemoryDelete, and add honest final-text reporting rules. - Update snapshot hashes and add a new contract test suite to pin key sequencing/intent phrases beyond byte-level snapshots.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/agents/bundled/skillConsolidator.ts | Adds explicit sequencing + safety rules for SkillWrite/SkillDelete and more honest final output requirements. |
| src/agents/bundled/memoryCurator.ts | Adds explicit sequencing rules for MemoryWriteAt/MemoryDelete and honest final output requirements. |
| src/agents/bundled/curator-prompt-contract.test.ts | New intent/contract tests to ensure critical sequencing phrases remain present in prompts. |
| src/agents/role-prompt.snapshot.test.ts | Updates expected snapshot hash for prompt semantic changes. |
| src/prompt/prompt-for-role.test.ts | Updates expected snapshot hash for prompt semantic changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+58
to
+59
| ' a. Call `SkillWrite(name:<surviving>, overwrite:true, ...)` with the', | ||
| " merged markdown. Frontmatter `roles` is the union of the group's", |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the prompt-side root causes (#2 + #6) from the 2026-05-26 dogfood Bug 1 — companion to #11 which physically blocked the silent data loss with a same-name destructive guard.
The dogfood signal:
skillConsolidatoremittedSkillWrite(name:X, overwrite:true) + SkillDelete(name:X)in one batch, SkillWrite failed validation, SkillDelete ran anyway, user's prior on-disk skill silently gone. Model final text:"nothing to merge this pass"— it didn't even notice it had destroyed user data.Prompt changes
skillConsolidator.tsStep 2 — explicit "single skill is never a merge candidate" guard. Today's degenerate case was a size-1 group treated as
X merges into X.Step 3 split into 3.a / 3.b / 3.c with hard sequencing:
3.aSkillWrite is a standalone tool_use; wait foris_error:false3.bdeletes only run after that confirmation, and never delete the surviving name3.conis_error:true, abort the group; no blind retryOutput discipline — final text must enumerate SkillWrite + SkillDelete success counts plus any aborted groups. Explicit
Never say "nothing to merge" if you called any SkillWrite or SkillDelete— that exact phrase is what the model used while it had just deleted the user's only skill.memoryCurator.tsSame sequencing applied to the
MemoryWriteAt + MemoryDeletepair:is_error:false, then MemoryDelete the originals. If the MemoryWriteAt returns is_error:true, abort the promotion — do not delete any originals."Test plan
role-prompt.snapshot.test.ts+prompt/prompt-for-role.test.ts) updated for the 2 roles — deliberate semantic change, not a structural refactor (per [feedback_prompt_changes]).curator-prompt-contract.test.ts(13 cases) anchors the key sequencing phrases as intent guards on top of the snapshot's byte pin. If a future rewrite forgets the sequence-after-success rule, contract tests fail loudly with which phrase is missing.pnpm test1724/1724 PASS (1711 baseline + 13 new contract cases)pnpm typecheckcleanpnpm buildcleanRelationship to #11
(userId, name)TTL refuses same-name delete after recent write failure; flow-style YAML now parses; actionable error msgs; stderrAfter both ship, the dogfood failure cannot recur even if any one layer regresses.
🤖 Generated with Claude Code