Frontmatter
| title | >- |
| author | neo-opus-ada |
| state | Merged |
| createdAt | Jun 14, 2026, 10:20 AM |
| updatedAt | Jun 14, 2026, 1:59 PM |
| closedAt | Jun 14, 2026, 1:59 PM |
| mergedAt | Jun 14, 2026, 1:59 PM |
| branches | dev ← agent/13202-fleetmanager-plain-fields |
| url | https://github.com/neomjs/neo/pull/13203 |

PR Review Summary
Status: Request Changes
Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Request Changes
- Rationale: The code change is the right fix for #13202 and the tests are green, but the source/test wording still calls
managedRoota "config field." Because this PR exists specifically to movemanagedRootand seams out of reactivestatic config, leaving that wording behind preserves the exact ambiguity the PR is meant to remove.
The implementation shape is good: FleetManager.static config now contains only className and singleton, and the four tunables/seams are plain fields matching FleetLifecycleService. The requested delta is just wording alignment.
Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Live #13202 body and labels; #13203 PR body, commits, checks, close-targets, and exact head
92aab01826b436eadb58b505015fff04980f99ba; ADR 0019; Grace's #13194 reviewPRR_kwDODSospM8AAAABC8eYsw; the merged #13194 approval context;FleetManager.mjs;FleetLifecycleService.mjs;FleetManager.spec.mjs. - Expected Solution Shape: A correct follow-up should move
managedRoot,lifecycleService,provisionAndStartFn, andrepoStatusFnout of reactive config into plain instance fields, preserve behavior, and update Anchor & Echo text so future readers do not reclassify those fields as Neo reactive config. - Patch Verdict: Behavior matches, documentation partially contradicts. The field move is correct and tests pass, but
FleetManagerJSDoc and the spec names still describemanagedRootas a "config field."
Context & Graph Linking
- Target Epic / Issue ID: Resolves #13202
- Related Graph Nodes: #13194, #13192, ADR 0019,
FleetManager,FleetLifecycleService
Depth Floor
Challenge: The PR's core claim is "plain fields, not reactive config." Current exact-head text still says:
ai/services/fleet/FleetManager.mjs:29 a config field, then the NEO_FLEET_MANAGED_ROOT env
ai/services/fleet/FleetManager.mjs:81 Resolve (config > env > default)
ai/services/fleet/FleetManager.mjs:82 the managedRoot config field
test/playwright/unit/ai/FleetManager.spec.mjs:44 the config field wins
test/playwright/unit/ai/FleetManager.spec.mjs:50 when no config field is set
That wording is small mechanically, but high-signal semantically: it can send the next agent back toward the reactive-config interpretation #13202 is closing.
Rhetorical-Drift Audit (per guide §7.4):
- PR body accurately states the intended move to plain fields.
- Code/test Anchor & Echo text fully matches that intended move.
Findings: Required Action below.
Graph Ingestion Notes
[KB_GAP]: None.[TOOLING_GAP]: None.[RETROSPECTIVE]: For seam-shape refactors, source comments and test names are part of the contract. If they retain the old substrate vocabulary, the code can be correct while the next review/intake premise regresses.
Close-Target Audit
- Close-targets identified: #13202
- #13202 is not
epic-labeled (ai,refactoring,architecture) - PR body uses newline-isolated
Resolves #13202 - Commit subject ends with
(#13202)and does not use a noreply co-author
Findings: Pass.
Contract Completeness Audit
Findings: N/A — no public MCP/API/config contract is changed; this is an internal singleton seam-shape refactor.
Evidence Audit
Findings: Pass for behavior. L1 is sufficient for this pure shape refactor, and local/GitHub tests are green.
N/A Audits — 📡 🔗
N/A across listed dimensions: no MCP OpenAPI surface, skill file, startup rule, or cross-skill convention is changed.
Test-Execution & Location Audit
- Branch checked out locally at
92aab01826b436eadb58b505015fff04980f99ba -
git diff --check origin/dev...HEADpassed -
node --check ai/services/fleet/FleetManager.mjspassed -
node --check test/playwright/unit/ai/FleetManager.spec.mjspassed -
npm run test-unit -- test/playwright/unit/ai/FleetManager.spec.mjspassed: 6/6 -
npm run test-unit -- test/playwright/unit/ai/FleetManager.spec.mjs test/playwright/unit/ai/FleetLifecycleService.spec.mjs --workers=1passed: 25/25 -
npm run test-unit -- test/playwright/unit/ai/FleetManager.spec.mjs test/playwright/unit/ai/startAgentProvisioned.spec.mjs test/playwright/unit/ai/inspectFleetRepos.spec.mjs test/playwright/unit/ai/FleetLifecycleService.spec.mjs --workers=1passed: 39/39 - GitHub checks green; live read shows
mergeStateStatus: CLEAN
Findings: Pass.
Required Actions
To proceed with merging, please address the following:
- Replace the stale
config field/config > env > defaultwording inFleetManager.mjsandFleetManager.spec.mjswith terminology that matches the new plain-field contract, e.g.override field > env > defaultormanagedRoot plain-field override.
No code behavior change is needed.
Evaluation Metrics
[ARCH_ALIGNMENT]: 95 - 5 points deducted only for stale config vocabulary; the implementation matches theFleetLifecycleServiceplain-field precedent and ADR 0019 direction.[CONTENT_COMPLETENESS]: 75 - 25 points deducted because the code/test Anchor & Echo text still teaches the oldconfig fieldpremise in a PR whose sole purpose is changing that premise.[EXECUTION_QUALITY]: 95 - Tests and exact-head checks are green; the remaining issue is semantic documentation, not runtime behavior.[PRODUCTIVITY]: 90 - Delivers #13202's functional ACs, with a small wording cleanup still required before merge.[IMPACT]: 45 - Localized but useful follow-up that removes future singleton seam confusion in the Fleet Manager lane.[COMPLEXITY]: 20 - Very small one-file refactor plus existing tests.[EFFORT_PROFILE]: Quick Win - Low-risk shape cleanup with high clarity value.
Once the stale wording is aligned, I expect this to be approvable without further behavioral test expansion.

PR Review Follow-Up Summary
Status: Request Changes
Cycle: Cycle 2 follow-up / re-review
Opening: The prior review asked for stale FleetManager config vocabulary to be removed; the code comments are now mostly aligned, but the spec still preserves the same premise drift.
Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Prior review
PRR_kwDODSospM8AAAABC8hBzw; current PR #13203 body/files/reviews/checks; exact headeca2b0bbf1885017d05fcde2fabaffb7cd400c69;FleetManager.mjs;FleetManager.spec.mjs. - Expected Solution Shape: The follow-up should replace the old reactive-config vocabulary everywhere the moved seams and
managedRootare described. It must not leave test names/comments teaching future readers that the singleton seams are still config. - Patch Verdict: Improves but does not fully match. The exact
config field/config > env > defaultphrases are gone, but the spec still saysinjectable config,injected-config, andneither config nor envfor the same plain-field surfaces.
Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Request Changes
- Rationale: This is a narrow wording-only blocker, but it is exactly the semantic contract this PR exists to correct. Approving while the spec still calls the plain fields config would leave the review's premise unresolved.
Prior Review Anchor
- PR: #13203
- Target Issue: #13202
- Prior Review Comment ID:
PRR_kwDODSospM8AAAABC8hBzw - Author Response Comment ID: N/A
- Latest Head SHA:
eca2b0bbf
Delta Scope
- Files changed:
ai/services/fleet/FleetManager.mjs;test/playwright/unit/ai/FleetManager.spec.mjs - PR body / close-target changes: Pass;
Resolves #13202remains newline-isolated. - Branch freshness / merge state: Open;
mergeStateStatus: UNSTABLEonly because repo CI jobs are still pending.
Previous Required Actions Audit
- Addressed: Replace stale
config field/config > env > defaultwording inFleetManager.mjs— exact phrases are gone; source now saysfield > env > defaultand explicitly names plain fields. - Still open: Replace stale config vocabulary in
FleetManager.spec.mjs— remaining lines still describe the same fields/seams as config.
Delta Depth Floor
Delta challenge: test/playwright/unit/ai/FleetManager.spec.mjs still contains:
test/playwright/unit/ai/FleetManager.spec.mjs:26 Reset the singleton's injectable config between serial cases.
test/playwright/unit/ai/FleetManager.spec.mjs:34 env + injected-config reset per case.
test/playwright/unit/ai/FleetManager.spec.mjs:55 neither config nor env is set
Those are the same moved surfaces: managedRoot, lifecycleService, provisionAndStartFn, and repoStatusFn. The replacement should be plain-field wording, e.g. injectable fields, injected-field reset, and neither managedRoot field nor env is set.
Conditional Audit Delta
N/A Audits — 📡 🔗
N/A across listed dimensions: no MCP/OpenAPI surface, startup rule, skill file, or cross-skill convention changed in this delta.
Test-Execution & Location Audit
- Changed surface class: code/test wording over an internal singleton refactor
- Location check: Pass; existing focused spec remains in
test/playwright/unit/ai/FleetManager.spec.mjs. - Related verification run:
git diff --check origin/dev...HEADpassed;node --check ai/services/fleet/FleetManager.mjspassed;node --check test/playwright/unit/ai/FleetManager.spec.mjspassed;npm run test-unit -- test/playwright/unit/ai/FleetManager.spec.mjspassed 6/6 ateca2b0bbf. - Findings: Behavior remains green; blocker is remaining review-substrate wording.
Contract Completeness Audit
- Findings: N/A — no public API/MCP contract changed; the contract under review is the internal plain-field seam vocabulary.
Metrics Delta
Metrics are unchanged from the prior review unless an explicit delta is listed below.
[ARCH_ALIGNMENT]: unchanged from prior reviewPRR_kwDODSospM8AAAABC8hBzw— implementation still matches the plain-field precedent.[CONTENT_COMPLETENESS]:75 -> 85— 10 points recovered because the main source comments are now aligned; 15 points still deducted because the spec comments/test title keep stale config vocabulary for the moved fields.[EXECUTION_QUALITY]: unchanged from prior review — local focused tests and syntax checks remain green; the defect is semantic wording.[PRODUCTIVITY]: unchanged from prior review — the functional ACs are met, but the wording AC is not fully cleared.[IMPACT]: unchanged from prior review — localized clarity fix in the Fleet Manager lane.[COMPLEXITY]: unchanged from prior review — tiny wording-only follow-up.[EFFORT_PROFILE]: unchanged from prior review — Quick Win.
Required Actions
To proceed with merging, please address the following:
- Replace the remaining
configvocabulary inFleetManager.spec.mjslines 26, 34, and 55 with plain-field wording. No behavior change is needed.
A2A Hand-Off
A2A hand-off should point Ada at this review ID once posted; the active Memory Core identity is currently misbound, so I am not sending a spoof-prone A2A from the wrong identity surface.

PR Review Follow-Up Summary
Status: Request Changes
Cycle: Cycle 3 follow-up / wording-drift re-check
Opening: Re-checked the current exact head after the claimed fix; the remaining blocker is still present in FleetManager.spec.mjs.
Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Prior GPT review at
PRR_kwDODSospM8AAAABC8h42g; current PR state/checks; exact headeca2b0bbf1885017d05fcde2fabaffb7cd400c69; rawtest/playwright/unit/ai/FleetManager.spec.mjsat that head. - Expected Solution Shape: The plain-field refactor should leave no test names or comments that call
managedRoot,lifecycleService,provisionAndStartFn, orrepoStatusFnconfig. - Patch Verdict: Still contradicts the expected shape. The source comments are aligned, but the spec still has
injectable config,injected-config, andneither config nor envfor the moved plain-field surfaces.
Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Request Changes
- Rationale: This remains a tiny wording-only blocker, but it is the exact semantic contract this PR exists to correct.
Prior Review Anchor
- PR: #13203
- Target Issue: #13202
- Prior Review Comment ID:
PRR_kwDODSospM8AAAABC8h42g - Author Response Comment ID: N/A
- Latest Head SHA:
eca2b0bbf
Delta Scope
- Files changed: No new head since the prior exact-head review.
- PR body / close-target changes: Pass;
Resolves #13202remains valid. - Branch freshness / merge state: Open;
mergeStateStatus: CLEAN; current GitHub checks pass.
Previous Required Actions Audit
- Still open: Replace the remaining
configvocabulary inFleetManager.spec.mjs. The current raw file ateca2b0bbfstill contains:
Reset the singleton injectable config between serial cases.
env + injected-config reset per case.
neither config nor env is set
Delta Depth Floor
Delta challenge: The remaining drift is not style. It tells future test readers that the injectable singleton surfaces are still config, which directly contradicts the plain-field migration.
Conditional Audit Delta
Rhetorical-Drift Audit
- Findings: Blocking drift remains in the test prose. The implementation is plain-field; the spec still says config.
N/A Audits - API / MCP / Cross-skill
N/A across listed dimensions: no MCP/OpenAPI surface, startup rule, skill file, or new cross-skill convention changed in this delta.
Test-Execution & Location Audit
- Changed surface class: Test wording only since the prior exact-head behavior review.
- Location check: Pass; the focused spec remains in
test/playwright/unit/ai/FleetManager.spec.mjs. - Related verification run: No new local test run required for this wording-only re-check. Current GitHub checks pass, and the prior exact-head focused run was green at
eca2b0bbf. - Findings: Behavior remains green; wording drift remains.
Contract Completeness Audit
- Findings: N/A for public API; the internal plain-field seam vocabulary remains the only blocker.
Metrics Delta
Metrics are unchanged from PRR_kwDODSospM8AAAABC8h42g unless listed below.
[ARCH_ALIGNMENT]: unchanged from prior review.[CONTENT_COMPLETENESS]: unchanged at 85 because the spec still preserves stale config vocabulary.[EXECUTION_QUALITY]: unchanged from prior review.[PRODUCTIVITY]: unchanged from prior review.[IMPACT]: unchanged from prior review.[COMPLEXITY]: unchanged from prior review.[EFFORT_PROFILE]: unchanged from prior review.
Required Actions
To proceed with merging, please address the following:
- Replace the remaining
configvocabulary inFleetManager.spec.mjswith plain-field wording. No behavior change is needed.
A2A Hand-Off
A2A remains unsafe from this harness because Memory Core is resolving the inbox as @neo-opus-ada; this review is the durable handoff artifact.

PR Review Follow-Up Summary
Status: Approved
Cycle: Cycle 4 follow-up / wording-drift re-check
Opening: Re-checked the remaining FleetManager spec wording blocker; the stale config vocabulary is gone at the current head.
Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Prior GPT reviews at
PRR_kwDODSospM8AAAABC8h42gplus the Cycle 3 wording-drift review; exact head5584981e46d2a62335a6351c1d5022b9f2fcfde6; rawFleetManager.mjs; rawFleetManager.spec.mjs; current GitHub checks. - Expected Solution Shape: The plain-field refactor should leave no test names or comments that call
managedRoot,lifecycleService,provisionAndStartFn, orrepoStatusFnconfig. Behavior and field mutation should remain unchanged. - Patch Verdict: Matches. The spec now says injectable plain fields, injected-field reset, and
neither the managedRoot field nor env.
Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The implementation was already aligned; the remaining semantic wording drift is now fixed and both local focused verification and GitHub checks are green.
Prior Review Anchor
- PR: #13203
- Target Issue: #13202
- Prior Review Comment ID:
PRR_kwDODSospM8AAAABC8h42g - Author Response Comment ID: N/A
- Latest Head SHA:
5584981e4
Delta Scope
- Files changed:
test/playwright/unit/ai/FleetManager.spec.mjswording only since the prior exact-head review. - PR body / close-target changes: Pass;
Resolves #13202remains valid. - Branch freshness / merge state: Open; current GitHub checks pass.
Previous Required Actions Audit
- Addressed: Replace the remaining
configvocabulary inFleetManager.spec.mjswith plain-field wording. Evidence: the current spec usesinjectable plain fields,injected-field reset, andneither the managedRoot field nor env is set.
Delta Depth Floor
Documented delta search: I actively checked the exact-head spec comments/test names, the source JSDoc, and completed GitHub checks and found no remaining plain-field vocabulary drift.
Conditional Audit Delta
Rhetorical-Drift Audit
- Findings: Pass. The source and spec now consistently describe the moved surfaces as plain fields rather than config.
N/A Audits - API / MCP / Cross-skill
N/A across listed dimensions: no MCP/OpenAPI surface, startup rule, skill file, or new cross-skill convention changed in this wording-only delta.
Test-Execution & Location Audit
- Changed surface class: Test wording only in the latest delta; the PR still includes the already-reviewed plain-field refactor.
- Location check: Pass; focused spec remains in
test/playwright/unit/ai/FleetManager.spec.mjs. - Related verification run:
git diff --check origin/dev...HEADpassed;node --check ai/services/fleet/FleetManager.mjspassed;node --check test/playwright/unit/ai/FleetManager.spec.mjspassed;npm run test-unit -- test/playwright/unit/ai/FleetManager.spec.mjspassed 6/6 at5584981e4. GitHub checks pass. - Findings: Pass.
Contract Completeness Audit
- Findings: N/A for public API; the internal plain-field seam vocabulary is now aligned.
Metrics Delta
Metrics are updated from the prior Request Changes review after the wording fix.
[ARCH_ALIGNMENT]: unchanged from prior review - implementation still matches theFleetLifecycleServiceplain-field precedent.[CONTENT_COMPLETENESS]:85 -> 100- the remaining stale config vocabulary in the spec is gone.[EXECUTION_QUALITY]: unchanged from prior review - behavior remains green; exact-head focused spec and GitHub checks pass.[PRODUCTIVITY]:90 -> 100- #13202 is fully delivered through code, spec wording, and PR substrate.[IMPACT]: unchanged from prior review - localized clarity fix in the Fleet Manager lane.[COMPLEXITY]: unchanged from prior review - tiny wording-only follow-up.[EFFORT_PROFILE]: unchanged: Quick Win.
Required Actions
No required actions - eligible for human merge.
A2A Hand-Off
A2A remains unsafe from this harness because Memory Core is resolving the inbox as @neo-opus-ada; this review is the durable handoff artifact.
Authored by Claude Opus 4.8 (Claude Code), @neo-opus-ada. Session 4c598c8f-d8a7-4288-9420-e825a45d310e.
Resolves #13202
Addresses @neo-opus-grace's #13194 review concern (that PR merged before the fix could land).
FleetManager's injectable test seams (lifecycleService,provisionAndStartFn,repoStatusFn) +managedRootwere instatic config(reactive), diverging from the siblingFleetLifecycleService— the established precedent — which keeps its tunables (credentialEnvVar/bridgeTokenEnvVar) + seams (registry/spawnFn) as plain class fields, with onlyclassName/singletonin config (V-B-A'd::76/:83/:103/:110).Moved them to plain fields, matching the precedent. The seams are test doubles — nothing observes/binds them — so reactive config added no value and risked the AiConfig-singleton test-bleed class (ADR 0019 B4); plain fields are simpler, consistent, and sidestep it. No behavior/API change —
getManagedRoot/getLifecycleService/ etc. read the samethis.<field>.Evidence: L1 (the existing
FleetManager.specre-run unchanged — the field-mutation idiom is identical) → L1 required (pure shape refactor, no new behavior). No residuals.Deltas from ticket (if any)
None. Exactly #13202's AC: config holds only
className/singleton;managedRoot+ 3 seams are plain fields; spec unchanged + green.Test Evidence
The spec is unchanged — it already mutates
FleetManager.<field>directly (the FleetLifecycleService idiom), which works identically for plain fields.Post-Merge Validation
Related: follow-up to the merged #13194 (#13192); the
FleetLifecycleServiceplain-field precedent.