FE-745: Merge slice worktrees into epic-scoped dir for verify-epic#152
FE-745: Merge slice worktrees into epic-scoped dir for verify-epic#152kostandinang wants to merge 1 commit into
Conversation
verify-epic now runs against a freshly-merged <parentSandboxDir>/__epic__/<epicId>/
built from the epic's completed slice worktrees. Slices apply in plan declaration
order; later slices overwrite earlier ones on the same path and the collision is
reported via the new epic-sandbox-merged event. Per-slice worktrees are not mutated.
Unblocks multi-slice cook runs (verify-epic previously fell back to the empty
parent worktree root, halting every multi-slice plan).
- new src/orchestrator/src/epic-sandbox-merge.{ts,test.ts}
- net-compiler.ts verify-epic case wires the merger + emits epic-sandbox-merged
- engine-contract.test.ts: 'verify-epic receives parent sandboxDir' assertion
updated to pin the new __epic__/<epicId>/ invariant
- memory/SPEC.md: new I124-K invariant alongside I123-K
- memory/PLAN.md: petri-epic-verification-merge → Recently Completed; bullet
retired from petri-graph-compilation (no longer blocked on FE-700)
PR SummaryMedium Risk Overview Adds Reviewed by Cursor Bugbot for commit 701c569. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
🤖 Augment PR SummarySummary: This PR fixes multi-slice epic verification by running Changes:
Technical Notes: Merge order follows slice declaration order; later slices overwrite earlier paths and conflicts are surfaced in the event payload. Per-slice worktrees remain unmodified and the epic sandbox is rebuilt on each verification run. 🤖 Was this summary useful? React with 👍 or 👎 |
| if (!existsSync(sliceDir)) continue; | ||
|
|
||
| for (const file of walkFiles(sliceDir)) { | ||
| const rel = relative(sliceDir, file); |
There was a problem hiding this comment.
src/orchestrator/src/epic-sandbox-merge.ts:44: relative(sliceDir, file) can contain .. if walkFiles() traverses symlinked directories (it uses statSync, which follows symlinks), which would let dest escape epicSandboxDir and overwrite files outside the epic sandbox. Consider preventing symlink traversal and/or validating that the computed rel stays within the slice root before copying.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 701c569. Configure here.
|
|
||
| const dest = join(epicSandboxDir, rel); | ||
| mkdirSync(join(dest, '..'), { recursive: true }); | ||
| cpSync(file, dest, { dereference: false }); |
There was a problem hiding this comment.
Merge paths escape epic directory
High Severity
By following directory symlinks, walkFiles can traverse outside a slice's intended directory. This causes relative to generate paths with .. segments, allowing cpSync to write files outside the epicSandboxDir and potentially overwrite other slice worktrees.
Reviewed by Cursor Bugbot for commit 701c569. Configure here.
| if (existsSync(epicSandboxDir)) { | ||
| rmSync(epicSandboxDir, { recursive: true, force: true }); | ||
| } | ||
| mkdirSync(epicSandboxDir, { recursive: true }); |
There was a problem hiding this comment.
Parent segment epic id wipes worktree
High Severity
epicSandboxDir is built with join(parentSandboxDir, '__epic__', epicId) without constraining epicId. An id like .. normalizes to parentSandboxDir, so the pre-merge rmSync recursively deletes the whole parent worktree (all per-slice sandboxes) before verify-epic runs.
Reviewed by Cursor Bugbot for commit 701c569. Configure here.
| const { version } = JSON.parse(readFileSync(pkgPath, 'utf8')) as { version: string }; | ||
| console.log(version); | ||
| process.exit(0); | ||
| } |
There was a problem hiding this comment.
Global version flag blocks cook
Low Severity
--version / -V are detected on any argument in process.argv via a Set of all tokens. Invocations like brunch cook <dir> --version print the version and exit before cook runs, even though cook does not define that flag.
Reviewed by Cursor Bugbot for commit 701c569. Configure here.
|
|
||
| const dest = join(epicSandboxDir, rel); | ||
| mkdirSync(join(dest, '..'), { recursive: true }); | ||
| cpSync(file, dest, { dereference: false }); |
There was a problem hiding this comment.
Path type collision crashes merge
Medium Severity
When two slices contribute the same relative path but one is a file and the other a directory, declaration-order overwrite uses cpSync without removing the prior node type. Node then throws (e.g. EISDIR/ENOTDIR), aborting verify-epic with no handled error—unlike content collisions where the last file wins cleanly.
Reviewed by Cursor Bugbot for commit 701c569. Configure here.



verify-epic now runs against a freshly-merged /epic//
built from the epic's completed slice worktrees. Slices apply in plan declaration
order; later slices overwrite earlier ones on the same path and the collision is
reported via the new epic-sandbox-merged event. Per-slice worktrees are not mutated.
Unblocks multi-slice cook runs (verify-epic previously fell back to the empty
parent worktree root, halting every multi-slice plan).
updated to pin the new epic// invariant
retired from petri-graph-compilation (no longer blocked on FE-700)