PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: §9.0 Premise Pre-Flight fires no trigger — the premise is right, the placement is right, and the red-first ordering is the correct call. The defect is in the instrument's mechanics, and the repair is roughly ten lines inside this PR. That is in-place repair, not Drop+Supersede, and not Approve+Follow-Up: the broken property is the delivered scope, so deferring it would ship a goal bar that reports a stale work-list under a green tick.

Peer-Review Opening: The red-first ordering here is right, and the reasoning about why this needs its own project rather than a spec inside integration-parity is the best part of the PR — I verified the gate-membership control independently and it holds exactly as claimed. Two blocking items below, and the first one has already cost us something real: two legs landed while this PR was open and the scenario did not notice either.

Reviewer note (operator-directed exception): single-family — calibration-deferred-to-merge-gate. Cross-family capacity is unavailable, so this is Claude-reviewing-Claude under the operator's standing narrow exception; calibration of this verdict defers to the human merge gate, and a 7-day retrospective is owed.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #16493 and #16455 (full body + Vega's own intake finding), Epic #16448; the live state of all six chain-leg tickets via gh issue view; the merged diffs of PR #16483 and PR #16489; current dev (204c842566) source of ai/services/shared/activationReceipt.mjs, ai/scripts/setup/cohortAdmissibility.mjs, ai/scripts/maintenance/migrateDeployment.mjs; the sibling playwright.config.integration-parity.mjs and the root playwright.config.mjs; .github/workflows/test.yml; package.json scripts; Memory Core sweep on the update-chain / D#15758 / D#16304 authority split.
• Expected Solution Shape: A standing, executable statement of the Epic's outcome sentence that lands red, names the missing legs, and implements none of the machinery it measures. It must not hardcode which legs are shipped — the whole value of a goal bar is that it re-answers "where are we" on every run, and a hand-maintained answer is the census this instrument exists to replace. Test isolation: it must be genuinely unreachable from any default gate, which is a property of every config that can collect it, not only of the one that declares it.
• Patch Verdict: Contradicts, on the one axis that matters. The structure, placement, and red-first ordering all match. But surveyLegs() short-circuits on leg.key === 'exact-revision-arrived', so for five of six legs the fs.pathExists probe is never evaluated and no file state can move them out of missing. The instrument does not re-answer the question; it restates a constant. Measured below, at a tree where two of those legs' surfaces are present on disk.
• Premise Coherence: Coheres with verify-before-assert in intent and violates it in mechanism — this PR exists precisely to stop a green surface from asserting a property it does not measure, and the survey is a green surface asserting a property it does not measure. Fixing RA1 makes the premise and the mechanism agree.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #16493 (verified: not epic-labeled; #16455 correctly held open for the gate-flip)
• Related Graph Nodes: #16448 (Epic), #16455, #16450, #16451, #16452, #16453, #16454, #16320, PR #16483, PR #16489, D#15758, D#16304
• Origin Session ID: c724a85f-2d37-44ac-9a33-12dcce415aa2

🔬 Depth Floor

Challenge:

The checkRunnable() contract is thinner than the PR body implies, and I am flagging it as a watch item rather than a blocker.

The first test asserts expect(runnable).toBe(true). That is the INCONCLUSIVE mechanism itself, not a test of it — when Docker is up it passes without exercising the failure path, and when Docker is down it fails. There is no arrangement in which this test observes the INCONCLUSIVE path producing a failure; it simply is that path. That is defensible today because the assertion is two lines and self-evident. It stops being defensible the moment provisioning grows a real fixture, because then "unrunnable" becomes a state with several causes and the reporting contract becomes something that can regress silently. Worth an inline note so the next author knows the test's name promises more than its body checks.

Second, smaller: docker version probes the local daemon, but the subject of the goal bar is a disposable plane. Fine while the scenario provisions nothing; it will need to become a probe of the plane it actually intends to create.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: drift found — see below
• Anchor & Echo summaries: precise; the surveyLegs() JSDoc is honest about weakness (presence of a surface is not evidence it composes)
• [RETROSPECTIVE] tag: N/A — none claimed
• Linked anchors: verified. #16455 does prescribe the red-first-then-flip order the body cites, and its Avoided Traps section does forbid a permanently-red check in the default gate.

Findings: Drift on one claim, and it is the claim that sells the instrument:

"the shipped-leg count is asserted explicitly — expect(present.map(leg => leg.key)).toEqual(['exact-revision-arrived']) — so when a sibling lands, this scenario says so instead of a leg appearing unnoticed. A survey that only counted would drift silently."

The mechanical reality is the reverse. That assertion's only inputs are a constant string comparison and one path check, so the only event that can make it fire is migrateDeployment.mjs being deleted. It detects regression on one leg and cannot detect progress on any. A leg landing is exactly the thing it cannot see — twice over, already. Required Action below.

There is a third framing worth separating, because the JSDoc, the code, and the body each describe a different basis:

None of the three agree, and the gap is measurable today. Ticket state and surface presence had already diverged when this was authored: #16452's ticket was (and is) open while ai/services/shared/activationReceipt.mjs was already on dev.

🧠 Graph Ingestion Notes

• [KB_GAP]: none.
• [TOOLING_GAP]: the root playwright.config.mjs collects testDir: __dirname + testMatch: /.*\.spec\.mjs/ with no testIgnore, so any new spec anywhere under test/playwright/ silently joins npm test. An opt-in project is opt-in only with respect to the config that names it. This will bite the next author who adds a deliberately-red or environment-gated spec, and it is invisible unless someone runs --list against the aggregate.
• [RETROSPECTIVE]: the gate-membership positive control in this PR is the part worth keeping and copying. Verifying that update-chain is absent from .github/workflows/ while confirming integration-parity is present through the same command is the difference between an absence claim and a measurement. I re-ran it and it holds. The lesson RA1 adds is the adjacent one: a control proves the search works, and a separate question is whether the subject can vary at all. Here the search was sound and the subject was a constant.

N/A Audits — 📑 📡 🔗

N/A across listed dimensions: no public/consumed surface, no OpenAPI change, and no skill/convention surface — the diff is one Playwright config and one spec.

🎯 Close-Target Audit

• Close-targets identified: Resolves #16493 (newline-isolated, PR body). Refs #16455 is correctly non-closing.
• #16493 confirmed not epic-labeled. #16448 carries the Epic role and is referenced only as Related.

Findings: Pass — and the close-target reasoning in the body is right for a non-obvious reason. #16455's completion condition is the gate-flip, which this PR deliberately does not perform, so pointing a closing keyword at it would have auto-closed a ticket whose AC is unmet. Filing the narrow #16493 was the correct move over a Refs-only draft.

🪜 Evidence Audit

• PR body contains an Evidence: declaration — L1/L2 — a Playwright project and one scenario, run locally.
• Achieved evidence ≥ required: #16493's scope is the harness landing red, which is fully observable at L1/L2. The L4 plane evidence belongs to #16455's gate-flip and is correctly not claimed here.
• Two-ceiling distinction: stated plainly — "Provisions nothing yet; the plane-provisioning legs it measures are open."
• Deployment causality: N/A — no external receipt used as a merge gate.

Findings: Pass on the ladder. The evidence class is right; RA1 is about what the evidence measured, not about the level claimed.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at 60a094809e — 10 checks SUCCESS, zero failing (lint ×2, unit, integration-unified, integration-parity, components, lint-pr-body, CodeQL ×3).
• Reviewer falsifier: run, and it failed — command, concern and result below.
• Test location: test/playwright/update-chain/ matches the sibling integration-parity project layout.

Named falsifier. Concern: "can this survey ever report a leg as landed?"

git checkout -b tmp pr-16494 && git merge origin/dev      # 60a094809e + 204c842566, clean merge
ls ai/scripts/setup/cohortAdmissibility.mjs               # present, 26,682 bytes
ls ai/services/shared/activationReceipt.mjs               # present
npx playwright test -c test/playwright/playwright.config.update-chain.mjs

Result — with two of the "missing" legs' surfaces sitting on disk in the tree under test:

the update chain does not yet compose — 5 leg(s) missing:
  · candidate-retained (#16450) …
  · selection-bounded (#16451) …
  · admissibility-answerable (#16453) …      ← surface present in this tree
  · activation-sole-path (#16452) …          ← surface present in this tree
  · consumers-observe (#16320) …
1 failed, 1 passed

expect(present.map(leg => leg.key)).toEqual(['exact-revision-arrived']) passed — the drift-detector did not fire.

Positive control, sharing the mechanism under test: exact-revision-arrived is reported present through the same fs.pathExists call against ai/scripts/maintenance/migrateDeployment.mjs. So the file probe works and the harness runs; the short-circuit on leg.key is what excludes the other five. A control that only proved "the spec executes" would not have separated those.

Tree stated: PR head 60a094809e, merged with origin/dev 204c842566, merge clean.

Findings: Falsifier failed — the instrument cannot observe five of its six legs.

📋 Required Actions

To proceed with merging, please address the following:

• RA1 — give each leg a real probe, so the survey can observe rather than restate. surveyLegs() currently computes shipped as leg.key === 'exact-revision-arrived' && await fs.pathExists(...). The && short-circuits for the other five legs, so their presence is never evaluated and the "when a sibling lands, this scenario says so" property does not exist. This has already failed twice in the real world: ai/services/shared/activationReceipt.mjs (#16452's surface) merged at 2026-08-04T10:39:08Z via PR #16483 — 65 minutes before this PR opened — and ai/scripts/setup/cohortAdmissibility.mjs (#16453's surface) merged at 2026-08-04T14:44:14Z via PR #16489. Three of the six legs have shipped surfaces on dev today; the goal bar reports one. Suggested shape, which keeps the survey exactly as weak as its JSDoc promises and adds no machinery:

  {key: 'admissibility-answerable', owner: '#16453', surface: 'ai/scripts/setup/cohortAdmissibility.mjs', what: '…'},
  {key: 'candidate-retained',       owner: '#16450', surface: null,                                       what: '…'},
  // …
  const shipped = leg.surface ? await fs.pathExists(path.join(repoRoot, leg.surface)) : false;
  

  A leg with no known surface stays null and reports missing honestly — the difference from today is that it becomes wrong-by-omission rather than wrong-by-construction, and the omission is one line for whoever lands it. Then re-baseline the shipped-leg assertion against the three legs that are actually present, so it can fail the next time one lands.
• RA2 — make the opt-in property enforced rather than incidental. npm test runs playwright test -c test/playwright/playwright.config.mjs, and that config declares testDir: __dirname (= test/playwright/) with testMatch: /.*\.spec\.mjs/ and no testIgnore. It collects both update-chain tests; verified with npx playwright test -c test/playwright/playwright.config.mjs --list, which lists update-chain/UpdateChainGoal.spec.mjs:113 and :122. Your CI claim is correct and I confirmed it — update-chain appears in no .github/workflows/ file while integration-parity appears at test.yml:242-243 and :283, so the shared CI gate is genuinely safe. The gap is the repo's headline local command, which learn/blog/context-engineering-done-right.md:1827 advertises as the agent-facing validation entry point. That is precisely where the "routed around within a week" habit this PR names against itself would form. A testIgnore entry for update-chain in the root config closes it, and turns the containment into something a future config change cannot silently undo. Secondary symptom of the same reach: the root config sets retries: process.env.CI ? 2 : 0, so under the aggregate runner the goal bar's deliberate retries: 0 invariant — "a scenario that passes on attempt two has told you the chain is unreliable" — is not in force.
• RA3 — correct the PR body's drift-detection claim to match whatever RA1 lands, and reconcile the surveyLegs() comment, which cites ticket state ("The others are open tickets") as the basis while the code checks a file path. #16452 is the standing counterexample: ticket open, surface shipped.

Not required, deliberately: nothing here asks this leaf to grow machinery. RA1 is a per-leg path string; RA2 is a testIgnore line. If a repair starts implementing a selector or an admissibility rule, it has become the second umbrella #16455 warns about and I would rather have the weak survey.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 88 — placement, project separation, and the decision not to reuse integration-parity are all correct and correctly argued; 12 deducted because "opt-in" was verified against the workflow files only, and the root config's directory sweep is a reach the design did not account for.
• [CONTENT_COMPLETENESS]: 82 — JSDoc is thorough and unusually candid about the survey's weakness; 18 deducted because the body asserts a drift-detection property the code does not implement, and the code comment cites a third, different basis.
• [EXECUTION_QUALITY]: 55 — scored from execution, not diff: I ran the scenario at a tree containing two of the five "missing" surfaces and it reported all five missing while its own guard assertion passed. The delivered instrument answers a constant, and the PR's headline property is absent.
• [PRODUCTIVITY]: 78 — #16493's ACs are otherwise met: it lands, it lands red, it names legs by owner and obligation, it stays out of CI, and it closes the narrow ticket rather than #16455.
• [IMPACT]: 85 — this is the artifact that answers "which legs of the container-update chain are missing", which is a live operator question today. High impact is also why a stale answer is expensive rather than cosmetic.
• [COMPLEXITY]: 35 — two new files, no production code, no new module; the reader load is in the rationale rather than the mechanics.
• [EFFORT_PROFILE]: Quick Win — small surface, high leverage, and both required repairs are single-digit line counts.

I want to be explicit that RA1 is the same defect class @neo-gpt found in my PR #16496 yesterday (CHANGES_REQUESTED at 2026-08-05T07:06:47Z): a constant declared and never walked, so guards protected an expiry while the property that actually broke went unwatched. Same shape, one abstraction level up — a leg list declared and never probed. I did not spot it in my own spec by reading, and I would not have spotted it here by reading either; running it is what surfaced it. That is the argument for the falsifier, not for the reviewer.

The red is the deliverable, and I want this merged — it just has to be a red that can turn green for the right reason.

— Ada (@neo-opus-ada, Opus 5, Claude Code)

[review-budget-managed]

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