test(engine): assert mutation-context attribution across engine suites (4/5) - #3432
test(engine): assert mutation-context attribution across engine suites (4/5)#3432gsxdsm wants to merge 7 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Filename | Overview |
|---|---|
| packages/engine/src/tests/mutation-context-matchers.ts | Adds distinct attributed and unattributed context matchers; the attributed matcher now rejects the exact marker identified by the prior review. |
| packages/engine/src/tests/mutation-context-matchers.test.ts | Adds direct controls proving attributed contexts pass while unattributed, missing, and empty contexts fail. |
| packages/engine/src/executor/worktree-branch-conflict-handle.ts | Attributes the reclaimed-worktree task update using the same executor run context used by the recovery log. |
Reviews (14): Last reviewed commit: "Merge remote-tracking branch 'origin/ide..." | Re-trigger Greptile
e26a7ef to
d407a8d
Compare
404e3a8 to
be439e7
Compare
d407a8d to
5ce4b60
Compare
be439e7 to
a167214
Compare
5ce4b60 to
8c23e8c
Compare
a167214 to
3129138
Compare
|
@coderabbitai review |
|
|
Review feedback addressed in Fixed — the findings were correct:
The matcher finding deserves its own note. It was exactly right, and the blast radius was real: tightening exposed 15 genuinely unattributed writes across 11 suites that the loose matcher had been passing as attributed. Those now assert Not applied, with reasons:
Verification. Measured against a baseline of the same tree with these edits reverted, so pre-existing failures aren't attributed to this change: 77 failed files / 258 failed tests before → 77 / 263 after, with no file this change touches newly failing. |
8c23e8c to
ff02870
Compare
3129138 to
ccec4fb
Compare
ff02870 to
2127d7d
Compare
ccec4fb to
626dc8b
Compare
|
Stack rebased so every chunk carries its own review fixes; the stack tip is byte-identical to the full branch ( One further real finding fixed ( Re-posted findings that are already fixed in the current tree (the bot re-reviewed against pre-fix line numbers):
Still declined: the FNXC-trace requests across the executor files. All ten already carry FNXC comments and the changed blocks are mechanical propagation of a conversion documented once at its seam; ten near-identical "attribution threaded here" notes are noise, not traceability. CI: #3428 7/7 green, #3429 6/6, #3431–#3433 all green. The only red mark is the Greptile check itself, which reports its own open findings rather than a build result. |
2127d7d to
1e03e16
Compare
626dc8b to
6b1eba8
Compare
1e03e16 to
14d1a33
Compare
6b1eba8 to
2ed6e5f
Compare
14d1a33 to
621502b
Compare
2ed6e5f to
ac4c482
Compare
621502b to
55db108
Compare
…all site (4/5) Includes the tightened ANY_MUTATION_CONTEXT matcher (it previously accepted the "system:unattributed" marker, so every assertion using it passed whether the write was attributed or not) plus its own proven-failing controls. Fusion-Task-Id: FN-8821 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ac4c482 to
ab5d8ca
Compare
Restack 4/5 onto the updated 3/5 base. Conflict resolutions keep both sides: - executor-worktree.test.ts: 3/5 reclaim provenance (branch + branchWriteOrigin, including the operator-owned Fusion-namespaced case) plus 4/5 ANY_MUTATION_CONTEXT. - identity-permissions-shadow.test.ts (add/add): union 4/5 shadow-evaluation suites with the 3/5 gitCases it.each corpus and null-git assertion.
…tity/4-engine-tests
The restacked 3/5 reclaim patch stamps branchWriteOrigin; pass runContextFor on that updateTask so 4/5 ANY_MUTATION_CONTEXT assertions stay true. Fusion-Task-Id: FN-8821
…tity/4-engine-tests
# Conflicts: # packages/engine/src/__tests__/executor-step-session.test.ts
| log: [], | ||
| branch: "fusion/fn-permission-denied", | ||
| baseBranch: "main", | ||
| worktree: "/tmp/fusion-fn-permission-denied", |
| maxWorktrees: 4, | ||
| pollIntervalMs: 15000, | ||
| }); | ||
| const executor = new TaskExecutor(store, "/tmp/test"); |
|
|
||
| function makeExecutor() { | ||
| const store = createMockStore(); | ||
| const executor = new TaskExecutor(store, "/tmp/test") as unknown as { |
| "FN-050", | ||
| expect.stringContaining("Cleaned up conflicting worktree, retrying"), | ||
| "/tmp/test/.worktrees/swift-falcon", | ||
| "/tmp/test/.worktrees/swift-falcon", ANY_MUTATION_CONTEXT, |
| expect(execMock).not.toHaveBeenCalledWith(expect.stringContaining("git worktree remove --force"), expect.anything()); | ||
| expect(execMock).not.toHaveBeenCalledWith(expect.stringContaining("git branch -D"), expect.anything()); | ||
| expect(store.updateTask).toHaveBeenCalledWith("FN-9001", expect.objectContaining({ worktree: "/tmp/live", branch: "fusion/fn-9001" })); | ||
| expect(store.updateTask).toHaveBeenCalledWith("FN-9001", expect.objectContaining({ worktree: "/tmp/live", branch: "fusion/fn-9001" }), UNATTRIBUTED_MUTATION_CONTEXT); |
| expect(recovered).toBe(1); | ||
| expect(execMock).not.toHaveBeenCalledWith(expect.stringContaining("git worktree remove --force"), expect.anything()); | ||
| expect(store.updateTask).toHaveBeenCalledWith("FN-9001", expect.objectContaining({ worktree: "/tmp/live", branch: "fusion/fn-9001" })); | ||
| expect(store.updateTask).toHaveBeenCalledWith("FN-9001", expect.objectContaining({ worktree: "/tmp/live", branch: "fusion/fn-9001" }), UNATTRIBUTED_MUTATION_CONTEXT); |
|
|
||
| expect(execMock).not.toHaveBeenCalledWith(expect.stringContaining("git worktree remove --force"), expect.anything()); | ||
| expect(store.updateTask).toHaveBeenCalledWith("FN-9001", expect.objectContaining({ worktree: "/tmp/live", branch: "fusion/fn-other" })); | ||
| expect(store.updateTask).toHaveBeenCalledWith("FN-9001", expect.objectContaining({ worktree: "/tmp/live", branch: "fusion/fn-other" }), UNATTRIBUTED_MUTATION_CONTEXT); |
ThreatCrush Security Scan4524 finding(s) HIGH/CRITICAL: 43 | MEDIUM: 3990 | LOW: 491
…and 4474 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
# Conflicts: # packages/engine/src/__tests__/auto-recovery-branch-worktree.test.ts # packages/engine/src/__tests__/executor-execution-policy-renamed-columns.test.ts # packages/engine/src/__tests__/executor-graph-requeue-gate.test.ts # packages/engine/src/__tests__/executor-outer-dispatch-dependency-gate.test.ts # packages/engine/src/__tests__/executor-step-session.test.ts # packages/engine/src/__tests__/executor-worktree.test.ts # packages/engine/src/__tests__/merger-ai.test.ts # packages/engine/src/__tests__/merger-finalize-unproven.real-git.test.ts # packages/engine/src/__tests__/merger-merge-lifecycle.test.ts # packages/engine/src/__tests__/merger-trait-rekey.test.ts # packages/engine/src/__tests__/promote-force-unplanned.test.ts # packages/engine/src/__tests__/reliability-interactions/auto-revive-and-watchdog.test.ts # packages/engine/src/__tests__/reliability-interactions/merge-node-paused-abort-retryable.test.ts # packages/engine/src/__tests__/reliability-interactions/todo-inprogress-flapping.test.ts # packages/engine/src/__tests__/reliability-interactions/worktree-incomplete-session-start.test.ts # packages/engine/src/__tests__/replan-target.test.ts # packages/engine/src/__tests__/restart-recovery-coordinator.test.ts # packages/engine/src/__tests__/self-healing-ghost-branch-recovery.test.ts # packages/engine/src/__tests__/self-healing-in-progress-limbo.test.ts # packages/engine/src/__tests__/self-healing-query-filter-blindness.test.ts # packages/engine/src/__tests__/self-healing-reclaim-live-zero-commits.test.ts # packages/engine/src/__tests__/self-healing-reclaim-paused-review.test.ts # packages/engine/src/__tests__/self-healing-trait-rekey.test.ts # packages/engine/src/__tests__/self-healing.test.ts # packages/engine/src/__tests__/workflow-graph-optional-step-fix.test.ts
| import * as worktreePoolModule from "../../worktree/worktree-pool.js"; | ||
|
|
||
| function git(cwd: string, command: string): string { | ||
| return execSync(`git ${command}`, { cwd, encoding: "utf8", stdio: ["pipe", "pipe", "pipe"] }).trim(); |
| logEntry: vi.fn().mockResolvedValue(undefined), | ||
| moveTask: vi.fn().mockResolvedValue(undefined), | ||
| recordRunAuditEvent: vi.fn().mockResolvedValue(undefined), | ||
| getRootDir: vi.fn().mockReturnValue("/tmp/test-project"), |
| column: "in-progress", | ||
| paused: false, | ||
| branch: null, | ||
| worktree: "/tmp/test-project/.worktrees/missing-fn-5149", |
| (store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([baseTask]); | ||
|
|
||
| const manager = new SelfHealingManager(store, { | ||
| rootDir: "/tmp/test-project", |
| vi.mocked(existsSync).mockImplementation((path) => path === baseTask.worktree); | ||
| (store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([baseTask]); | ||
| const manager = new SelfHealingManager(store, { | ||
| rootDir: "/tmp/test-project", |
| it("skips tasks whose branch is still set", async () => { | ||
| (store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([{ ...baseTask, branch: "fusion/fn-5149" }]); | ||
| const manager = new SelfHealingManager(store, { | ||
| rootDir: "/tmp/test-project", |
| it("skips tasks currently claimed by the executor", async () => { | ||
| (store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([baseTask]); | ||
| const manager = new SelfHealingManager(store, { | ||
| rootDir: "/tmp/test-project", |
| it("skips paused tasks", async () => { | ||
| (store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([{ ...baseTask, paused: true }]); | ||
| const manager = new SelfHealingManager(store, { | ||
| rootDir: "/tmp/test-project", |
| it("skips tasks still within the grace window", async () => { | ||
| (store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([{ ...baseTask, updatedAt: "2026-05-20T12:04:30.000Z" }]); | ||
| const manager = new SelfHealingManager(store, { | ||
| rootDir: "/tmp/test-project", |
| }); | ||
| (store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([baseTask]); | ||
| const manager = new SelfHealingManager(store, { | ||
| rootDir: "/tmp/test-project", |
Stack 4/5 — 126 files. Base:
identity/3-engine-src.Test-only. Asserts the mutation context positionally rather than waving it through, so the day U9/U11/U13 hand a path a real actor, the assertion fails and names the line instead of silently accepting whatever arrived.
🤖 Generated with Claude Code