diff --git a/CHANGELOG.md b/CHANGELOG.md index 9209054..dd5ca78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 existing `threadManagement` capability, with the local writer kept as the fallback for a sidecar too old to know the method. +- **Change ledger records provenance.** Each entry now carries `derivedFrom` — + the files the turn read before making that change — answering the question + after "what changed" and "how do I undo it": _what was it derived from_. That + is what you ask when a generated file is wrong and you need to know which + input to fix. Shown by `deepcode ledger show`. Built on the existing ledger + rather than a second store. + + It is observed, not declared: only `Read` counts (`Grep`/`Glob` take a search + root and return many paths — calling the root an input claims a derivation the + turn did not make), a failed read is not an input, and the file being written + is excluded so an `Edit` does not look self-derived. Absent rather than empty + when there is nothing to say. + ### 🔒 Security - **A sub-agent did not inherit the file contract.** The `Task` delegation diff --git a/apps/cli/src/ledger-cmd.ts b/apps/cli/src/ledger-cmd.ts index 81cb16c..93dc13c 100644 --- a/apps/cli/src/ledger-cmd.ts +++ b/apps/cli/src/ledger-cmd.ts @@ -140,6 +140,11 @@ async function show( out.write(` actor : ${r.actor}${r.tool ? ` (${r.tool})` : ''}\n`); if (r.intent) out.write(` intent : ${r.intent}\n`); out.write(` paths : ${r.paths.length > 0 ? r.paths.join(', ') : '—'}\n`); + // Only shown when there is something to show. A "derived from: —" line on + // every Bash record would train people to stop reading the field. + if (r.derivedFrom && r.derivedFrom.length > 0) { + out.write(` from : ${r.derivedFrom.join(', ')}\n`); + } out.write(` summary : ${r.summary}\n`); if (r.rollbackHint) { out.write(` rollback : ${r.rollbackHint.kind}`); diff --git a/docs/FLOATBOAT_ADOPTION_PLAN.md b/docs/FLOATBOAT_ADOPTION_PLAN.md index 5e3642e..24d5a51 100644 --- a/docs/FLOATBOAT_ADOPTION_PLAN.md +++ b/docs/FLOATBOAT_ADOPTION_PLAN.md @@ -500,15 +500,15 @@ File Contract 接入（PR 2）是唯一需要谨慎评审的一步。 写下与计划不符的地方，比宣称"照计划完成"有用。 -| 项 | 计划 | 实际 | -| ---------------------------------- | ----------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| PR 0 的问题陈述 | 称无人值守可能"静默放行" | **计划写错了**。`ask` 路径本来就 fail-closed（`runHeadless` 传 `approval: async () => false`）。真正缺的是"停下来"的能力和可见性，PR #237 按事实重写了范围 | -| 权限档位的钳制 | 放在 PR 0 | 推迟到 PR 7。没有 opt-in 的钳制只是破坏，等 `TriggerProfile` 落地才安全 | -| 契约 `deny` 与 `bypassPermissions` | 计划未明确 | 实施时决定 **`deny` 不可被 `bypassPermissions` 豁免**。它是关于路径的常驻声明，不是逐次提示；否则契约最强的一句话也最容易被关掉 | -| 四客户端一致性测试 | 计划要求 4 个客户端逐字段相等 | 实际只有 CLI 与 app-server **独立解析**策略；VS Code / LSP 是协议瘦客户端，逐字节消费 server 的答复，构造上即相等。测试断言前两者，并在文档里说明后两者的理由 —— 不宣称验证了 4 条独立路径 | -| Grep/Glob 结果过滤 | 列为 PR 1 的已知缺口 | 仍未做。契约对 Grep/Glob 只裁决搜索根，命中结果里混入 deny 路径的内容需要工具输出层二次过滤 | -| 制品 `provenance` | 列在 PR 8（P2） | 未做 | -| 触发源抽象（ICS / watch） | 列在 PR 8（P2） | 未做。`cron` 仍只有时间源 | +| 项 | 计划 | 实际 | +| ---------------------------------- | ----------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| PR 0 的问题陈述 | 称无人值守可能"静默放行" | **计划写错了**。`ask` 路径本来就 fail-closed（`runHeadless` 传 `approval: async () => false`）。真正缺的是"停下来"的能力和可见性，PR #237 按事实重写了范围 | +| 权限档位的钳制 | 放在 PR 0 | 推迟到 PR 7。没有 opt-in 的钳制只是破坏，等 `TriggerProfile` 落地才安全 | +| 契约 `deny` 与 `bypassPermissions` | 计划未明确 | 实施时决定 **`deny` 不可被 `bypassPermissions` 豁免**。它是关于路径的常驻声明，不是逐次提示；否则契约最强的一句话也最容易被关掉 | +| 四客户端一致性测试 | 计划要求 4 个客户端逐字段相等 | 实际只有 CLI 与 app-server **独立解析**策略；VS Code / LSP 是协议瘦客户端，逐字节消费 server 的答复，构造上即相等。测试断言前两者，并在文档里说明后两者的理由 —— 不宣称验证了 4 条独立路径 | +| Grep/Glob 结果过滤 | 列为 PR 1 的已知缺口 | 仍未做。契约对 Grep/Glob 只裁决搜索根，命中结果里混入 deny 路径的内容需要工具输出层二次过滤 | +| 制品 `provenance` | 列在 PR 8（P2） | 已做。建在 change ledger 上而不是第二套存储：每条变更记录带 `derivedFrom`（本轮在写之前读过的文件）。**观察得来而非声明**——只算 `Read`，失败的读不算，被写的文件本身不算。见 [`change-ledger.md`](change-ledger.md) | +| 触发源抽象（ICS / watch） | 列在 PR 8（P2） | 未做。`cron` 仍只有时间源 | **未解假设的最终结论**（§7 提的四个）： diff --git a/docs/change-ledger.md b/docs/change-ledger.md index a992fd8..639abcb 100644 --- a/docs/change-ledger.md +++ b/docs/change-ledger.md @@ -55,6 +55,7 @@ digest wherever you like. "intent": "fix the expired-token branch in auth.ts", "paths": ["src/auth.ts"], "summary": "edited src/auth.ts", + "derivedFrom": ["schema.json"], "rollbackHint": { "kind": "snapshot", "ref": "7" }, } ``` @@ -64,6 +65,38 @@ records stay readable after the repo moves. `rollbackHint` points at the snapshot or git checkpoint already taken for that specific call — and is **absent when no checkpoint exists**, rather than guessing at one. +## Provenance — what a change came from + +`derivedFrom` lists the files the turn **read** before making the change. It +answers the third question, after "what changed" and "how do I undo it": _what +was this derived from_ — which is what you ask when a generated file is wrong and +you need to know which input to fix, or when something turns up in a commit and +you need to know what the turn had open. + +```bash +deepcode ledger show chg-lz4k2p-01 +# paths : src/client.ts +# from : config/gen.yaml, schema.json +``` + +It is **observed, not declared**. These are the reads the turn actually +performed, so a tool that ignored its inputs shows nothing rather than a +plausible list. Consequences worth knowing: + +- **Only `Read` counts.** `Grep` and `Glob` take a search _root_ and return many + paths; calling the root an input would claim a derivation the turn did not + make, and listing every hit would drown the real inputs in whatever the search + swept up. Narrow and true beats wide and approximate. +- **A failed read is not an input.** It gave the turn nothing, and crediting it + would send someone to fix a file that was never opened. +- **The file being written is excluded.** `Edit` reads its own target by + construction, and including it would make every edit look self-derived. +- The field is **absent**, not empty, when there is nothing to say. A field that + is always present is a field that stops being read. + +Provenance is a derivation record, not a dependency graph: it says what this one +turn read, not what the file transitively depends on. + ## What is not recorded - **Reads.** A ledger answering "what changed" has nothing to say about a read, diff --git a/packages/core/src/agent.test.ts b/packages/core/src/agent.test.ts index 9384302..1d5a58c 100644 --- a/packages/core/src/agent.test.ts +++ b/packages/core/src/agent.test.ts @@ -1023,6 +1023,74 @@ describe('runAgent', () => { }; } + const readTool: ToolHandler = { + name: 'Read', + definition: { name: 'Read', description: 'r', inputSchema: { type: 'object' } }, + async execute() { + return { content: 'file contents' }; + }, + }; + + it('records what the write was derived from', async () => { + // Observed, not declared: the reads the turn actually performed. This is + // the question you ask when a generated file is wrong and you need to + // know which input to fix. + const ledger = recordingSink(); + await runAgent({ + provider: new MockProvider([ + toolUse('reading', { + type: 'tool_use', + id: 'r-1', + name: 'Read', + input: { file_path: 'schema.json' }, + }), + toolUse('writing', writeCall()), + endTurn('done'), + ]), + tools: new ToolRegistry([readTool, writeTool]), + systemPrompt: '', + userMessage: 'regenerate the client', + model: 'deepseek-chat', + cwd, + ledger: ledger.sink, + }); + const [, record] = ledger.entries[0]!; + expect(record.derivedFrom).toEqual(['schema.json']); + }); + + it('does not credit a read that failed', async () => { + // A read that errored gave the turn nothing, so claiming the output came + // from it would send someone to fix a file that was never opened. + const failingRead: ToolHandler = { + name: 'Read', + definition: { name: 'Read', description: 'r', inputSchema: { type: 'object' } }, + async execute() { + return { content: 'ENOENT', isError: true }; + }, + }; + const ledger = recordingSink(); + await runAgent({ + provider: new MockProvider([ + toolUse('reading', { + type: 'tool_use', + id: 'r-1', + name: 'Read', + input: { file_path: 'missing.json' }, + }), + toolUse('writing', writeCall()), + endTurn('done'), + ]), + tools: new ToolRegistry([failingRead, writeTool]), + systemPrompt: '', + userMessage: 'regenerate', + model: 'deepseek-chat', + cwd, + ledger: ledger.sink, + }); + const [, record] = ledger.entries[0]!; + expect(record.derivedFrom).toBeUndefined(); + }); + it('records a completed write, with the turn request as intent', async () => { const ledger = recordingSink(); await runAgent({ diff --git a/packages/core/src/agent.ts b/packages/core/src/agent.ts index 6a454f1..8fd4605 100644 --- a/packages/core/src/agent.ts +++ b/packages/core/src/agent.ts @@ -6,7 +6,7 @@ import type { PermissionRules } from './config/types.js'; import type { FileContract } from './config/file-contract.js'; import type { UnattendedApprovalPolicy } from './cron/index.js'; import type { LedgerSink } from './ledger/index.js'; -import { buildToolCallRecord, ledgerKindForTool } from './ledger/record-tool-call.js'; +import { buildToolCallRecord, ledgerKindForTool, readPathFor } from './ledger/record-tool-call.js'; import { dispatchToolCall, type DispatchVerdict } from './harness/tool-dispatcher.js'; import { TaskManager, type TaskRunner } from './tasks/manager.js'; import type { HookDispatcher } from './hooks/index.js'; @@ -285,6 +285,11 @@ export async function runAgent(opts: RunAgentOptions): Promise { // modeSignal is mutable — EnterPlanMode / ExitPlanMode flip these; the agent // loop owner reads them after the run to switch mode (default ⇄ plan). const modeSignal: { exitPlanMode?: boolean; enterPlanMode?: boolean } = {}; + + // Inputs this run has read, in call order, deduped. Provenance for every + // change it goes on to make: the question "which input do I fix" is only + // answerable if somebody wrote down what was open at the time. + const readsThisTurn = new Set(); const toolCtx: ToolContext = { cwd: opts.cwd, signal: opts.signal, @@ -797,6 +802,15 @@ export async function runAgent(opts: RunAgentOptions): Promise { } } + // Reads observed so far in this run become the provenance of whatever + // this turn writes next. Recorded here, at the one place every completed + // tool call passes through, for the same reason the ledger append is + // here: a per-tool hook is a hook a new tool forgets. + if (!tr.isError) { + const readPath = readPathFor(toolUse.name, toolUse.input); + if (readPath) readsThisTurn.add(readPath); + } + // One write point for the whole loop. Per-tool writes are exactly the // shape AGENTS.md rules out — a new mutating tool would silently go // unrecorded if each site had to remember. @@ -812,6 +826,7 @@ export async function runAgent(opts: RunAgentOptions): Promise { threadId: opts.session?.id, turnId: opts.session?.turnId, snapshotSeq: preSeq, + readPaths: [...readsThisTurn], }); // Never let bookkeeping fail a completed edit; FileLedger already // swallows its own I/O errors, this covers a host-supplied sink. diff --git a/packages/core/src/ledger/index.ts b/packages/core/src/ledger/index.ts index b6c0a00..ca089b8 100644 --- a/packages/core/src/ledger/index.ts +++ b/packages/core/src/ledger/index.ts @@ -51,6 +51,20 @@ export interface LedgerRecord { intent?: string; /** Workspace-relative paths. Empty when the effect cannot be pinned to files. */ paths: string[]; + /** + * Workspace-relative paths this turn read before making this change. + * + * The ledger already answers "what changed" and "how do I undo it". This + * answers "what was it derived from" — the question you ask when a generated + * file is wrong and you need to know which input to fix, or when a secret + * turns up somewhere and you need to know what the turn had open. + * + * Observed, not declared: these are the reads the turn actually performed, so + * a tool that ignored its inputs shows an empty list rather than a plausible + * one. Absent on records written before this existed, and on turns that read + * nothing. + */ + derivedFrom?: string[]; summary: string; rollbackHint?: RollbackHint; } diff --git a/packages/core/src/ledger/record-tool-call.test.ts b/packages/core/src/ledger/record-tool-call.test.ts new file mode 100644 index 0000000..d27b110 --- /dev/null +++ b/packages/core/src/ledger/record-tool-call.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, it } from 'vitest'; +import { buildToolCallRecord, readPathFor } from './record-tool-call.js'; + +// Provenance: what a change was derived from, so a wrong generated file leads +// back to the input that produced it. +describe('derivedFrom', () => { + const base = { cwd: '/work/repo', intent: 'regenerate the client' }; + + it('records the reads that preceded the write', () => { + const record = buildToolCallRecord({ + ...base, + tool: 'Write', + input: { file_path: '/work/repo/src/client.ts', content: '…' }, + readPaths: ['/work/repo/schema.json', '/work/repo/config/gen.yaml'], + }); + expect(record?.derivedFrom).toEqual(['config/gen.yaml', 'schema.json']); + }); + + it('is absent when the turn read nothing', () => { + // Not an empty array: a field that is always present is a field that stops + // being read. + const record = buildToolCallRecord({ + ...base, + tool: 'Write', + input: { file_path: 'a.ts', content: 'x' }, + }); + expect(record?.derivedFrom).toBeUndefined(); + }); + + it('excludes the file being written', () => { + // Edit reads its own target by construction; listing it would make every + // edit look self-derived. + const record = buildToolCallRecord({ + ...base, + tool: 'Edit', + input: { file_path: '/work/repo/src/a.ts', old_string: 'x', new_string: 'y' }, + readPaths: ['/work/repo/src/a.ts', '/work/repo/schema.json'], + }); + expect(record?.derivedFrom).toEqual(['schema.json']); + }); + + it('normalizes to workspace-relative and dedupes', () => { + const record = buildToolCallRecord({ + ...base, + tool: 'Write', + input: { file_path: 'out.ts', content: 'x' }, + readPaths: ['/work/repo/schema.json', 'schema.json', '/work/repo/./schema.json'], + }); + expect(record?.derivedFrom).toEqual(['schema.json']); + }); + + it('attaches to Bash too — a command has inputs even without a declarable output', () => { + const record = buildToolCallRecord({ + ...base, + tool: 'Bash', + input: { command: 'make generate' }, + readPaths: ['/work/repo/Makefile'], + }); + expect(record?.derivedFrom).toEqual(['Makefile']); + }); +}); + +describe('readPathFor', () => { + it('reports a Read', () => { + expect(readPathFor('Read', { file_path: '/a/b.ts' })).toBe('/a/b.ts'); + }); + + it('ignores Grep and Glob', () => { + // They take a search *root* and return many paths. Calling the root an + // input claims a derivation the turn did not make; enumerating every hit + // drowns the real inputs in whatever the search swept up. + expect(readPathFor('Grep', { path: '/a' })).toBeUndefined(); + expect(readPathFor('Glob', { path: '/a' })).toBeUndefined(); + }); + + it('ignores writes and anything without a path', () => { + expect(readPathFor('Write', { file_path: '/a' })).toBeUndefined(); + expect(readPathFor('Read', {})).toBeUndefined(); + }); +}); diff --git a/packages/core/src/ledger/record-tool-call.ts b/packages/core/src/ledger/record-tool-call.ts index e1e0f68..3374f12 100644 --- a/packages/core/src/ledger/record-tool-call.ts +++ b/packages/core/src/ledger/record-tool-call.ts @@ -26,6 +26,21 @@ const RECORDABLE: Record = { Bash: { kind: 'changes' }, }; +/** + * The path a read tool opened, or undefined for anything else. + * + * Only `Read` and `NotebookEdit`-adjacent single-file reads count. `Grep` and + * `Glob` take a search *root* and return many paths; calling the root an input + * would claim a derivation the turn did not make, and enumerating every hit + * would drown the real inputs in whatever the search happened to sweep up. + * Provenance is more useful narrow and true than wide and approximate. + */ +export function readPathFor(tool: string, input: Record): string | undefined { + if (tool !== 'Read') return undefined; + const raw = input.file_path; + return typeof raw === 'string' && raw ? raw : undefined; +} + export function isRecordableTool(tool: string): boolean { return tool in RECORDABLE; } @@ -45,6 +60,11 @@ export interface ToolCallRecordInput { actor?: string; /** Snapshot/checkpoint sequence captured before the call, if any. */ snapshotSeq?: number; + /** + * Absolute paths this turn read before this call. Normalized and deduped + * here; the caller only has to observe. + */ + readPaths?: string[]; } /** Build the record for a completed tool call, or null if the tool isn't recordable. */ @@ -61,6 +81,18 @@ export function buildToolCallRecord(input: ToolCallRecordInput): NewLedgerRecord } } + const written = new Set(paths); + const derivedFrom = [ + ...new Set( + (input.readPaths ?? []) + .map((raw) => normalizeContractPath(input.cwd, raw) ?? raw) + // A file the call is itself writing is not an input it was derived + // from — Edit reads its target by construction, and listing that would + // make every edit look self-derived. + .filter((path) => !written.has(path)), + ), + ].sort(); + return { actor: input.actor ?? 'agent', tool: input.tool, @@ -68,6 +100,7 @@ export function buildToolCallRecord(input: ToolCallRecordInput): NewLedgerRecord ...(input.turnId ? { turnId: input.turnId } : {}), ...(input.intent ? { intent: input.intent } : {}), paths, + ...(derivedFrom.length > 0 ? { derivedFrom } : {}), summary: summarize(input.tool, input.input, paths), ...(rollbackFor(input) ? { rollbackHint: rollbackFor(input)! } : {}), };