-
Notifications
You must be signed in to change notification settings - Fork 0
Implement plan validation and ledger-backed review core #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
422527c
Add Evidence Desk design system and design review specs
mchwang 933744b
Add plan schema, import format, and plan-author prompt
mchwang 5edf756
Fix clone isolation and plan validation contracts
mchwang 270c7c9
Address automated review of plan safety and design docs
mchwang 2229e88
Address second Copilot review: container lock-down, symlinks, cmd wor…
mchwang c9e5260
Implement plan validation and ledger-backed Git review core
mchwang 537199e
Preserve BOM changes in Git review evidence
mchwang 30729a2
Prevent Git preferences from hiding review changes
mchwang 39a6d70
Track file scope at the owning commit across renames
mchwang 9020224
Isolate Git object reads from inherited environment
mchwang c88b510
Reject borrowed Git object stores and verify rename evidence
mchwang 7efeeba
Bound cumulative Git blob memory per review history
mchwang 8cf8320
Reject symlinked Git object storage before history reads
mchwang 876d142
Bound directory scanning and document Git selection policy
mchwang 1d6f5ac
Bound aggregate Git diff bytes and retained file records
mchwang d798e1c
Merge approved design baseline from main
mchwang f223089
Align plan library and review identities with merged v1 contracts
mchwang 9a2df1f
Reject all Unicode control characters in v1 paths and commands
mchwang a8534cc
Update library examples and verify scored rename parsing
mchwang a6f2717
Bound history replay and fix rename lineage and link target checks
mchwang 38d36b8
Retain metadata ownership when deleted paths are reused
mchwang 611aedd
Reject Git graft and shallow ancestry metadata
mchwang File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| name: CI | ||
| on: | ||
| push: | ||
| branches: [main, 'codex/**'] | ||
| pull_request: | ||
| permissions: | ||
| contents: read | ||
| jobs: | ||
| test: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '26.7.0' | ||
| cache: npm | ||
| - run: npm ci --ignore-scripts | ||
| - run: npm run typecheck | ||
| - run: npm test |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| node_modules/ | ||
| coverage/ | ||
| dist/ | ||
| .DS_Store |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| # codeboost | ||
|
|
||
| Review agent-made Git changes one plan item at a time. The approved plan lists each item's files and acceptance checks; the review engine shows which item produced each change and flags foreign or overlapping work. | ||
|
|
||
| **Status:** the first library slice is implemented. There is no application, server, agent runner, database, or merge command yet. Follow the [build order](docs/designs/codeboost-plan-indexed-review.md#build-order-and-the-gono-go-check); the read-only screen and real-PR go/no-go experiment come before agent execution. | ||
|
|
||
| ## Development | ||
|
|
||
| Requires Node 26.7 or later and Git. | ||
|
|
||
| ```sh | ||
| npm ci --ignore-scripts | ||
| npm run typecheck | ||
| npm test | ||
| ``` | ||
|
|
||
| Tests create disposable local repositories. They do not invoke agents, access GitHub, or execute plan acceptance commands. | ||
|
|
||
| ## Library | ||
|
|
||
| - `core/plan.ts`: schema validation; YAML/JSON import; projected file-state and dependency checks; literal command parsing; individual suggestion validation and application. | ||
| - `git/history.ts`: reads an immutable base-to-head commit range and file blobs. Uses argv, disables external diff/textconv helpers, hooks, and replacement objects. | ||
| - `core/linking.ts`: replays line changes using an explicit `Map<commitSha, planItemId | null>` supplied by the caller. Trailers never establish ownership. Foreign work is Unplanned; overlapping item edits are Ambiguous; undeclared edits stay on their owner's row as out of scope. | ||
| - `core/approvals.ts`: approval snapshots, dependency staleness, assignments, and accept-as-is choices keyed by content and duplicate occurrence/count. | ||
|
|
||
| Example from TypeScript (Node can load these source modules): | ||
|
|
||
| ```ts | ||
| import { importPlan } from './core/plan.ts'; | ||
| import { readHistory } from './git/history.ts'; | ||
| import { linkHistory } from './core/linking.ts'; | ||
|
|
||
| const { plan, warnings } = importPlan(planText, 'yaml', { | ||
| identity: storedPlanIdentity, // Stable repositoryId, taskId, and planId from storage. | ||
| baseEntries: entriesAtBaseCommit, // Typed file/gitlink entries; symlinks include target text. | ||
| pathKey: checkoutPathKey, // Actual checkout case/Unicode identity; fail if unknown. | ||
| allowedCommands: [['npm', 'test']], | ||
| issue: 412, | ||
| }, nextRevision); | ||
| const history = readHistory(repoPath, baseCommit, headCommit); | ||
| const segments = linkHistory(plan, history, trustedCommitLedger, checkoutPathKey); | ||
| ``` | ||
|
|
||
| Inputs such as `planText` and the ledger must come from the caller. The future `runner/store` owns the database and ledger; this library does not infer them from commit messages. Before saving a suggested edit, the store must load its captured identity/revision binding by opaque suggestion ID, reject canceled or consumed IDs, and compare-and-swap the plan revision plus consume/invalidate old suggestions in one transaction. The pure `applySuggestion` function requires that trusted binding and validates a copy, but cannot lock storage or prevent replay by itself. Applying one card stales its siblings; refresh and review regenerated cards before the next Apply. Approvals and choices likewise require the stored plan identity. | ||
|
|
||
| ## Current limits and safety | ||
|
|
||
| - History must be linear and descend from the requested base. Merge histories are rejected with a rebase instruction; repositories using grafts, shallow ancestry, object alternates, or symlinked object storage are rejected (storage inspection is limited to 100,000 entries). Reads are bounded to 500 commits, 32 MiB per Git response, 64 MiB of unique blob bytes across the history (callers may lower `maxBlobBytes`), 8 MiB of cumulative diff output, 20,000 cumulative file records (including the final diff), and a shared 30-second monotonic deadline for a read. Git children receive only the remaining time and are killed on timeout. Filesystem inspections check the deadline between operations; a blocked filesystem syscall still requires an external worker supervisor. Oversized work fails explicitly. | ||
| - Linking separately bounds cumulative split lines and candidate segments to 100,000 each, reference/work operations to 1,000,000, and text/origin strings to 32 Mi UTF-16 code units. It checks before expanding lines/origin sets; grouping no longer repeatedly splits accumulated content. Callers may lower these budgets. Linking has a 30-second overall deadline and each line diff uses at most 2 seconds or the remaining total, whichever is smaller. | ||
| - The caller selects and trusts the repository and its Git administrative directory. Normal Git discovery, linked-worktree gitfiles, and symlinked gitdirs are supported; object-storage links and alternates inside that selected gitdir are rejected. This adapter is not a filesystem-containment boundary for untrusted repository roots. | ||
| - Git administrative metadata and object storage must remain unchanged during a read; these library checks do not isolate a concurrently hostile filesystem. | ||
| - Ownership uses line diffs, not semantic inference. Within one replacement block, new lines inherit all affected owners conservatively. Function context comes from Git hunk headers, not an AST. | ||
| - The importer requires accurate typed base entries, stable plan identity, a selected issue, and a trusted checkout path-identity function. It rejects path traversal, Git metadata paths, and traversal through a listed file/symlink/submodule. Runtime symlink and write-scope enforcement belong to the future container/runner; plan validation alone is not a sandbox. | ||
| - Allowed commands restrict accidents, not hostile programs or changed scripts. Parsing returns argv and never executes it. An unlisted valid command is a warning and must not run until allowed. | ||
| - No code here claims container isolation, vendor-only network access, credential protection, or safe dependency installation. Those controls must be implemented before running agents. | ||
|
|
||
| See [implementation decisions and evidence](docs/implementation/build-step-1.md) and the [plan format](docs/plan-format.md). |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| import { identityKey, type PlanIdentity } from './identity.ts'; | ||
| import type { Plan, PlanItem } from './plan.ts'; | ||
| import type { Segment } from './linking.ts'; | ||
|
|
||
| /** Stable representation ignores object-key order and normalizes CRLF, not whitespace. */ | ||
| function stable(value: unknown): string { | ||
| if (typeof value === 'string') return JSON.stringify(value.replace(/\r\n/g, '\n')); | ||
| if (Array.isArray(value)) return `[${value.map(stable).join(',')}]`; | ||
| if (value !== null && typeof value === 'object') return `{${Object.entries(value).sort(([a], [b]) => a.localeCompare(b)).map(([key, val]) => `${JSON.stringify(key)}:${stable(val)}`).join(',')}}`; | ||
| return JSON.stringify(value); | ||
| } | ||
| const contentKey = (s: Segment) => stable({ path: s.path, oldPath: s.oldPath, kind: s.kind, operation: s.operation, content: s.content }); | ||
| export interface SegmentChoice { key: string; action: 'assign' | 'accept'; item: string | null } | ||
| /** Position among identical segments and total copies prevent approval transfer. */ | ||
| export function choiceKeys(segments: readonly Segment[], identity: PlanIdentity): string[] { | ||
| const identityValue = identityKey(identity); | ||
| const counts = new Map<string, number>(), seen = new Map<string, number>(); | ||
| for (const segment of segments) { const key = contentKey(segment); counts.set(key, (counts.get(key) ?? 0) + 1); } | ||
| return segments.map(segment => { | ||
| const key = contentKey(segment), copy = (seen.get(key) ?? 0) + 1; seen.set(key, copy); | ||
| return stable([identityValue, key, copy, counts.get(key)]); | ||
| }); | ||
| } | ||
| export function applyChoices(plan: Plan, segments: readonly Segment[], choices: readonly SegmentChoice[], identity: PlanIdentity): Segment[] { | ||
| const keys = choiceKeys(segments, identity); | ||
| const byKey = new Map(choices.map(choice => [choice.key, choice])); | ||
| return segments.map((segment, i) => { | ||
| const choice = byKey.get(keys[i]!); | ||
| if (!choice || !['Ambiguous', 'Unplanned'].includes(segment.row)) return { ...segment }; | ||
| if (choice.action === 'accept') return { ...segment, row: 'Accepted' }; | ||
| if (!plan.items.some(item => item.id === choice.item)) throw new Error('Assigned item does not exist.'); | ||
| return { ...segment, row: choice.item! }; | ||
| }); | ||
| } | ||
| export interface Approval { item: string; fingerprint: string } | ||
| function fingerprint(item: PlanItem, segments: readonly Segment[], identity: PlanIdentity): string { | ||
| return stable({ identity: identityKey(identity), item, segments: segments.filter(s => s.row === item.id).map(s => ({ | ||
| path: s.path, oldPath: s.oldPath, kind: s.kind, operation: s.operation, | ||
| content: s.content, context: s.context, owners: [...s.owners].sort(), | ||
| })) }); | ||
| } | ||
| export function approveItem(plan: Plan, segments: readonly Segment[], itemId: string, identity: PlanIdentity, confirmNoChange = false): Approval { | ||
| const item = plan.items.find(item => item.id === itemId); | ||
| if (!item) throw new Error('Unknown item.'); | ||
| if (!segments.some(segment => segment.row === itemId) && !confirmNoChange) throw new Error('Confirm no change needed before approving.'); | ||
| return { item: itemId, fingerprint: fingerprint(item, segments, identity) }; | ||
| } | ||
| export function approvalStates(plan: Plan, segments: readonly Segment[], approvals: readonly Approval[], identity: PlanIdentity): Record<string, 'unreviewed' | 'approved' | 'stale'> { | ||
| const result: Record<string, 'unreviewed' | 'approved' | 'stale'> = Object.create(null); | ||
| for (const item of plan.items) { | ||
| const approval = approvals.find(approval => approval.item === item.id); | ||
| result[item.id] = !approval ? 'unreviewed' : approval.fingerprint !== fingerprint(item, segments, identity) || | ||
| item.depends_on.some(dep => result[dep] === 'stale') ? 'stale' : 'approved'; | ||
| } | ||
| return result; | ||
| } | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| /** Stable IDs from trusted application storage, never model output or UI selection. */ | ||
| export interface PlanIdentity { repositoryId: string; taskId: string; planId: string } | ||
| export function identityKey(identity: PlanIdentity): string { | ||
| const values = [identity?.repositoryId, identity?.taskId, identity?.planId]; | ||
| if (!values.every(v => typeof v === 'string' && v.length > 0)) throw new Error('Stable repository/task/plan identity is required.'); | ||
| return JSON.stringify(values); | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.