Skip to content

FE-745: Merge slice worktrees into epic-scoped dir for verify-epic#152

Open
kostandinang wants to merge 1 commit into
ka/fe-743-petri-parallel-executionfrom
ka/fe-745-petri-epic-verification-merge
Open

FE-745: Merge slice worktrees into epic-scoped dir for verify-epic#152
kostandinang wants to merge 1 commit into
ka/fe-743-petri-parallel-executionfrom
ka/fe-745-petri-epic-verification-merge

Conversation

@kostandinang
Copy link
Copy Markdown
Contributor

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).

  • 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// 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)

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)
@cursor
Copy link
Copy Markdown

cursor Bot commented May 22, 2026

PR Summary

Medium Risk
Changes filesystem behavior for epic verification by materializing and using a merged worktree plus emitting a new report event; mistakes could cause missing artifacts or unexpected overwrites during cook runs. Impact is contained to orchestrator execution/test paths and is covered by new/updated tests.

Overview
verify-epic no longer runs against the parent sandbox dir; it now rebuilds and uses a merged epic sandbox at __epic__/<epicId>/ composed from completed slice worktrees in plan declaration order, recording path-collision conflicts.

Adds mergeSlicesIntoEpicSandbox plus focused tests, updates the engine contract test to assert the new sandboxDir and presence of an epic-sandbox-merged report event, and documents the new invariant in memory/SPEC.md/memory/PLAN.md. Also adds brunch --version/-V output to the CLI.

Reviewed by Cursor Bugbot for commit 701c569. Bugbot is set up for automated code reviews on this repo. Configure here.

Copy link
Copy Markdown
Contributor Author

@kostandinang kostandinang changed the title FE-745: merge slice worktrees into epic-scoped dir for verify-epic FE-745: Merge slice worktrees into epic-scoped dir for verify-epic May 22, 2026
@augmentcode
Copy link
Copy Markdown

augmentcode Bot commented May 22, 2026

🤖 Augment PR Summary

Summary: This PR fixes multi-slice epic verification by running verify-epic against a deterministic, freshly-merged epic-scoped sandbox directory.

Changes:

  • Added mergeSlicesIntoEpicSandbox to materialize <parentSandboxDir>/__epic__/<epicId>/ from completed slice worktrees, with last-wins collision handling.
  • Updated the Petri net compiler’s verify-epic wiring to invoke the merger and emit a new epic-sandbox-merged report event.
  • Extended contract coverage to assert verify-epic receives the merged epic sandbox and that the merge event is logged.
  • Documented the new invariant (I124-K) and marked the related plan frontier item as completed.

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 👎

Copy link
Copy Markdown

@augmentcode augmentcode Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. 1 suggestion posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

if (!existsSync(sliceDir)) continue;

for (const file of walkFiles(sliceDir)) {
const rel = relative(sliceDir, file);
Copy link
Copy Markdown

@augmentcode augmentcode Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 4 potential issues.

Fix All in Cursor

❌ 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 });
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 701c569. Configure here.

if (existsSync(epicSandboxDir)) {
rmSync(epicSandboxDir, { recursive: true, force: true });
}
mkdirSync(epicSandboxDir, { recursive: true });
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 701c569. Configure here.

Comment thread src/server/cli.ts
const { version } = JSON.parse(readFileSync(pkgPath, 'utf8')) as { version: string };
console.log(version);
process.exit(0);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 &lt;dir&gt; --version print the version and exit before cook runs, even though cook does not define that flag.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 701c569. Configure here.


const dest = join(epicSandboxDir, rel);
mkdirSync(join(dest, '..'), { recursive: true });
cpSync(file, dest, { dereference: false });
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 701c569. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant