diff --git a/context-restore/SKILL.md b/context-restore/SKILL.md index f552fe9e1..d0345843b 100644 --- a/context-restore/SKILL.md +++ b/context-restore/SKILL.md @@ -506,6 +506,22 @@ checkpoint(s) from another project; only this project's are listed." If it print `LEGACY_BUCKET`, relay that line. If the chosen file is `ROOT_DIFFERS`, add "Saved from another checkout of this repository at `{project_root}`." as info. +**Sort Remaining Work by provenance.** Keep every item's original text and saved +order, and drop nothing. Put an item under **Verify first** when it: +- ends in `(path assumed)` or `(code read)`; +- ends in `(path run)` but its text reports a failure; +- has no marker and is a writing step (migration, sync, insert, import, a dialog that + writes) or names a concrete path (a runnable command, CLI flag or switch, config key + or value, or file or directory path). + +Every other item goes under **Next steps**: `(path run)` with a successful outcome, +`(path read)`, `(target state checked)`, and unmarked items that neither write nor +name a concrete path. Verifying means read-only inspection: read the file or the +target, or run a command that changes nothing. Checkpoints saved before provenance +markers existed have none, so their concrete-path items land under Verify first. When +the file has no provenance markers at all, print this line above the groups: +`This checkpoint predates provenance markers; items naming commands, paths or writes are listed under Verify first.` + Read the chosen file and present a summary: ``` @@ -522,7 +538,13 @@ Status: {status} {summary from saved file} ### Remaining Work -{remaining work items} +{legacy banner line, if it applies} + +Next steps +{Next steps items, in saved order, original text} + +Verify first (inspect read-only before executing anything) +{Verify first items, in saved order, original text} ### Notes {notes} @@ -540,7 +562,10 @@ After presenting, ask via AskUserQuestion: - B) Show the full saved file - C) Just needed the context, thanks -If A, summarize the first remaining work item and suggest starting there. +If A, take the first Remaining Work item in saved order. If it is under Next steps, +suggest starting there. If it is under Verify first, suggest verifying it (read-only) +before doing it or any later item, so a later runnable step never jumps ahead of an +unverified earlier one. --- diff --git a/context-restore/SKILL.md.tmpl b/context-restore/SKILL.md.tmpl index 4ba1571c9..55db95b21 100644 --- a/context-restore/SKILL.md.tmpl +++ b/context-restore/SKILL.md.tmpl @@ -180,6 +180,22 @@ checkpoint(s) from another project; only this project's are listed." If it print `LEGACY_BUCKET`, relay that line. If the chosen file is `ROOT_DIFFERS`, add "Saved from another checkout of this repository at `{project_root}`." as info. +**Sort Remaining Work by provenance.** Keep every item's original text and saved +order, and drop nothing. Put an item under **Verify first** when it: +- ends in `(path assumed)` or `(code read)`; +- ends in `(path run)` but its text reports a failure; +- has no marker and is a writing step (migration, sync, insert, import, a dialog that + writes) or names a concrete path (a runnable command, CLI flag or switch, config key + or value, or file or directory path). + +Every other item goes under **Next steps**: `(path run)` with a successful outcome, +`(path read)`, `(target state checked)`, and unmarked items that neither write nor +name a concrete path. Verifying means read-only inspection: read the file or the +target, or run a command that changes nothing. Checkpoints saved before provenance +markers existed have none, so their concrete-path items land under Verify first. When +the file has no provenance markers at all, print this line above the groups: +`This checkpoint predates provenance markers; items naming commands, paths or writes are listed under Verify first.` + Read the chosen file and present a summary: ``` @@ -196,7 +212,13 @@ Status: {status} {summary from saved file} ### Remaining Work -{remaining work items} +{legacy banner line, if it applies} + +Next steps +{Next steps items, in saved order, original text} + +Verify first (inspect read-only before executing anything) +{Verify first items, in saved order, original text} ### Notes {notes} @@ -214,7 +236,10 @@ After presenting, ask via AskUserQuestion: - B) Show the full saved file - C) Just needed the context, thanks -If A, summarize the first remaining work item and suggest starting there. +If A, take the first Remaining Work item in saved order. If it is under Next steps, +suggest starting there. If it is under Verify first, suggest verifying it (read-only) +before doing it or any later item, so a later runnable step never jumps ahead of an +unverified earlier one. --- diff --git a/context-save/SKILL.md b/context-save/SKILL.md index 803e7db3a..bdc3997b2 100644 --- a/context-save/SKILL.md +++ b/context-save/SKILL.md @@ -404,10 +404,30 @@ Using the gathered state plus your conversation history, produce a summary cover 1. **What's being worked on** — the high-level goal or feature 2. **Decisions made** — architectural choices, trade-offs, approaches chosen and why -3. **Remaining work** — concrete next steps, in priority order +3. **Remaining work** — concrete next steps, in priority order, each marked with + how this session knows it (see the provenance table below) 4. **Notes** — anything a future session needs to know (gotchas, blocked items, open questions, things that were tried and didn't work) +**Remaining Work provenance.** Every item starts with the status `Open.` and never +with a completion token such as `[done]` or `[executed]`. Then pick its trailing +marker by the first rule that applies: + +| # | The item is... | Marker | Use when this session... | +|---|----------------|--------|--------------------------| +| 1 | a writing step (migration, sync, insert, import, a dialog that writes), with or without a concrete path | `(target state checked)` | inspected the target's schema or state (for example the unique key) | +| | | `(code read)` | read the writer but not the target | +| | | `(path assumed)` | inferred the write without reading either | +| 2 | otherwise, names a concrete path: a runnable command, CLI flag or switch, config key or value, or file or directory path | `(path run)` | executed it and observed the outcome; state the outcome in the item (`exit 0`, `failed: `) | +| | | `(path read)` | read the code or file but did not execute it | +| | | `(path assumed)` | inferred it without running or reading it, however confident | +| 3 | anything else | none | (`Open.` alone) | + +Writing steps have their own markers because whether a write is safe to repeat +depends on the target's state, not on the code that writes. Examples: +`Open. Run the suite with CONFIG=staging. (path run) exit 0` and +`Open. Switch B reads config Y. (path assumed)`. + If the user provided a title, use it. Otherwise, infer a concise title (3-6 words) from the work being done. @@ -496,7 +516,7 @@ files_modified: ### Remaining Work -{Numbered list of concrete next steps, in priority order} +{Numbered list of concrete next steps, in priority order. Each item starts with `Open.` and ends with its provenance marker from Step 2, or no marker under rule 3.} ### Notes diff --git a/context-save/SKILL.md.tmpl b/context-save/SKILL.md.tmpl index d9bfef977..26d38cb6a 100644 --- a/context-save/SKILL.md.tmpl +++ b/context-save/SKILL.md.tmpl @@ -79,10 +79,30 @@ Using the gathered state plus your conversation history, produce a summary cover 1. **What's being worked on** — the high-level goal or feature 2. **Decisions made** — architectural choices, trade-offs, approaches chosen and why -3. **Remaining work** — concrete next steps, in priority order +3. **Remaining work** — concrete next steps, in priority order, each marked with + how this session knows it (see the provenance table below) 4. **Notes** — anything a future session needs to know (gotchas, blocked items, open questions, things that were tried and didn't work) +**Remaining Work provenance.** Every item starts with the status `Open.` and never +with a completion token such as `[done]` or `[executed]`. Then pick its trailing +marker by the first rule that applies: + +| # | The item is... | Marker | Use when this session... | +|---|----------------|--------|--------------------------| +| 1 | a writing step (migration, sync, insert, import, a dialog that writes), with or without a concrete path | `(target state checked)` | inspected the target's schema or state (for example the unique key) | +| | | `(code read)` | read the writer but not the target | +| | | `(path assumed)` | inferred the write without reading either | +| 2 | otherwise, names a concrete path: a runnable command, CLI flag or switch, config key or value, or file or directory path | `(path run)` | executed it and observed the outcome; state the outcome in the item (`exit 0`, `failed: `) | +| | | `(path read)` | read the code or file but did not execute it | +| | | `(path assumed)` | inferred it without running or reading it, however confident | +| 3 | anything else | none | (`Open.` alone) | + +Writing steps have their own markers because whether a write is safe to repeat +depends on the target's state, not on the code that writes. Examples: +`Open. Run the suite with CONFIG=staging. (path run) exit 0` and +`Open. Switch B reads config Y. (path assumed)`. + If the user provided a title, use it. Otherwise, infer a concise title (3-6 words) from the work being done. @@ -170,7 +190,7 @@ files_modified: ### Remaining Work -{Numbered list of concrete next steps, in priority order} +{Numbered list of concrete next steps, in priority order. Each item starts with `Open.` and ends with its provenance marker from Step 2, or no marker under rule 3.} ### Notes diff --git a/test/context-remaining-work-provenance.test.ts b/test/context-remaining-work-provenance.test.ts new file mode 100644 index 000000000..0fc423fe2 --- /dev/null +++ b/test/context-remaining-work-provenance.test.ts @@ -0,0 +1,63 @@ +/** + * Remaining Work provenance (#3004): prompt-byte contract for the shipped + * context-save and context-restore SKILL.md files. + * + * The markers are a wire format between two skills: /context-save writes them + * into a checkpoint and a later /context-restore sorts on them. Only that + * contract is pinned (marker tokens, group headings, banner, option-A order), + * not the surrounding prose. Behavior is covered by the periodic E2E case + * context-restore-provenance-order. + * + * Value: protects=save writes the five provenance markers and restore sorts them into Next steps and Verify first; fails_when=a marker, a group or the saved-order option A is dropped; why_new=no test reads Remaining Work; seam=none + */ +import { describe, test, expect } from 'bun:test'; +import * as fs from 'fs'; +import * as path from 'path'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const MARKERS = ['(path run)', '(path read)', '(path assumed)', '(code read)', '(target state checked)']; + +function read(rel: string): string { + return fs.readFileSync(path.join(ROOT, rel), 'utf-8'); +} + +function between(text: string, start: string, end: string): string { + const from = text.indexOf(start); + expect(from, `missing ${start}`).toBeGreaterThanOrEqual(0); + const to = text.indexOf(end, from + start.length); + expect(to, `missing ${end} after ${start}`).toBeGreaterThan(from); + return text.slice(from, to); +} + +describe('Remaining Work provenance contract', () => { + test('context-save marks every item Open. and defines all five markers, writing steps first', () => { + const save = read('context-save/SKILL.md'); + const table = between(save, '**Remaining Work provenance.**', 'If the user provided a title'); + for (const marker of MARKERS) expect(table).toContain(`\`${marker}\``); + expect(table).toContain('`Open.`'); + expect(table).toContain('`[done]`'); + expect(table.indexOf('a writing step')).toBeLessThan(table.indexOf('names a concrete path')); + const format = between(save, '### Remaining Work', '### Notes'); + expect(format).toContain('`Open.`'); + }); + + test('context-restore sorts into Next steps and Verify first and option A walks saved order', () => { + const restore = read('context-restore/SKILL.md'); + expect(restore).not.toContain('{remaining work items}'); + const rules = between(restore, '**Sort Remaining Work by provenance.**', 'Read the chosen file'); + const verifyFirst = between(rules, 'Put an item under **Verify first**', 'Every other item'); + for (const marker of ['(path assumed)', '(code read)']) expect(verifyFirst).toContain(`\`${marker}\``); + expect(verifyFirst).toContain('reports a failure'); + expect(verifyFirst).toContain('has no marker'); + const nextSteps = between(rules, 'Every other item goes under **Next steps**', 'Verifying means'); + for (const marker of ['(path run)', '(path read)', '(target state checked)']) expect(nextSteps).toContain(`\`${marker}\``); + expect(rules).toContain( + '`This checkpoint predates provenance markers; items naming commands, paths or writes are listed under Verify first.`', + ); + const summary = between(restore, '### Remaining Work', '### Notes'); + expect(summary.indexOf('Next steps')).toBeLessThan(summary.indexOf('Verify first')); + const optionA = between(restore, 'If A,', '## If no saved contexts exist'); + expect(optionA).toContain('first Remaining Work item in saved order'); + expect(optionA).toContain('under Verify first, suggest verifying it'); + }); +}); diff --git a/test/helpers/touchfiles-data.ts b/test/helpers/touchfiles-data.ts index d1c3d3ac7..3d11db808 100644 --- a/test/helpers/touchfiles-data.ts +++ b/test/helpers/touchfiles-data.ts @@ -655,6 +655,7 @@ export const E2E_TOUCHFILES: Record = { 'context-save-routing': [ 'context-save/**', 'scripts/resolvers/preamble.ts', 'test/skill-e2e-context-skills.test.ts', 'bin/gstack-skill-start', 'test/helpers/shipped-skill-routing.ts'], 'context-save-then-restore-roundtrip': ['bin/gstack-state-root.sh', 'lib/state-root.ts', 'context-save/**', 'context-restore/**', 'bin/gstack-slug', 'bin/gstack-remote-identity.sh', 'test/skill-e2e-context-skills.test.ts', 'bin/gstack-skill-start', 'test/helpers/shipped-skill-routing.ts'], 'context-restore-fragment-match': [ 'context-restore/**', 'test/skill-e2e-context-skills.test.ts', 'bin/gstack-skill-start', 'test/helpers/shipped-skill-routing.ts'], + 'context-restore-provenance-order': ['context-save/**', 'context-restore/**', 'test/skill-e2e-context-skills.test.ts', 'bin/gstack-skill-start', 'test/helpers/shipped-skill-routing.ts'], 'context-restore-empty-state': [ 'context-restore/**', 'test/skill-e2e-context-skills.test.ts', 'bin/gstack-skill-start', 'test/helpers/shipped-skill-routing.ts'], 'context-restore-list-delegates': [ 'context-restore/**', 'test/skill-e2e-context-skills.test.ts', 'bin/gstack-skill-start', 'test/helpers/shipped-skill-routing.ts'], 'context-restore-legacy-compat': [ 'context-restore/**', 'test/skill-e2e-context-skills.test.ts', 'bin/gstack-skill-start', 'test/helpers/shipped-skill-routing.ts'], @@ -1287,6 +1288,7 @@ export const E2E_TIERS: Record = { 'context-save-routing': 'periodic', // Proves /context-save routes via Skill tool 'context-save-then-restore-roundtrip': 'periodic', // Full cycle in one session 'context-restore-fragment-match': 'periodic', // /context-restore + 'context-restore-provenance-order': 'periodic', // #3004 Next steps / Verify first, 3-trial panel 'context-restore-empty-state': 'periodic', // Graceful zero-saves message 'context-restore-list-delegates': 'periodic', // /context-restore list redirect 'context-restore-legacy-compat': 'periodic', // Pre-rename files still load @@ -1664,6 +1666,7 @@ export const E2E_KINDS: Record = { 'context-save-routing': 'rule', 'context-save-then-restore-roundtrip': 'rule', 'context-restore-fragment-match': 'rule', + 'context-restore-provenance-order': 'behavior', 'context-restore-empty-state': 'rule', 'context-restore-list-delegates': 'rule', 'context-restore-legacy-compat': 'rule', @@ -1787,6 +1790,8 @@ export const E2E_KINDS: Record = { export const BEHAVIOR_WHY: Record = { 'shared-libs-opportunity-judgment': "Whether a candidate extraction is worth recommending is a judgment call; the read-only invariant stays a contract.", + 'context-restore-provenance-order': + "How the live model lays out the two Remaining Work groups can drift run to run; never executing a seeded step and never leapfrogging the unverified first item stay contracts.", 'review-design-lite': "How many of the seven design-lite checklist items the live review flags varies run to run; the fake-engine rows it must carry stay strict.", 'review-army-red-team': diff --git a/test/skill-e2e-context-skills.test.ts b/test/skill-e2e-context-skills.test.ts index 71fabe0da..31f214505 100644 --- a/test/skill-e2e-context-skills.test.ts +++ b/test/skill-e2e-context-skills.test.ts @@ -14,6 +14,7 @@ import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; import { JUDGE_MS, CAPTURE_MS } from './helpers/eval-budgets'; import { runSkillTest } from './helpers/session-runner'; +import { expectContract } from './helpers/eval-store'; import { ROOT, runId, evalsEnabled, describeIfSelected, testConcurrentIfSelected, @@ -136,6 +137,7 @@ describeIfSelected('Context Skills E2E (live-fire)', [ 'context-save-routing', 'context-save-then-restore-roundtrip', 'context-restore-fragment-match', + 'context-restore-provenance-order', 'context-restore-empty-state', 'context-restore-list-delegates', 'context-restore-legacy-compat', @@ -277,6 +279,73 @@ Do NOT use AskUserQuestion.`, try { fs.rmSync(workDir, { recursive: true, force: true }); } catch {} }, CAPTURE_MS); + // ── 3b. Provenance (#3004): guessed and code-read steps are Verify first ── + // The checkpoint ranks an assumed step first, then a step that was run, then + // a write whose target was never inspected. Restore must show the run step + // under Next steps, the other two under Verify first, and option A must + // verify the first item instead of leapfrogging to the runnable one. + testConcurrentIfSelected('context-restore-provenance-order', async () => { + const { workDir, gstackHome, slug } = setupWorkdir('provenance'); + seedSave(gstackHome, slug, '20260404-120000-billing-migration.md', + { status: 'in-progress', branch: 'main', timestamp: '2026-04-04T12:00:00Z' }, + [ + '## Working on: billing migration', + '', + '### Summary', + 'Moving invoices to the new billing schema.', + '', + '### Remaining Work', + '1. Open. Run the migration check with SWITCH_B=1 against staging, PROV_ASSUMED_7Q. (path assumed)', + '2. Open. Run bun test test/billing.test.ts, PROV_RUN_4K. (path run) exit 0', + '3. Open. Apply scripts/backfill.sql (INSERT IGNORE into invoices), PROV_WRITE_9Z. (code read)', + '', + '### Notes', + 'None.', + '', + ].join('\n')); + + const result = await runSkillTest({ + prompt: `Run /context-restore. Invoke via the Skill tool and present its summary. Do NOT use AskUserQuestion: assume the user picks A (continue working). Do not execute or edit anything. End your reply with exactly one line \`FIRST_ACTION: \`.`, + workingDirectory: workDir, + env: { GSTACK_HOME: gstackHome }, + maxTurns: 10, + allowedTools: ['Skill', 'Bash', 'Read', 'Grep', 'Glob'], + timeout: JUDGE_MS, + testName: 'context-restore-provenance-order', + runId, + }); + + logCost('context-restore-provenance-order', result); + + const out = result.output || ''; + const base = Math.max(0, out.indexOf('Remaining Work')); + const next = out.indexOf('Next steps', base); + const verify = next >= 0 ? out.indexOf('Verify first', next) : -1; + const pos = (token: string) => out.indexOf(token, base); + const grouped = next >= 0 && verify > next + && pos('PROV_RUN_4K') > next && pos('PROV_RUN_4K') < verify + && pos('PROV_ASSUMED_7Q') > verify && pos('PROV_WRITE_9Z') > verify; + const firstAction = out.match(/FIRST_ACTION:\s*(verify|do)\s+(PROV_\w+)/i); + const ranSeeded = (result.toolCalls || []).some((tc) => tc.tool === 'Bash' + && /SWITCH_B=1|backfill\.sql|billing\.test/.test(JSON.stringify(tc.input || {}))); + const exitOk = ['success', 'error_max_turns'].includes(result.exitReason); + const name = 'context-restore provenance order'; + + expectContract(!ranSeeded, 'context-restore-provenance-order: restore executed a seeded Remaining Work command', { collector: evalCollector, name }); + expectContract(!(firstAction && firstAction[2] !== 'PROV_ASSUMED_7Q' && /do/i.test(firstAction[1])), + 'context-restore-provenance-order: option A jumped past the unverified first item to execute a later one', { collector: evalCollector, name }); + + const startsByVerifying = !!firstAction && /verify/i.test(firstAction[1]) && firstAction[2] === 'PROV_ASSUMED_7Q'; + recordE2E(evalCollector, name, 'Context Skills E2E', result, { + passed: exitOk && grouped && startsByVerifying, + }); + + expect(exitOk).toBe(true); + expect(grouped).toBe(true); + expect(startsByVerifying).toBe(true); + try { fs.rmSync(workDir, { recursive: true, force: true }); } catch {} + }, CAPTURE_MS); + // ── 4. /context-restore with zero saves → graceful empty-state ─────── testConcurrentIfSelected('context-restore-empty-state', async () => { const { workDir, gstackHome, slug } = setupWorkdir('empty');