FEA-1346: Bound bootstrap preflight#232
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
- Move bootstrap into the launch-owned preflight path. - Bound bootstrap execution with typed outcomes and redacted diagnostics. - Cover timeout, event ordering, and additional-repo continuation behavior. Testing: Desktop typecheck, lint, and full test suite passed during finalization. Risks: Bootstrap timeout handling touches process lifecycle behavior and is covered by focused gateway/process tests.
peterulsteen
force-pushed
the
fix/fea-1162-pending-exit
branch
from
f291bd2 to
315828f
Compare
Compatibility Smoke Test Results
|
- Move bootstrap plugin registry fixture into shared symphony test utilities. - Reuse shared file and PID polling helpers across bootstrap tests. Testing: Focused bootstrap/gateway/multi-repo tests, desktop typecheck, and lint passed. Risks: Test-only refactor with no production behavior change.
| if ( | ||
| worktreeDir && | ||
| (body.command === LoopCommand.Plan || | ||
| body.command === LoopCommand.Execute || |
There was a problem hiding this comment.
This guard covers Plan/Execute/RequestChanges. On main, bootstrap also ran for GeneratePrd and RequestPrdChanges because it was embedded inside ensureWorktreeImpl (line 1637) and ensureLoopWorktreeMaterialized (line 1735), both of which setupPrdWorktree calls. Those functions no longer call runBootstrapIfNeeded, and this block skips PRD commands. Is the PRD bootstrap loss intentional? Probably fine since PRD generation doesn't use generated agents, but want to confirm it's not an oversight from extracting bootstrap out of the worktree helpers.
There was a problem hiding this comment.
Valid catch. This was an unintended regression from moving bootstrap ownership out of worktree materialization.
Addressed in f56a18b: GeneratePrd and RequestPrdChanges now use the launch-owned bootstrap preflight alongside Plan/Execute/RequestChanges, so PRD primary and additional repo worktrees keep the previous bootstrap behavior while still posting Started before bootstrap output.
I also added focused coverage in symphony-loop-multi-repo-spawn.test.ts for PRD commands with and without peers: 3 bootstrap-completed markers for primary + two peers, and 1 marker for primary-only.
Validation: focused bootstrap/gateway/multi-repo tests passed (38 tests), plus desktop typecheck and lint.
thadeusb
left a comment
thadeusb
left a comment
There was a problem hiding this comment.
Clean restructure. The process-group kill, redaction, and non-terminal outcomes all look right. One question about PRD commands below.
- Include GeneratePrd and RequestPrdChanges in launch-owned bootstrap preflight. - Cover PRD primary and additional repo bootstrap markers in multi-repo tests. Testing: Focused bootstrap/gateway/multi-repo tests, desktop typecheck, and lint passed. Risks: Restores prior PRD bootstrap behavior after moving bootstrap ownership out of worktree materialization.
3 participants
Summary
Validation
pnpm --dir apps/desktop exec tsx --test test/symphony-bootstrap.test.ts test/symphony-loop-cloud-failures.test.ts test/symphony-loop-multi-repo-spawn.test.tspnpm --dir apps/desktop run typecheckpnpm --dir apps/desktop run lintNote: the first sandboxed focused test attempt failed on tsx IPC pipe creation with
listen EPERM; the same command passed outside the sandbox.