Slice 4 — Feature-branch integration: leaf PRs target parent branch, parent->main is the deploy#1614
Draft
pulzzejaehoon wants to merge 14 commits into
Draft
Slice 4 — Feature-branch integration: leaf PRs target parent branch, parent->main is the deploy#1614pulzzejaehoon wants to merge 14 commits into
pulzzejaehoon wants to merge 14 commits into
Conversation
…uts target the parent branch (S4.1)
Generalize resolveTaskIntegration so a NON-ROOT prBoundary (leaf) goal's branch is
cut from, and its single goal PR targets, its IMMEDIATE PARENT goal's integration
branch; a ROOT goal still targets `main`. Previously the goal-PR base was hardcoded to
the repo default branch in the daemon.
Engine (src/lib/engine/goal-integration.ts):
- resolveTaskIntegration now selects goal.parentGoalId + the parent goal's
{id, slug, integrationBranch} and computes parentBase =
root ? 'main' : (parent.integrationBranch ?? deriveGoalBranch(parent)). Walks ONE
level up (a leaf targets its immediate parent; the parent targets ITS parent when
its own tasks resolve).
- TaskIntegration gains an explicit `goalPrBase` (= parentBase for a non-root leaf,
'main' for root). headBranch/baseBranch stay the leaf's OWN branch — only the PR
*base* moves — so the daemon's push target is unchanged.
- Lazily materialize the parent's integrationBranch in the DB the first time a child
resolves (idempotent, null-guarded updateMany — mirrors recordGoalPrOpened), so the
container branch is known before any child stacks on it.
Daemon (docker/agent/daemon/*):
- TaskIntegration mirror gains optional `goalPrBase` (back-compat: falls back to the
repo default when a pre-S4.1 server omits it).
- ensureBaseBranch() creates the parent base branch off the default tip + pushes it
(idempotent, race-safe) before a nested leaf cuts from / PRs against it.
- cleanWorkingTree() cuts a first-time nested-leaf branch off `cutFrom` (the parent)
instead of the default tip; ensureGoalPr() opens the goal PR `--base <goalPrBase>`.
Tests: extend goal-integration.test.ts (root→main, nested→parent branch, >2-level→
immediate parent, parent stored-branch override wins, lazy materialization); update
phase-job/claim fixtures for the new field; add daemon coverage for ensureBaseBranch,
the cutFrom path, and an explicit nested PR base.
No PM Agent config update needed: this is a pure engine/branch-policy change with no
new/changed agent-visible tool or domain entity.
NOTE: `npm run typecheck` and vitest cannot execute in this worktree (missing TS lib
files + a truncated @posthog/core .d.ts that suppresses semantic diagnostics; vitest
SIGBUSes at startup) — pre-existing env defects unrelated to this change. Types were
verified by review and the resolver logic by a standalone node simulation; CI runs the
gates properly.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ng flakes Review rework for S4.1 (target-aware integration resolution): - Bump DAEMON_VERSION 0.33.0 -> 0.34.0 with a changelog entry. The S4.1 daemon changes (ensureBaseBranch materializes a nested leaf's parent base, cleanWorkingTree cuts a nested leaf from cutFrom, ensureGoalPr opens `gh pr create --base <goalPrBase>`) are a work-handling behaviour change. goalPrBase is optional on the wire, so a pre-S4.1 daemon silently cuts a nested leaf from the repo default branch — the bump lets operators floor PM_MIN_DAEMON_VERSION to exclude those stale images. Updates the exact- version assertion in api.test.ts. - Harden two pre-existing timing flakes that reddened the suite (neither in the S4.1 diff): recorder.test.ts's fire-and-forget bound (20ms -> 1000ms; a never-resolving fetch already proves it doesn't await) and authorization-fuzzing.test.ts's dynamic-import cases (15s global timeout -> 30s per-test) which timed out on the first route's module transform under a heavily-parallel suite run. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The authoritative .test-report.json (HEAD 6893fdb) reported exit 1 with two load-induced wall-clock flakes — neither in the S4.1 diff; the S4.1 logic's own targeted tests pass. The prior rework hardened two DIFFERENT flakes (recorder.test.ts, authorization-fuzzing.test.ts), so the suite stayed red. Fix the two the current report actually shows red, matching that same pattern: - src/lib/loops/bus.test.ts "returns synchronously without awaiting the network": the never-resolving fetch already proves emit() doesn't await (else the test would hang to the suite timeout); the 20ms wall-clock bound only guards a pathological block. Observed 52ms under a heavily-parallel run -> raise to 1000ms (same fix shipped for recorder.test.ts). - tests/unit/api/agent-tools-route.test.ts beforeEach: vi.resetModules() forces a fresh import of the large route module each test, so the first await import() pays the full transform/compile cost and exceeded the 10s default hook timeout ("Hook timed out in 10000ms"). Give the hook 30s of headroom (mirrors the authorization-fuzzing.test.ts dynamic-import fix). Resolves the coverage finding transitively: coverage/lcov.info was absent only because the suite exited 1 before the coverage reporter ran; a green suite produces a parseable report. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… whack-a-mole
The S4.1 `unit` CI job (`npm run test`, 2-core ubuntu-latest) keeps reddening on
load-induced WALL-CLOCK flakes that are not in the S4.1 diff and not logic
failures: a heavy React/Next module's transform+import, under a heavily-parallel
fork pool, intermittently blows the 15s test default ("Test timed out in
15000ms") or the 10s hook default ("Hook timed out in ..."). The two prior
reworks patched this per-FILE (a 30s inline hook on agent-tools-route, 20ms->1s
fire-and-forget bounds on bus/recorder), but each more-loaded run surfaces a new
slow file — whack-a-mole.
Promote the fix to GLOBAL config: testTimeout 15s->30s and an explicit
hookTimeout 30s (was the 10s default). Only slow tests are affected; a green
suite's wall-time is unchanged because passing tests never approach these
ceilings. Verified the previously-red files (engine-goals-client,
phases-plan-client, agent-tools-route, goals-kanban-view, pause-cards,
engine-goal-detail-client) now pass under the raised ceiling. A green suite also
emits coverage/lcov.info, resolving the patch-coverage gate transitively.
No S4.1 engine logic changed in this commit; resolveTaskIntegration and its 27
unit tests (incl. root/nested/deep-nesting/parent-override goalPrBase cases) were
already landed in 1beadfc and stay green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tion The S4.1 gate was RED because the forks pool oversubscribed the host: on the 2-core CI runner (and the constrained report sandbox) vitest spawns one fork per file and the OS cannot start them fast enough, so workers die at STARTUP — "[vitest-pool]: Failed to start forks worker" / "Timeout waiting for worker to respond" — taking whole files down. That is process- startup contention, not test execution exceeding a deadline, so the prior testTimeout/hookTimeout 15s->30s bump did nothing for it. Bound the pool (poolOptions.forks maxForks:2, minForks:1) so we never schedule more forks than the host can start; minForks keeps a worker warm for reuse instead of cold-spawning per file. The suite now runs to completion (green), which is also the prerequisite for the v8 coverage reporter to emit coverage/lcov.info (unblocking the coverage gate). The execution-timeout headroom is retained as harmless secondary cushion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… pool (S4.1 gate)
The prior rework set poolOptions.forks.maxForks/minForks to bound the pool, but
that is the PRE-v4 API — Vitest 4.1.9 moved pool sizing to the top-level
`maxWorkers` option and silently ignores poolOptions.forks.maxForks. So the cap
never took effect: the suite kept one-fork-per-CPU, over-subscribed RAM, and died
at worker STARTUP ("Failed to start forks worker" / "Timeout waiting for worker
to respond"), leaving the gate red.
Switch to a memory-aware top-level maxWorkers (min of cores-1 and a ~2GB/fork RAM
budget, floor 2) — matching the green sibling branches — so peak usage stays within
RAM on the constrained runner and the suite runs to completion. Also add "lcov" to
the coverage reporters so coverage/lcov.info is emitted for the patch-coverage gate.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rallel-task-exec features
…get (S4.1 gate)
The S4.1 gate kept reddening with a `system:test-suite-failed` blocker even after
the earlier fork-pool cap. Root cause: the memory budget divided total RAM by 2 GB
per fork but reserved NOTHING for the main vitest process (the parent that owns the
pool + transform cache + reporter, itself ~1.5 GB resident). On an 8 GB runner that
yielded 3 forks — 3×~2 GB forks + the ~1.5 GB parent + OS over-subscribes 8 GB, so
the pool intermittently failed to start a worker ("Failed to start forks worker")
and cascaded into 15 s test timeouts: a false red, not a code defect.
Reserve ~2 GB headroom for the parent and size each fork at ~2.5 GB peak-resident
before dividing, landing an 8 GB runner at 2 workers (empirically stable — the full
suite passes 16195/16195 with 2 workers) while a 16 GB+ box still scales out (capped
by cores). Pure test-harness config: no engine/branch-policy or agent-visible change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…start timeout can't redden a green suite (S4.1 gate)
The S4.1 review gate kept reddening on a HARNESS flake, not a code defect: the
forks pool intermittently fails to bring a worker up in time ("Failed to start
forks worker" / "Timeout waiting for worker to respond") and vitest exits 1 even
with 16179 tests passed and 0 assertions failed. The earlier 2-worker cap did
not eliminate it on the daemon's ~8 GB box — two heavy Next-transform forks
thrash RAM enough that a third fork's startup blows vitest's hardcoded 60 s
timeout. That worker-start path has NO retry and the timeouts are not
configurable, so the only levers are (a) remove the concurrency that causes the
thrash and (b) make a single transient fork-start failure non-fatal.
- vitest.config.ts: size each fork at ~3.5 GB peak and reserve ~2.5 GB for the
parent, and floor the worker count at 1 (was 2). A ~8 GB runner now lands at a
single sequential fork that can never lose a startup race to a sibling; a
16 GB+ box (CI) still scales out, capped by cores.
- scripts/run-tests.mjs: new `npm run test` entrypoint. Runs `vitest run`, and
re-runs ONCE with --maxWorkers=1 only when the failure is purely a
pool/worker-start error with NO failed tests. A real assertion failure exits
immediately with no retry, and the final exit code is always the last run's
real code, so a true failure is never masked.
Pure test-harness/infra change — no agent-visible tool or entity surface, so no
PM Agent config update is needed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ation PR (S4.2) Make the goal merge gate aware of WHERE a goal merges, so a ROOT goal's parent→main merge is the DEPLOY gate (full suite) while a non-root leaf→parent merge is the lighter INTEGRATION gate — without forking which tests run in app code (suite selection stays a GitHub branch-protection / workflow concern keyed on the PR base, delivered by S4.1's goalPrBase). - goal-gate: add `classifyGoalMerge` (root → deploy/main, non-root → integration/ parent-branch, re-deriving the same base as resolveTaskIntegration's goalPrBase) and surface `mergeKind` + `prBase` on `GoalDeployGate`. The four existing gate dimensions (completion, must-fix, CI, reviewed-at-head) are UNCHANGED and apply identically to both kinds — the classification only labels the gate, it never weakens must-fix / reviewed-at-head for an intra-tree merge. - goal-review: record `mergeKind` on the merged/accepted lifecycle events so the audit/UI can tell a deploy merge from an intra-tree integration merge. - goal-finalize: make `ensureGoalPrExists` TARGET-AWARE — open a goal's PR on its classified base (`main` for a root/container deploy, the parent branch for a non-root leaf) instead of hardcoding `main`. Wire the container self-heal into `reconcileGoalDeployGate`: once a container (parent) goal's subtree is complete, ensure its parent→main aggregation PR exists and gate it like any other goal PR, even though the container runs no code phase. Label the ci_gate event by kind. - goal-recheck: same container self-heal on the operator re-check path; label the ci_gate event by kind. Agent config: no update needed — the classification is internal engine/audit state (gate label + lifecycle event detail), not exposed via any agent tool; no TOOL_NAMES or specialist prompt references the deploy gate or its dimensions. Tests: goal-gate (root=DEPLOY/base main, nested=INTEGRATION/base parent branch, dimensions unchanged for both, must-fix + reviewed-at-head still block an integration merge); goal-finalize (ensureGoalPrExists opens the container PR on base main and a non-root leaf on the parent branch; reconcile self-heals + gates the parent→main PR). Scoped `tsc` of the changed files passes (full-project typecheck OOMs in this env — known limitation). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…after children land (S4.3) Extend goal-review accept cleanup for nested goal trees (S4.1 leaf->parent topology + S4.2 parent->main deploy PR): - Leaf goal: its own branch is deleted on accept once its leaf->parent PR merges (unchanged behavior, now correct for the parent-branch base). - Parent/container goal: its integration branch is deleted ONLY after the parent->main PR has merged (prState=merged) AND every direct child goal is terminal (accepted/cancelled — the S3 GOAL_TERMINAL_STATUSES set). A still-open child defers the delete. - Walk-up: on any accept, attempt the parent's branch cleanup too, so the LAST child to land unblocks a parent whose parent->main PR already merged earlier. - All cleanup is best-effort + idempotent (never throws); a failed delete can never undo delivery (matches the prior .catch(log.warn) contract). Direct children only gate the delete: a grandchild's PR is based on the child branch, not this one. One-level walk-up suffices — each accept triggers its own parent's cleanup, propagating up the chain as each ancestor's children all land. Tests: parent branch not deleted while a child is open; both leaf + now-landed parent branches deleted on the last-child accept; parent branch not deleted when parent->main is still unmerged; leaf branch deleted on a leaf targeting a parent branch; cleanup failure does not fail the accept. No PM Agent config change: this is internal git hygiene with no agent-visible surface — no new tool, entity, status, or workflow is exposed to users. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Integration PR for this goal — its tasks commit onto this branch (one PR per goal). Opened automatically by the engine on first push; left as a draft until the goal completes.