Skip to content

FE-883: Cook artifact lifecycle — git-merge slice composition#230

Closed
kostandinang wants to merge 8 commits into
ka/fe-881-cook-agent-skillsfrom
ka/fe-883-orchestrator-improvements
Closed

FE-883: Cook artifact lifecycle — git-merge slice composition#230
kostandinang wants to merge 8 commits into
ka/fe-881-cook-agent-skillsfrom
ka/fe-883-orchestrator-improvements

Conversation

@kostandinang

@kostandinang kostandinang commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Stack Context

Part of the FE-883 cook artifact-lifecycle work. Stacked on #227 (FE-881, cook agent skills); GC follow-up is #231.

What changed

Wires the already-built run-artifact.ts git-merge composer (previously dead code) into the live brownfield cook path, replacing the file-copy union that silently resolved overlapping edits as last-slice-wins.

  • Composer is correct under dependency-seeding — slice commits record their dependency commits as parents, so a dep-seeded file folds as an edit, not an add/add false conflict.
  • Brownfield promotion calls harvestCookRun — a fold conflict is now a fatal run outcome, not a buried event field. The user's checkout is never touched.
  • Brownfield verify-epic composes via the same fold in a detached worktree, so the tree verified equals the tree shipped; a real conflict fails the epic instead of verifying a partial tree. Greenfield keeps the file-copy union (no common ancestor for a 3-way merge).

Effect

Two slices editing different hunks of the same pre-existing file now both survive; a true conflict halts fail-closed instead of silently shipping last-writer-wins.

Verification

Full orchestrator suite green; check + build pass. (One client build-boundary test fails only in the dev worktree due to build-plugin resolution — it passes on base and in CI.)

Follow-ups

  • FE-883: Cook run worktree/branch GC #231 (stacked) — worktree/branch GC; retires the now-dead promoteBrownfieldRun.
  • Multi-slice end-to-end runCook test is the remaining coverage gap (merge logic is unit-tested; the 1-slice brownfield-smoke exercises the engine path). Tracked in memory/CARDS.md.

kostandinang commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

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.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@kostandinang kostandinang changed the title FE-883: fix run-artifact fold false-conflict on dependency-seeded files FE-883: Cook artifact lifecycle — git-merge slice composition Jun 18, 2026
@kostandinang kostandinang marked this pull request as ready for review June 18, 2026 10:37
@cursor

cursor Bot commented Jun 18, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes brownfield artifact composition and epic verification semantics; mitigated by fail-closed conflicts, plumbing-only git operations (I135-K), and expanded unit tests, but multi-slice end-to-end engine coverage is still a noted gap.

Overview
Brownfield cook now composes slice output with a git merge-tree fold instead of copying trees with last-slice-wins. The existing run-artifact.ts composer is hooked into promotion (harvestCookRun in cook-cli) and verify-epic (materializeEpicVerifyTree in net-compiler), so the tree verified matches the tree shipped. Greenfield still uses the file-copy union in mergeSlicesIntoEpicSandbox.

Slice commits now record dependency commits as parents (via commit-tree), fixing false add/add conflicts when a dependent slice extends dep-seeded files. commitSliceWorktree is idempotent so promotion reuses commits from an earlier verify pass. Real fold conflicts fail closed (CLI exit / epic passed:false) instead of being buried in merge events.

CLI finish output moves to cookFinishLines; presenter phase detection accepts ✓ cook → promoted. SPEC I124-K and planning docs describe the plan.mode composer fork. promoteBrownfieldRun is no longer used on the live brownfield path (removal deferred per CARDS 1d).

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

Comment thread src/orchestrator/src/run-artifact.ts
@kostandinang kostandinang force-pushed the ka/fe-883-orchestrator-improvements branch from b902c7a to 3890b32 Compare June 18, 2026 10:42
@kostandinang kostandinang changed the base branch from ka/fe-864-pi-timeout-600s to ka/fe-881-cook-agent-skills June 18, 2026 10:42
@kostandinang kostandinang force-pushed the ka/fe-881-cook-agent-skills branch from fbd12b2 to c830824 Compare June 18, 2026 11:43
@kostandinang kostandinang force-pushed the ka/fe-883-orchestrator-improvements branch from 3890b32 to 6b3aaba Compare June 18, 2026 11:43

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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 1 potential issue.

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 6b3aaba. Configure here.

Comment thread src/orchestrator/src/cook-cli.ts
@kostandinang kostandinang force-pushed the ka/fe-881-cook-agent-skills branch from c830824 to a811396 Compare June 18, 2026 11:46
@kostandinang kostandinang force-pushed the ka/fe-883-orchestrator-improvements branch from 6b3aaba to 4df8c49 Compare June 18, 2026 11:46
@kostandinang kostandinang requested a review from lunelson June 18, 2026 12:59
@kostandinang kostandinang self-assigned this Jun 18, 2026
@kostandinang kostandinang force-pushed the ka/fe-883-orchestrator-improvements branch from d9937b3 to c361f84 Compare June 18, 2026 13:07
kostandinang and others added 5 commits June 18, 2026 14:15
The git-merge run-artifact composer (871ef08) was left unwired pending "a
live-run check of the dependency-seed interaction". That check, encoded as a
test, fails: a dependent slice that extends a dep-seeded file (slice B has A's
output copied in by seedSliceSandboxFromDeps, then edits it) raises a spurious
merge-tree conflict, because every slice branch is rooted at the run base with
no inter-slice ancestry — so the dep-seeded file looks like an add/add.

Fix: commit each slice (in dependency order, which harvestCookRun already
uses) recording its completed-dependency commits as additional parents. The
fold's merge-base then becomes the dependency, so a dep-seeded file reads as an
edit the dependent slice evolved — clean fold, dependent's version wins —
while genuine sibling conflicts still surface fail-closed. commitSliceWorktree
switches from `git commit` to commit-tree + update-ref to carry the parents.

Also corrects the happy-path harvest test, which declared B as depending on A
but never seeded A's output into B — impossible under real dep-seeding; made
them the disjoint siblings the test name claims.

Composer correctness only; wiring it into the live promotion/verify path
(replacing the file-copy union) is the remaining Slice 1 work.

🍳 Built with brunch

Co-Authored-By: Opus 4.8 <noreply@anthropic.com>
Re-record the scope against the true FE-864 seam: the git-merge composer
(run-artifact.ts, 871ef08) already exists, so FE-883 is wiring it in, not
building it. CARDS.md tracks Slice 1 sub-steps (1a done; 1b wire, 1c verify-epic
consistency design fork, 1d delete file-copy path) + Slice 2 GC. PLAN.md gets
the frontier definition + a Parallel/Low-conflict sequencing entry.

🍳 Built with brunch

Co-Authored-By: Opus 4.8 <noreply@anthropic.com>
The headline correctness win over the file-copy union, pinned: two independent
slices editing different lines of the same pre-existing file both survive the
merge-tree fold, where a whole-file last-slice-wins copy would drop one.

🍳 Built with brunch

Co-Authored-By: Opus 4.8 <noreply@anthropic.com>
…mechanism)

1c decision: verify-epic must run tests against the same merged tree promotion
ships, not a file-copy union that diverges on same-file edits. This builds the
load-bearing mechanism the unknown was flagged on (checking a merge-tree fold
out as a worktree):

- Factor foldToCommit out of foldSliceBranches (fold N slice commits onto a base,
  fail-closed, write no refs) so callers can either publish (promotion) or check
  out (verify).
- materializeFoldedWorktree: fold onto a base + `git worktree add --detach` the
  result at destDir, rework-safe (removes any prior worktree first). Caller
  relinks shareable gitignored entries (node_modules) since the fold tree carries
  only tracked content.

Test proves different-hunk edits to one file both land on disk in the checked-out
verify worktree (the file-copy union would drop one) and that re-materializing is
idempotent. Mechanism only; the net-compiler verify-epic + cook-cli promotion
wiring is the remaining 1b/1c integration.

🍳 Built with brunch

Co-Authored-By: Opus 4.8 <noreply@anthropic.com>
… plan

🍳 Built with brunch

Co-Authored-By: Opus 4.8 <noreply@anthropic.com>
kostandinang and others added 3 commits June 18, 2026 14:15
Brownfield cook now composes by the merge-tree fold end-to-end, replacing the
file-copy union that silently last-slice-wins on same-file edits:

- net-compiler verify-epic: brownfield builds __epic__/<epicId>/ via
  materializeEpicVerifyTree (commit slices in dep order → fold → checkout a
  detached worktree → relink node_modules), so verify-epic tests the SAME tree
  promotion ships (1c). A real fold conflict fails the epic (emits a passed:false
  epic-verified report → fail sibling) instead of verifying a partial tree.
  Greenfield keeps the file-copy union (no common ancestor; spike decision).
- cook-cli promotion: brownfield calls harvestCookRun instead of
  promotionSourceDir + promoteBrownfieldRun; fold conflicts are a fatal run
  outcome (recordCookExitStatus(false)), not a buried event field. I135-K holds
  (all plumbing; the user's checkout is never touched).

Shared plumbing: commitSlicesInDependencyOrder factored out of harvestCookRun and
reused by verify; commitSliceWorktree made idempotent so the promotion pass reuses
the commits a prior verify pass made (else add -A finds nothing and drops every
slice). Exported resolveEpicSandboxDir + SHAREABLE_TOP_LEVEL_ENTRIES.

Tests: verify→promote reuse, different-hunk 3-way, fold materialization on disk.
Full orchestrator suite green (672). Remaining: 1d delete now-unused
promoteBrownfieldRun + the stale epic-sandbox-merge.ts TODO; SPEC I124-K amendment.

🍳 Built with brunch

Co-Authored-By: Opus 4.8 <noreply@anthropic.com>
- epic-sandbox-merge.ts: replace the now-resolved cook-artifact-lifecycle TODO
  with a note that brownfield folds (run-artifact.ts, wired) while
  mergeSlicesIntoEpicSandbox is the greenfield composer by design.
- SPEC I124-K: amended to record the plan.mode fork — brownfield git merge-tree
  fold (verify == ship, fail-closed) vs greenfield file-copy union.
- CARDS.md: 1b/1c done; 1d (retire dead promoteBrownfieldRun, blocked on the
  landCookBranch fixture rewrite) + a multi-slice end-to-end engine test remain.

🍳 Built with brunch

Co-Authored-By: Opus 4.8 <noreply@anthropic.com>
When a run promotes, replace the ad-hoc `✓ promoted → …` one-liner with a
clean finish block: the navigable directory, the branch + short commit, and
copy-paste `next` commands tailored to how the run landed (greenfield cd-in,
brownfield merge-when-ready, --land outcomes). Centralized as a pure,
golden-tested `cookFinishLines` builder alongside the other cook-report
strings; halt/conflict/error paths keep their existing inline lines.

The brigade phase tracker keyed on the literal `✓ promoted` prefix to light
plate; broaden it to match the `promoted` token after the ✓ so the new
`✓ cook → promoted` phrasing still advances the footer.
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