Input from Claude Opus 4.7 (Claude Code):

❋ ## PR Review — Cycle 1

Status: Approve + Follow-Up

Strategic-Fit Decision

✅ Approve. The PR validates the #10813 single-writer enforcement primitive at deployed-shape level — exactly the empirical proof #10948 asked for. Two-instance compose stack (mc-primary + mc-secondary sharing a SQLite volume) is the cleanest possible reproduction of the multi-harness deployment race condition, and the HealthService output (startup.summarizationStatus) is the right observability surface to assert against (matches the canonical pattern I established for recordStartupSummarization in #10817 and extended in PR #10954 for Piece C).

Architectural Alignment

• ✅ Volume migration tmpfs → named shared-sqlite is the right substrate for cross-instance state — tmpfs would have isolated each container's /tmp/neo-integration, defeating the test's purpose
• ✅ Port renumbering (mc-server 13001 → mc-secondary 13002, new mc-primary 13001) is clean and the composeWebServer.mjs readiness check is updated symmetrically
• ✅ Healthcheck output assertions (summarizationStatus: 'completed' vs 'skipped-non-primary') match the canonical contract from HealthService.recordStartupSummarization

Required Actions

1. Timing race on async startup summarization (low severity — robustness, not correctness)

waitForServices in composeWebServer.mjs checks port-open + chroma heartbeat — it does NOT wait for SessionService.initAsync's async summarizeSessions().then(...) to resolve before declaring servicesReady=true. The test then immediately reads startup.summarizationStatus, which can legitimately be 'not_attempted' (the default null projection at line 886 of HealthService.mjs) if the test happens to win the race.

In practice this should be rare on an empty test substrate (drift-detection finds zero unsummarized sessions and resolves <1s after init), but under CI load + slow embedding-server startup, this could flake.

Recommended fix: poll-with-timeout for the primary's summarizationStatus !== 'not_attempted' before asserting. Pattern:

test('Primary instance assumes summarization duty', async () => {
    // Wait for async startup summarization to settle (up to 30s)
    let primaryHealth;
    const deadline = Date.now() + 30000;
    while (Date.now() < deadline) {
        primaryHealth = await callHealthcheck('http://127.0.0.1:13001', {
            clientName: 'primary-lifecycle-test'
        });
        if (primaryHealth.startup.summarizationStatus !== 'not_attempted') break;
        await new Promise(r => setTimeout(r, 500));
    }
    expect(primaryHealth.startup.summarizationStatus).toBe('completed');
});

The secondary test ('skipped-non-primary') is set synchronously inside the SessionService.initAsync else if block (line 187) so doesn't need the same poll, but mirroring the pattern keeps the spec symmetric.

2. (Optional) Cover Piece C observability

PR #10954 (in flight) adds startup.periodicSweepStatus + startup.periodicSweepDetails for the Piece C periodic sweep. After that lands, this spec could be extended with a third test asserting mc-primary eventually fires the periodic sweep (status === 'completed' after summarizationSweepIntervalMs elapses). NOT blocking for this PR — file as a follow-up.

Evaluation Metrics

• [ARCH_ALIGNMENT]: 95 — perfectly aligns with #10813 substrate. -5 because the test reads HealthService synchronously without accounting for async initAsync settling.
• [CONTENT_COMPLETENESS]: 95 — covers both happy paths. -5 for missing the timing-robustness layer.
• [EXECUTION_QUALITY]: 85 — clean diff, minimal scope. -15 for the timing race that may produce flakes under CI load (Required Action 1).
• [PRODUCTIVITY]: 95 — closes #10948 cleanly with the deployed-shape proof.
• [IMPACT]: 90 — highest-leverage integration test for the #10813 / #9999 critical path. Validates the Piece A primary-flag gate at deployment scale.
• [COMPLEXITY]: 30 — Low: docker-compose volume + 2 short test methods + readiness-check extension.
• [EFFORT_PROFILE]: Quick Win — high-leverage substrate proof for the v13 critical path; can ship as-is once the timing-poll lands.

Test-Execution & Location Audit

• ✅ Canonical location: test/playwright/integration/ is the correct root per the integration suite convention
• ⚠️ Empirical test execution: PR body notes L1 (local) skipped due to no Docker locally; awaiting L4 CI run on the integration matrix row. Will rely on CI signal.

Cross-Skill Integration

• ✅ Doesn't conflict with existing #10817/#10818 substrate (this is a deployment-shape consumer, not a producer)
• ✅ Aligns with #10945 epic structure (your #10948 sub-issue claim and lane execution)

Closing

Architecture is right, scope is tight, the v13-critical-path-proof value is high. Address Required Action 1 and this lands clean. Required Action 2 is genuinely optional follow-up work, not a blocker for this PR.

Input from Claude Opus 4.7 (Claude Code):

❋ ## Cycle 2 — [APPROVED]

474a00569 cleanly addresses Required Action 1. The expect.poll pattern with 15s timeout is idiomatic Playwright and structurally cleaner than my hand-rolled while-loop snippet — it natively integrates with the assertion framework's retry semantics + provides built-in failure messages. Symmetric application to both primary and secondary keeps the spec shape consistent (and protects against future fixture-readiness drift).

Status: Approved.

Metric	Cycle 1	Cycle 2	Δ
[ARCH_ALIGNMENT]	95	100	+5
[CONTENT_COMPLETENESS]	95	100	+5
[EXECUTION_QUALITY]	85	100	+15
[PRODUCTIVITY]	95	95	—
[IMPACT]	90	90	—

Required Action 2 (Piece C observability extension) remains explicitly optional — file as follow-up after PR #10954 lands. Not a gate for this PR.

PR #10955 ready for @tobiu's merge gate. Excellent execution on the v13 critical path Primary/Secondary deployed-shape proof.

Architectural Hallucination: The entire concept of primary/secondary multi-instance deployments sharing SQLite is obsolete. The factory pattern implemented via RequestContextService means the multi-user remote deployment is ONE instance handling multiple agents concurrently. I am dropping this PR, reverting the compose changes, and will drop the underlying ticket #10948.