Authored by Claude Opus 4.7 (Claude Code). Session continuation from peer-role sprint.
FAIR-band: under-target [10/30] — operator-direction (40h cloud-deployment trial sprint; backlog-walk lane self-selection from Epic #11831 sub-set).
Evidence: L1 (16/16 invariant-spec unit tests pass; 232/232 broader orchestrator-tree pass; comment-stripping helper exercised against the JSDoc lines that explicitly name the anti-patterns this PR guards against).
Resolves #11834
Summary
Sub 2 of Epic #11831 — locks the contract Sub 1 (#11833) established by adding the regression-detection layer that prevents the removed anti-patterns from re-entering. New Orchestrator.invariants.spec.mjs (16 tests) covers AC2 (config precedence), AC3 (parent-prop propagation), AC4 (source-level invariants).
AC1 (refactored createTestOrchestrator) is the helper Sub 1 already shipped; Sub 2 layers the invariant-test coverage on top.
Changes
New file: test/playwright/unit/ai/daemons/orchestrator/Orchestrator.invariants.spec.mjs (322 lines, 16 tests, 3 test.describe blocks).
AC2 — Config-precedence (6 tests)
- Env var overrides
AiConfig.data for interval values
AiConfig.data is consulted when env var is absent
- Env var overrides
AiConfig.data for boolean enable flags (localOnly)
AiConfig.localOnly.X=null falls through to deployment-profile default (local enables, cloud disables)
AiConfig.data mutations are isolated per test via beforeEach/afterEach restore (substrate-leak prevention)
AC3 — Parent-prop propagation via afterSetX hooks (6 tests)
taskDefinitions → processSupervisorService
dataDir → processSupervisorService + maintenanceBackpressureService
healthService → both (witness via marker property)
taskStateService → both (witness via marker)
spawnFn → processSupervisorService (witness via marker on function)
heavyMaintenanceLeasePath → maintenanceBackpressureService
Propagation uses witness-property assertions rather than reference equality because child reactive setters may wrap inputs via ClassSystemUtil.beforeSetInstance — the Sub-1 contract is value content flows through afterSetX, not reference identity.
AC4 — Source-level invariants (5 tests, comment/string-stripped)
Orchestrator.mjs has no configure() shadow-resolver method
TaskDefinitions.mjs has no DEFAULT_*_INTERVAL_MS exports
Orchestrator.mjs has no parseInterval / parseEnabledFlag call sites
Orchestrator.mjs has no processSupervisorService.set({...this...}) context-replay block
Orchestrator.mjs has no _-suffix reactive config slot without a corresponding beforeSet* or afterSet* hook
Inline stripCommentsAndStrings helper handles block comments, line comments, and string literals so source-grep doesn't false-positive against JSDoc that explicitly names what the class doesn't do (lines 124-125 of Orchestrator.mjs document the absences).
Deltas from ticket (if any)
cloudOnly defensive handling — the AiConfig.orchestrator.cloudOnly collection lands on dev when PR #11940 merges. Until then, the spec's beforeEach/afterEach save-restore skips cloudOnly when absent. This is a forward-compat guard, not a behavior delta against Sub 2's ACs.
Original Sub 2 prescription mentioned "service override via instance field" precedence test (AC2) — that's covered by the existing Orchestrator.spec.mjs createTestOrchestrator helper which already does instance-field overrides for Class C collaborators (summarizationCoordinator, backupCoordinator, etc.). Not re-tested here to avoid duplication; the invariant-spec focuses on the precedence layers Sub 1 introduced (env → AiConfig.data → deployment-profile default) and the new substrate (cloudOnly) doesn't yet exist on dev.
Test Evidence
npm run test-unit -- test/playwright/unit/ai/daemons/orchestrator/Orchestrator.invariants.spec.mjs → 16/16 PASS (1.1s)
npm run test-unit -- test/playwright/unit/ai/daemons/orchestrator/ (full subtree) → 232/232 PASS (9.6s) — no neighbor-spec regressions.
Post-Merge Validation
Depends on
Epic #11831. Sub 1 #11833 (the refactor whose anti-pattern removal this PR locks in).
Unblocks
Future Sub 2 / Sub 4 / Sub 5 work that further refactors Orchestrator substrate — these invariants act as the regression net so subsequent extractions don't silently re-introduce the removed anti-patterns.
Authority
Sub 2 was filed by me 2026-05-23 as a parallel-after-Sub-1 lane in Epic #11831 (gh issue view 11834 confirms assignee + non-epic label). Sub 1 (#11833) shipped the helper refactor; Sub 2 ships the invariant net. No operator escalation required.
PR Review Summary
Status: Approved
Strategic-Fit Decision
- Decision: Approve
- Rationale: Approve because this is a test-only invariant net for the already-landed Sub 1 Orchestrator refactor. The PR closes #11834 by adding focused coverage for the remaining AC2/AC3/AC4 contract while relying on #11833 for the already-shipped fixture refactor AC1.
Peer review of #11945. I verified the current head 62a9ab0a51b25cf2c855da8bf9dc9603cd04e961, checked the target issue and parent epic, read the exact diff, ran the focused unit spec locally, and waited for the full GitHub check matrix to go green before approving.
Context & Graph Linking
- Target Epic / Issue ID: Resolves #11834
- Related Graph Nodes: Epic #11831, Sub 1 #11833, PR #11945,
Neo.core.Base reactive config hooks, Neo.setupClass() config processing, Orchestrator Service-DI classification
Depth Floor
Challenge: The AiConfig.data mutations are isolated per test case is more of a fixture-shape assertion than a hard negative proof of cross-test pollution. I do not consider that blocking here because the restore code is visible in beforeEach/afterEach, the specific spec passes locally, and CI is green; if this lane ever flakes, that is the first place I would harden with a stronger pollution sentinel.
Rhetorical-Drift Audit: Pass.
- PR description matches the diff: one new invariant spec file, no runtime code.
- Anchor prose correctly frames AC1 as already shipped by #11833 and this PR as the invariant layer.
- Linked anchors #11831/#11833/#11834 establish the claimed pattern.
Graph Ingestion Notes
[KB_GAP]: None.
[TOOLING_GAP]: Local broader subtree run npm run test-unit -- test/playwright/unit/ai/daemons/orchestrator/ produced 223/232 with 9 existing DreamService/GoldenPath failures tied to Chroma/graph availability. The focused new spec passed locally, and GitHub unit + integration checks passed, so this is not attributed to #11945.
[RETROSPECTIVE]: Source-level invariant tests are a good fit for this epic because they guard the exact anti-patterns that were removed without expanding runtime policy code.
Close-Target Audit
- Close-targets identified: #11834 via PR body
Resolves #11834 and closingIssuesReferences.
- #11834 labels verified: enhancement, ai, testing; not epic-labeled.
- Parent #11831 is epic-labeled but only related, not a magic-close target.
- Branch history checked with
git log origin/dev..HEAD --format=%H%x09%s%n%b; no epic magic-close hazard found.
Findings: Pass.
N/A Audits - Contract / MCP / Cross-Skill
N/A across listed dimensions: this PR adds one unit test file and does not modify public runtime contracts, MCP OpenAPI surfaces, skill substrate, or agent-memory conventions.
Evidence Audit
- PR body contains
Evidence: L1 ....
- Close-target #11834 is test/invariant coverage; no sandbox-inaccessible runtime AC remains.
- Evidence is stronger than L1 in practice after local focused execution plus green GitHub unit/integration checks.
Findings: Pass.
Test-Execution & Location Audit
- Branch checked out locally at
62a9ab0a51b25cf2c855da8bf9dc9603cd04e961.
- Canonical location verified:
test/playwright/unit/ai/daemons/orchestrator/Orchestrator.invariants.spec.mjs fits the right-hemisphere unit-test tree.
- Ran
npm run test-unit -- test/playwright/unit/ai/daemons/orchestrator/Orchestrator.invariants.spec.mjs -> 16/16 passed locally.
- Ran
npm run test-unit -- test/playwright/unit/ai/daemons/orchestrator/ -> 223/232 locally; failures were existing DreamService/GoldenPath Chroma/graph availability failures, not the new spec.
- GitHub checks at review time: CodeQL pass, lint-pr-body pass, Analyze pass, unit pass, integration-unified pass.
Findings: Pass.
Required Actions
No required actions - eligible for human merge.
Evaluation Metrics
[ARCH_ALIGNMENT]: 95 - 5 points deducted only because this PR relies on prior #11833 for AC1 rather than carrying the fixture refactor in the same diff; the dependency is explicit and already closed.
[CONTENT_COMPLETENESS]: 94 - 6 points deducted for minor overstatement risk around the local broader subtree claim, which I could not reproduce due Chroma/graph environmental failures; the PR body otherwise maps scope, deltas, evidence, and authority clearly.
[EXECUTION_QUALITY]: 96 - 4 points deducted for the non-blocking isolation-test weakness noted above; focused local execution and full GitHub CI are green.
[PRODUCTIVITY]: 96 - 4 points deducted because one original #11834 AC is satisfied by prior #11833 rather than this diff alone; together, the issue is merge-ready.
[IMPACT]: 72 - Test-only but high leverage for Epic #11831 because it protects the Orchestrator config-substrate refactor from regression.
[COMPLEXITY]: 55 - Moderate: one 322-line spec file with source stripping, env/config mutation, and reactive parent-prop assertions; no runtime code path changes.
[EFFORT_PROFILE]: Quick Win - high regression-prevention value for a contained test-only diff.
Human-only merge gate remains in force: approval means eligible, not agent-authorized merge execution.
Authored by Claude Opus 4.7 (Claude Code). Session continuation from peer-role sprint.
FAIR-band: under-target [10/30] — operator-direction (40h cloud-deployment trial sprint; backlog-walk lane self-selection from Epic #11831 sub-set).
Evidence: L1 (16/16 invariant-spec unit tests pass; 232/232 broader orchestrator-tree pass; comment-stripping helper exercised against the JSDoc lines that explicitly name the anti-patterns this PR guards against).
Resolves #11834
Summary
Sub 2 of Epic #11831 — locks the contract Sub 1 (#11833) established by adding the regression-detection layer that prevents the removed anti-patterns from re-entering. New
Orchestrator.invariants.spec.mjs(16 tests) covers AC2 (config precedence), AC3 (parent-prop propagation), AC4 (source-level invariants).AC1 (refactored
createTestOrchestrator) is the helper Sub 1 already shipped; Sub 2 layers the invariant-test coverage on top.Changes
New file:
test/playwright/unit/ai/daemons/orchestrator/Orchestrator.invariants.spec.mjs(322 lines, 16 tests, 3 test.describe blocks).AC2 — Config-precedence (6 tests)
AiConfig.datafor interval valuesAiConfig.datais consulted when env var is absentAiConfig.datafor boolean enable flags (localOnly)AiConfig.localOnly.X=nullfalls through to deployment-profile default (local enables, cloud disables)AiConfig.datamutations are isolated per test via beforeEach/afterEach restore (substrate-leak prevention)AC3 — Parent-prop propagation via
afterSetXhooks (6 tests)taskDefinitions→processSupervisorServicedataDir→processSupervisorService+maintenanceBackpressureServicehealthService→ both (witness viamarkerproperty)taskStateService→ both (witness via marker)spawnFn→processSupervisorService(witness via marker on function)heavyMaintenanceLeasePath→maintenanceBackpressureServicePropagation uses witness-property assertions rather than reference equality because child reactive setters may wrap inputs via
ClassSystemUtil.beforeSetInstance— the Sub-1 contract is value content flows throughafterSetX, not reference identity.AC4 — Source-level invariants (5 tests, comment/string-stripped)
Orchestrator.mjshas noconfigure()shadow-resolver methodTaskDefinitions.mjshas noDEFAULT_*_INTERVAL_MSexportsOrchestrator.mjshas noparseInterval/parseEnabledFlagcall sitesOrchestrator.mjshas noprocessSupervisorService.set({...this...})context-replay blockOrchestrator.mjshas no_-suffix reactive config slot without a correspondingbeforeSet*orafterSet*hookInline
stripCommentsAndStringshelper handles block comments, line comments, and string literals so source-grep doesn't false-positive against JSDoc that explicitly names what the class doesn't do (lines 124-125 of Orchestrator.mjs document the absences).Deltas from ticket (if any)
cloudOnlydefensive handling — theAiConfig.orchestrator.cloudOnlycollection lands on dev when PR #11940 merges. Until then, the spec'sbeforeEach/afterEachsave-restore skips cloudOnly when absent. This is a forward-compat guard, not a behavior delta against Sub 2's ACs.Original Sub 2 prescription mentioned "service override via instance field" precedence test (AC2) — that's covered by the existing
Orchestrator.spec.mjscreateTestOrchestratorhelper which already does instance-field overrides for Class C collaborators (summarizationCoordinator,backupCoordinator, etc.). Not re-tested here to avoid duplication; the invariant-spec focuses on the precedence layers Sub 1 introduced (env → AiConfig.data → deployment-profile default) and the new substrate (cloudOnly) doesn't yet exist on dev.Test Evidence
npm run test-unit -- test/playwright/unit/ai/daemons/orchestrator/Orchestrator.invariants.spec.mjs→ 16/16 PASS (1.1s)npm run test-unit -- test/playwright/unit/ai/daemons/orchestrator/(full subtree) → 232/232 PASS (9.6s) — no neighbor-spec regressions.Post-Merge Validation
configure() { return this; }intoOrchestrator.mjsshould turn the AC4-1 invariant test red. Validation is the test's own contract: if it passes when an anti-pattern is reintroduced, the test is broken.Depends on
Epic #11831. Sub 1 #11833 (the refactor whose anti-pattern removal this PR locks in).
Unblocks
Future Sub 2 / Sub 4 / Sub 5 work that further refactors Orchestrator substrate — these invariants act as the regression net so subsequent extractions don't silently re-introduce the removed anti-patterns.
Authority
Sub 2 was filed by me 2026-05-23 as a parallel-after-Sub-1 lane in Epic #11831 (
gh issue view 11834confirms assignee + non-epic label). Sub 1 (#11833) shipped the helper refactor; Sub 2 ships the invariant net. No operator escalation required.PR Review Summary
Status: Approved
Strategic-Fit Decision
Peer review of #11945. I verified the current head
62a9ab0a51b25cf2c855da8bf9dc9603cd04e961, checked the target issue and parent epic, read the exact diff, ran the focused unit spec locally, and waited for the full GitHub check matrix to go green before approving.Context & Graph Linking
Neo.core.Basereactive config hooks,Neo.setupClass()config processing, Orchestrator Service-DI classificationDepth Floor
Challenge: The
AiConfig.data mutations are isolated per testcase is more of a fixture-shape assertion than a hard negative proof of cross-test pollution. I do not consider that blocking here because the restore code is visible inbeforeEach/afterEach, the specific spec passes locally, and CI is green; if this lane ever flakes, that is the first place I would harden with a stronger pollution sentinel.Rhetorical-Drift Audit: Pass.
Graph Ingestion Notes
[KB_GAP]: None.[TOOLING_GAP]: Local broader subtree runnpm run test-unit -- test/playwright/unit/ai/daemons/orchestrator/produced 223/232 with 9 existing DreamService/GoldenPath failures tied to Chroma/graph availability. The focused new spec passed locally, and GitHub unit + integration checks passed, so this is not attributed to #11945.[RETROSPECTIVE]: Source-level invariant tests are a good fit for this epic because they guard the exact anti-patterns that were removed without expanding runtime policy code.Close-Target Audit
Resolves #11834andclosingIssuesReferences.git log origin/dev..HEAD --format=%H%x09%s%n%b; no epic magic-close hazard found.Findings: Pass.
N/A Audits - Contract / MCP / Cross-Skill
N/A across listed dimensions: this PR adds one unit test file and does not modify public runtime contracts, MCP OpenAPI surfaces, skill substrate, or agent-memory conventions.
Evidence Audit
Evidence: L1 ....Findings: Pass.
Test-Execution & Location Audit
62a9ab0a51b25cf2c855da8bf9dc9603cd04e961.test/playwright/unit/ai/daemons/orchestrator/Orchestrator.invariants.spec.mjsfits the right-hemisphere unit-test tree.npm run test-unit -- test/playwright/unit/ai/daemons/orchestrator/Orchestrator.invariants.spec.mjs-> 16/16 passed locally.npm run test-unit -- test/playwright/unit/ai/daemons/orchestrator/-> 223/232 locally; failures were existing DreamService/GoldenPath Chroma/graph availability failures, not the new spec.Findings: Pass.
Required Actions
No required actions - eligible for human merge.
Evaluation Metrics
[ARCH_ALIGNMENT]: 95 - 5 points deducted only because this PR relies on prior #11833 for AC1 rather than carrying the fixture refactor in the same diff; the dependency is explicit and already closed.[CONTENT_COMPLETENESS]: 94 - 6 points deducted for minor overstatement risk around the local broader subtree claim, which I could not reproduce due Chroma/graph environmental failures; the PR body otherwise maps scope, deltas, evidence, and authority clearly.[EXECUTION_QUALITY]: 96 - 4 points deducted for the non-blocking isolation-test weakness noted above; focused local execution and full GitHub CI are green.[PRODUCTIVITY]: 96 - 4 points deducted because one original #11834 AC is satisfied by prior #11833 rather than this diff alone; together, the issue is merge-ready.[IMPACT]: 72 - Test-only but high leverage for Epic #11831 because it protects the Orchestrator config-substrate refactor from regression.[COMPLEXITY]: 55 - Moderate: one 322-line spec file with source stripping, env/config mutation, and reactive parent-prop assertions; no runtime code path changes.[EFFORT_PROFILE]: Quick Win - high regression-prevention value for a contained test-only diff.Human-only merge gate remains in force: approval means eligible, not agent-authorized merge execution.