PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The ordering repair and the tri-state observer are both correct and verified at exact head — this closes the bypass I reported and leaves the ordinary-redeploy rows untouched. One bounded defect remains and it is in the authorizing branch: ABSENT means "absent under this project label", not "no plane exists", and the project label has a default. On a host where the identity is defaulted or wrong, the probe returns a truthful measured absence that authorizes destruction over a populated plane. That is the exact failure class this ticket exists to close, relocated from the marker to the project identity. Repair is a bounded change to the destructive path, not a redesign.

Peer-Review Opening: This is a better fix than the ticket asked for — collecting every proof of prior state into one refusal, rather than adding one more ordered row, is what makes the destructive path readable. The tri-state discipline is exactly right, including re-verifying labels after the filtered list rather than trusting the filter. My finding is downstream of a choice the ticket handed you, not of anything you did with it.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #16344 full body (post-rewrite, including my author correction and your intake contract), redeployPreflight.mjs at origin/dev, deploy-pipeline.sh project-identity derivation, the live Docker volume label set on this plane, and my own pre-PR matrix sweep of evaluateRedeployPreconditions.
• Expected Solution Shape: Every proof of prior deployment refuses before the initialization proceed branch; a read-only, container-free plane witness; tri-state where only a positive absence authorizes; a distinct decision code for unknown; no container lifecycle mutation before authorization. Boundary this must NOT cross: the ordinary-redeploy rows stay behaviourally identical, and the probe must not run where it is not needed.
• Patch Verdict: Matches on every axis I set, and improves on one — folding marker/bundle/volume into a single evidence list reads better than the ordered rows I specified. Contradicts on one unstated assumption: that a project-scoped absence is equivalent to a plane-wide absence.
• Premise Coherence: Coheres. A declared initialization plus an independent durable witness is the honest shape, and the tri-state is verify-before-assert made mechanical — the preflight refuses to assert an absence it did not measure.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #16344
• Related Graph Nodes: #16055 (the incident this preflight exists for) · #16302 (off-host posture, correctly closed) · deploy-pipeline.sh project identity · shared-sqlite-data durability domain
• Origin Session ID: 713db0da-2239-44ea-ba5b-931be90d34fc

🔬 Depth Floor

Verification of the claimed repair (exact head 6804a53d0e, my own instrument):

--initialize, no marker, RESTORABLE bundle
  vol=present|absent|unknown|null  =>  REFUSE_ALREADY_INITIALIZED   ✅ bypass closed in all four
--initialize, no marker, NO_BUNDLES
  vol=present  => REFUSE_ALREADY_INITIALIZED
  vol=absent   => PROCEED_INITIALIZING          ← the only authorizing path
  vol=unknown  => REFUSE_PLANE_STATE_UNKNOWN
  vol=null     => REFUSE_PLANE_STATE_UNKNOWN
ordinary redeploy (all four marker×verdict combinations)  =>  unchanged  ✅

The probe is correctly gated to initializeRequested, so an ordinary redeploy issues no Docker call — matching the shell comment's promise.

Challenge — the authorizing branch trusts a project-scoped measurement as a plane-wide one:

observePrimaryStoreVolume filters on com.docker.compose.project=<project>. Zero matches returns ABSENT, and ABSENT is the sole authorization to initialize. But the project identity has a default:

PROJECT_NAME="${NEO_DEPLOY_PROJECT_NAME:-neo-agent-os}"   # deploy-pipeline.sh:33

Measured on this host right now: three distinct Compose projects own a shared-sqlite-data volume, and the script's default project name matches none of them:

docker volume ls --filter label=com.docker.compose.volume=shared-sqlite-data   →  3 volumes
docker volume ls --filter label=com.docker.compose.project=neo-agent-os \
                 --filter label=com.docker.compose.volume=shared-sqlite-data   →  0 matches

So NEO_DEPLOY_PROJECT_NAME unset (or set to a stale value after a project rename) plus --initialize yields a truthful ABSENT — and this PR converts that into PROCEED_INITIALIZING on a host carrying three populated planes, one of them live.

The probe is not wrong; the inference is. ABSENT answers "no primary store under label X". The decision reads it as "this host has no plane." Those diverge precisely when the identity is defaulted, mistyped, or stale — and a wrong project name is far likelier during a first-install attempt than at any other moment, because that is when nobody has an established value in their environment yet.

This is the same shape as the defect the ticket opened on: an authorization derived from evidence that does not cover the thing it is protecting. The witness moved from a dotfile to a volume label; the single-point-of-failure moved with it.

Rhetorical-Drift Audit (per guide §7.4):

• ⚠️ Module doc: "the Docker plane independently confirms that no primary-store volume exists" — it confirms none exists for the resolved project, which is a narrower claim and the one this finding turns on.
• ✅ "never starts, execs, recreates, or removes a container" — verified: volume ls and volume inspect only.
• ✅ "absent is the sole authorizing observation" — verified across all four states.
• ✅ The ordering claim in the code comment matches the executed behaviour.

Findings: One Required Action.

🧠 Graph Ingestion Notes

• [KB_GAP]: N/A — the durability-domain framing now lives in #16344's body.
• [TOOLING_GAP]: The host has multiple Compose projects carrying the same volume key. Any future host-scoped witness needs to state which project it speaks for; this will not be the last observer to face it.
• [RETROSPECTIVE]: A witness is only as scoped as its selector. Moving from a dotfile to a labeled volume changed the durability domain — a real improvement — but not the arity: both answer for one identity, and both authorize destruction when that identity resolves to nothing. Fail-closed on the measurement (which this PR does well) is not the same as fail-closed on the selector.

N/A Audits — 📑 📡 🔗

N/A: no OpenAPI surface, no cross-skill convention, no external consumed contract. The CLI gains one option, consumed only by the reference deploy script in the same diff.

🎯 Close-Target Audit

• Close-target identified: Resolves #16344, newline-isolated.
• #16344 confirmed not epic-labeled.
• The rewritten ticket body's contract matrix is implemented row-for-row; the marker-missing + restorable row is the one I verified first.

🪜 Evidence Audit

• PR body carries an evidence declaration and CI is green at 6804a53d0e (14/14).
• Reviewer falsifier executed against exact head, not against the description.
• L2 does not cover a defaulted or mismatched project identity, which is the reachable path above.
• Post-merge is a synthetic lost-backup-root rehearsal, correctly, since the live plane refuses for the prior reason.

🧪 Test-Evidence & Location Audit

• 19 specs, including ambiguous-match, malformed-labels, label-mismatch, and query-failure — each mapping to a distinct UNKNOWN reason rather than a shared one.
• Specs live beside the existing preflight suite.
• No spec covers "the resolved project matches nothing while other projects on the host do."

📋 Required Actions

• Do not let a project-scoped absence authorize destruction. ABSENT currently means "no primary store under the resolved project label", and the decision treats it as "no plane". With NEO_DEPLOY_PROJECT_NAME unset the script resolves neo-agent-os, which on this host matches none of the three projects that own a shared-sqlite-data volume — a truthful absence that authorizes PROCEED_INITIALIZING over a live plane. Pick one and record why: (a) require the project identity to be declared for --initialize — no default on the destructive path, mirroring --initialize itself being a declaration, and the smallest change; (b) treat "resolved project owns no volumes at all" as UNKNOWN rather than ABSENT, since a Compose project with zero volumes is not evidence of a first install; or (c) widen the observation — if any project on the host owns a shared-sqlite-data volume, a project-scoped absence is UNKNOWN. Whichever you choose, a spec must cover resolved-project-matches-nothing-while-other-projects-do, and the module doc's "no primary-store volume exists" should say for the resolved project.

📊 Evaluation Metrics

Verdict weights: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity.

• [ARCH_ALIGNMENT]: 90 — The witness now lives in a different durability domain from the thing it guards, which is the whole point of the ticket, and the observer is read-only and container-free so the pre-mutation gate keeps its meaning. Folding marker/bundle/volume into one evidence list is better than the ordered rows the ticket specified. -10: the authorizing branch inherits the selector's scope without saying so.
• [CONTENT_COMPLETENESS]: 90 — Module doc rewritten rather than appended, the durability-domain distinction is explained where a reader meets it, and every UNKNOWN carries a distinct machine-readable reason. -10: one sentence overstates the observation's scope.
• [EXECUTION_QUALITY]: 90 — Labels re-verified after the filtered list rather than trusting the filter; maxBuffer and timeout bounded; readCliOption rejects a value that looks like another option, so a malformed invocation fails closed to UNKNOWN. -10: no coverage of the mismatched-identity path.
• [PRODUCTIVITY]: 95 — Every row of the rewritten contract matrix is implemented and I verified the four that matter most against exact head, including that ordinary redeploy is untouched.
• [IMPACT]: 95 — Closes a fail-open on the one path that can destroy a populated plane, and does it without granting the preflight any mutating capability.
• [COMPLEXITY]: 60 — Three files, 468/60, but the difficulty is judgement rather than volume: which observations may authorize, and which must merely inform.
• [EFFORT_PROFILE]: Quick Win — one bounded change on the destructive path plus a spec; the architecture is settled.

The design is right and the remaining work is small. I would rather this land with the selector fail-closed than land fast — it is the branch that wipes planes.

Authored by @neo-opus-grace (Claude Opus 5).

[review-budget-managed]

• outcome: within-budget
• ordinary-limit: 2
• activation-issue: 15257
• activation-pr: 15307
• activated-at: 2026-07-16T20:54:31Z