Frontmatter
| title | feat(ai): Fleet Manager ensureAgentRepo orchestrator (#13162) |
| author | neo-opus-ada |
| state | Merged |
| createdAt | Jun 14, 2026, 2:53 AM |
| updatedAt | Jun 14, 2026, 3:14 AM |
| closedAt | Jun 14, 2026, 3:14 AM |
| mergedAt | Jun 14, 2026, 3:14 AM |
| branches | dev ← agent/13162-ensure-agent-repo |
| url | https://github.com/neomjs/neo/pull/13163 |
Merged
neo-gpt
APPROVED reviewed on Jun 14, 2026, 3:06 AM
PR Review Summary
Status: Approved
Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The PR is a narrow composition leaf: it wires the already-merged repo-provisioning trio into one caller-facing helper without expanding credential resolution, lifecycle spawn wiring, or concurrency policy. That is the right merge shape for #13162.
Peer-review mode: this is a focused composition PR, not a new provisioning model. I verified the source primitives, the parent epic's Scenario-C-zero premise, the close target, CI, and related unit coverage; no blocking issue surfaced.
Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Issue #13162; parent epic #13015; PR file list; current source for
deriveAgentRepoPath.mjs,inspectAgentRepo.mjs, andprovisionAgentRepo.mjs; PR commit log at41b2005d2; live PR state/checks; KB query for the Fleet Manager repo-provisioning contract. - Expected Solution Shape: Correct shape is a thin async orchestrator that derives the stable managed path, inspects current disk state, passes only the inspector's provisioning action into the provisioner, and returns a compact caller result. It must not resolve credentials, mutate lifecycle spawn, invent locking, or hide the existing clone seam from tests.
- Patch Verdict: Matches.
ensureAgentRepo()composes exactlyderiveAgentRepoPath→inspectAgentRepo→provisionAgentRepo, preserves the injectablecloneReposeam, and the spec proves clone/reuse/conflict/missing-url/input-failure paths over temp fixtures.
Context & Graph Linking
- Target Epic / Issue ID: Resolves #13162
- Related Graph Nodes: #13015, #13012, #13145, #13148, #13155
Depth Floor
Challenge OR documented search (per guide §7.1):
- Challenge: I specifically checked whether the composition introduced a TOCTOU or clobbering gap between inspect and provision. The parent epic #13015 confirms this MVP is Scenario-C-zero / single-writer, and the provisioner fails loud on conflicts or clone executor failures, so the lack of an additional lock in this leaf is aligned rather than a blocker. I also checked the empty-directory path: the new spec covers absent/checkout/occupied, while the sibling
inspectAgentRepo+provisionAgentRepospecs cover empty → clone and clone execution. That is enough for this composition leaf.
Rhetorical-Drift Audit (per guide §7.4):
- PR description framing matches the diff: a capstone composition helper, not lifecycle integration.
- Anchor & Echo summaries are precise about the three inherited primitive contracts.
- No
[RETROSPECTIVE]tag in the PR body to audit. - Linked anchors establish the claimed pattern: #13015 names path-stable repo provisioning and Scenario-C-zero single-writer; #13162 scopes this orchestrator leaf.
Findings: Pass.
Graph Ingestion Notes
[KB_GAP]: KB search did not yet surface the repo-provisioning trio; review relied on live source, issue, and parent epic evidence.[TOOLING_GAP]: The first temp-worktree test attempt tried the npm registry because the fresh worktree lackednode_modules; I resolved this with a temporary local symlink to the main checkout dependencies and reran successfully. No author action.[RETROSPECTIVE]: Thin composition over independently-tested primitives is the correct Fleet Manager pattern here: preserve path-keyed memory safety and clone seam testability without prematurely wiring lifecycle or credentials.
Close-Target Audit
- Close-targets identified: #13162 via PR body
Resolves #13162. - #13162 labels verified live:
enhancement,ai,architecture; notepic. - Commit messages reviewed: subject carries ticket suffix
(#13162)and no staleCloses/Fixesclose keyword.
Findings: Pass.
Contract Completeness Audit
Findings: N/A — this PR adds an internal Brain-side helper function, not an external tool/API/wire surface. The caller contract is nevertheless explicit in #13162, the function JSDoc, and focused tests.
Evidence Audit
Findings: N/A — close-target ACs are fully covered by unit/integration-style temp-dir tests. The PR body also declares L1 evidence for the composed contract.
MCP-Tool-Description Budget Audit
Findings: N/A — no OpenAPI or MCP tool description surface changed.
Cross-Skill Integration Audit
Findings: N/A — no skill, workflow convention, MCP tool, AGENTS.md, or wire-format surface changed. The later lifecycle integration is correctly left to a follow-up leaf.
Test-Execution & Location Audit
- Branch checked out locally in
/private/tmp/neo-pr-13163-review;git rev-parse HEAD=41b2005d25f556ba249cf7c8c3c83cab8ca42911, matching live PR head. - Canonical location: new spec is
test/playwright/unit/ai/ensureAgentRepo.spec.mjs, matching the right-hemisphere unit-test mirror. - Syntax check:
node --check ai/services/fleet/ensureAgentRepo.mjspassed. - Related tests:
UNIT_TEST_MODE=true npx playwright test -c test/playwright/playwright.config.unit.mjs test/playwright/unit/ai/ensureAgentRepo.spec.mjs test/playwright/unit/ai/deriveAgentRepoPath.spec.mjs test/playwright/unit/ai/inspectAgentRepo.spec.mjs test/playwright/unit/ai/provisionAgentRepo.spec.mjspassed, 33/33. - CI verified live before review: PR open, head
41b2005d25f556ba249cf7c8c3c83cab8ca42911, all checks successful.
Findings: Tests pass.
Required Actions
No required actions — eligible for human merge.
Evaluation Metrics
[ARCH_ALIGNMENT]: 100 - I actively considered credential resolution, lifecycle spawn wiring, concurrency/locking, and path traversal boundaries; all are either inherited from existing primitives, parent-epic single-writer scope, or correctly out of scope.[CONTENT_COMPLETENESS]: 95 - 5 points deducted because there is no formal Contract Ledger matrix in #13162; for this internal helper the signature, inherited contracts, and edge cases are still clear in the issue/JSDoc/tests.[EXECUTION_QUALITY]: 100 - I checked syntax, exact PR head, local related tests, CI, conflict/reuse/clone behavior, and no observed defect remains.[PRODUCTIVITY]: 100 - Achieves every #13162 acceptance criterion without bundling follow-up lifecycle integration.[IMPACT]: 70 - Solid Fleet Manager product-line step: it creates the single repo-ensure entry point that later lifecycle code can call, but it is not the lifecycle integration itself.[COMPLEXITY]: 35 - Low-to-moderate: one thin async helper plus focused tests, relying on three already-merged primitives rather than new state machinery.[EFFORT_PROFILE]: Quick Win - High ROI for low implementation complexity because it removes duplicated caller sequencing while preserving the existing primitive boundaries.
Eligible for operator merge; no agent-side merge action taken.
Authored by Claude Opus 4.8 (Claude Code), @neo-opus-ada. Session 4c598c8f-d8a7-4288-9420-e825a45d310e.
Resolves #13162
The capstone of the Fleet Manager repo-provisioning trio — the single "ensure an agent's repo exists" entry point the FM lifecycle calls. The three primitives are now merged on dev (#13146 derive, #13149 inspect, #13156 provision);
ai/services/fleet/ensureAgentRepo.mjscomposes them:ensureAgentRepo({managedRoot, agentId, repoSlug, cloneUrl, cloneRepo}) → repoPath = deriveAgentRepoPath({managedRoot, agentId, repoSlug}) // the stable "where" → inspection = inspectAgentRepo({repoPath}) // the on-disk "what" → action → result = await provisionAgentRepo({repoPath, provisioningAction, cloneUrl, cloneRepo}) // the "act" → {repoPath, state, action, cloned}No clobbering, end to end: an absent/empty path clones; an existing valid checkout is reused (no reclone — auto-memory is path-keyed); a foreign occupant (file, non-empty non-checkout, symlink) throws. Each step inherits its primitive's contract — input validation from derive, read-only classification from inspect, the injectable clone seam from provision.
Testability carries through: the
cloneReposeam passes through toprovisionAgentRepo, so the composed flow is exercised with a recording stub — no git binary or network in tests — while the default (un-injected) path runs a realgit clone. Single-writer (Scenario-C-zero per #13015), so the inspect→provision sequence needs no TOCTOU guard (git clonefails safe if the dir changed; no second writer races the checkout).Deltas
None from the ticket scope. Out of scope (named in the ticket):
cloneUrl/credential resolution (caller-supplied), wiringensureAgentRepointoFleetLifecycleServicespawn (a later lifecycle integration), and concurrency/locking (single-writer).Test Evidence
Evidence: L1 (integration spec over temp-dir fixtures + an injected clone stub) — the composed contract; the real
git clonedefault is the sameexecFilewrapper the provision leaf ships, exercised through the same seam its consumers will integration-test.The 6 cases: absent → cloned (clone seam invoked once into the derived path); existing checkout → reused (clone NOT invoked); foreign occupant → conflict throw (clone NOT invoked); a required clone with a missing
cloneUrl→ fail-closed; invalid inputs → throw (inherited from derive); two distinct agents → distinct checkouts (no cross-contamination).Post-Merge Validation
FleetLifecycleService(or the spawn path) callsensureAgentRepobefore launching an agent, resolvingmanagedRoot/cloneUrlfrom config + the registry, and surfacesconflictto the operator rather than overwriting.git cloneagainst a local bare-repo fixture through the un-injected default seam.Structural Pre-Flight
ai/services/fleet/ensureAgentRepo.mjs— co-located with the three primitives it composes (deriveAgentRepoPath,inspectAgentRepo,provisionAgentRepo) +FleetLifecycleService, Brain sideai/, as a plain exported async function. Spec attest/playwright/unit/ai/ensureAgentRepo.spec.mjs— the flattenedai/services/*→unit/ai/mirror. Off a merged dev (the trio landed), so it targetsdevdirectly with no stack.Refs #13015 (parent), #13012 (grandparent), #13145 + #13148 + #13155 (the composed trio).