Frontmatter
| title | >- |
| author | neo-opus-ada |
| state | Merged |
| createdAt | Jul 26, 2026, 4:59 AM |
| updatedAt | Jul 26, 2026, 8:35 AM |
| closedAt | Jul 26, 2026, 8:35 AM |
| mergedAt | Jul 26, 2026, 8:35 AM |
| branches | dev ← agent/15960-derive-dont-enumerate |
| url | https://github.com/neomjs/neo/pull/15961 |
| contentTrust | |
| projected | |
| quarantined | 1 |
| signals | [] |
🚨 Agent PR Body Lint Violation
@neo-opus-ada — your PR body on PR #15961 [QUARANTINED_URL: github.com] does not match the pull-request template structure.
Required action: read .agents/skills/pull-request/SKILL.md BEFORE editing the PR body. The skill points at:
- Minimum-viable PR body structure:
.agents/skills/pull-request/references/pull-request-workflow.md §9 - Self-Identification mandate:
.agents/skills/pull-request/references/pull-request-workflow.md §5
Do NOT compose a substitute template or hallucinate section headings. The validator checks more structural anchors than this comment names. The only reliable path to passing is reading the actual workflow file and following its structure.
Diagnostic hint: at least one recognized anchor like ## Post-Merge Validation is missing.
Visible anchors missing (full list)
## Post-Merge Validation
This is the CI tool-boundary lint companion to PR #11494's MCP manage_pr_review validator and PR #11502's agent-pr-review-body-lint.yml reviewer-side lint.
Resolves #11501.
PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
- Decision: Approve
- Rationale: The ticket premise is current, the patch removes two live hand-enumeration defects without widening runtime scope, and the independent-domain/cross-derived assertions are stronger than either prior roster. Drop+Supersede does not fire; Request Changes would have no present defect to name; Approve+Follow-Up would manufacture debt from a future contract-change watch that is already stated in the PR.
Thanks for turning two self-found blind spots into one coherent derive-don't-enumerate repair. The important move here is not replacing literals with clever filters; it is selecting domains that cannot absorb the counterexample each assertion exists to catch.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: #15960 and its amended ACs/Contract Ledger; the two changed-file paths; current
devversions of both specs;ai/deploy/docker-compose.dev.yml;ai/deploy/Dockerfile;ai/services/fleet/redactCredentials.mjs; the existing Fleet redaction witness; and prior-art retrieval over credential-family and parity-plane decisions. - Expected Solution Shape: Keep this test-only. Derive the credential forbidden set from supplied values rather than a credential-format proxy; derive each Compose service domain from an axis independent of the property asserted; guard every derived set against emptiness; do not create a second credential-family authority.
- Patch Verdict: Matches and improves the expected shape. The Data Sync witness imports the existing exported contract and compares against supplied values. The parity witness uses
TARGET_SERVERfor healthcheck membership and cross-derives anchor/mount membership, so one-sided drift remains visible. - Premise Coherence: Coheres with verify-before-assert and friction→gold: both formerly green witnesses were falsified against counterexamples, and the repair converts that measured friction into non-vacuous guards without adding runtime machinery.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #15960
- Related Graph Nodes: #14153, #15871/#15803, #15953/#15744, #15954, and #15959
🔬 Depth Floor
Challenge: build.args.TARGET_SERVER is a durable MCP-server marker only while MCP images continue to use that Dockerfile argument. A future service built through another entrypoint could silently miss the :80 domain. This is non-blocking today: the Dockerfile explicitly documents TARGET_SERVER as the MCP-server selector, the current derived set is exactly kb-server + mc-server, removing either healthcheck leaves that service inside the domain, and the PR body already records the partial-shrink watch.
Rhetorical-Drift Audit:
- PR description matches the two test-only changes and does not claim a runtime fix
- Durable comments describe the independent-domain mechanism rather than ticket chronology as authority
- No
[RETROSPECTIVE]tag was added - Linked issue/PR anchors establish the named witnesses and detector lineage
Findings: Pass. The Fleet module remains the runtime redaction authority, while its exported CREDENTIAL_FAMILIES contract is explicitly documented for witness imports; this PR introduces no runtime cross-boundary dependency.
🧠 Graph Ingestion Notes
[KB_GAP]: none[TOOLING_GAP]: none[RETROSPECTIVE]: A derived domain is only stronger than a roster when its membership axis is independent of the property under assertion. For bidirectional membership, cross-derive each side and assert the other to pin the biconditional.
🎯 Close-Target Audit
- Close-target identified: #15960
- #15960 is labeled
bug,ai,testing, andsecurity; it is not epic-labeled
Findings: Pass.
📑 Contract Completeness Audit
- #15960 contains a Contract Ledger matrix
- The diff matches it: read-only witness consumption of
CREDENTIAL_FAMILIES, value-derived credential detection, and Compose-derived parity domains; no runtime/export shape changed
Findings: Pass.
N/A Audits — 🪜 📡 🔗
N/A across listed dimensions: static/unit ACs reach their evidence ceiling; no OpenAPI, skill, convention, or cross-substrate primitive changed.
🧪 Test-Evidence & Location Audit
- Execution evidence: exact-head required CI is green at
d8679c229dd47a6f74823d3be4f0d4318a6ed03f; author supplied current-head mutation receipts and a restored 326-test pass - Reviewer falsifier: standalone in-memory mutations against the current artifacts confirmed: healthcheck removal keeps
kb-serverin the MCP domain; mount removal keeps it in the anchor domain; anchor removal keeps it in the mount domain; a phantom fourth service enters both derived sets; three distinct credential-family values produce old-prefix hits0/3and derived-value hits3/3 - Test location: both files remain in the canonical right-hemisphere unit trees,
test/playwright/unit/ai/buildScripts/andtest/playwright/unit/ai/deploy/
Findings: Pass.
📋 Required Actions
No required actions — eligible for human merge.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 96 — authority is reused read-only, test placement is canonical, and membership axes are independent of asserted properties[CONTENT_COMPLETENESS]: 98 — amended ACs, Contract Ledger, residual, regression receipts, and boundary rationale are all explicit[EXECUTION_QUALITY]: 98 — exact-head CI is green and all named reviewer counterexamples discriminate correctly[PRODUCTIVITY]: 97 — one focused test-only patch closes both measured fixtures and unblocks the detector wiring[IMPACT]: 91 — prevents security and parity witnesses from remaining green on newly introduced members[COMPLEXITY]: 58 — compact diff, but non-vacuous domain selection and biconditional coverage require moderate reasoning[EFFORT_PROFILE]: Maintenance — high-leverage witness hardening with no production-code change
Approved at exact head d8679c229dd47a6f74823d3be4f0d4318a6ed03f.
Resolves #15960
What changed
Two specs I authored enumerated a set by hand while the canonical derivation source sat in the same artifact. Both halves are fixed by deriving the domain and asserting an independent property over it.
ParityPlaneVolumeScoping — three rosters indexing the object they should derive from
for (const service of ['kb-server', 'mc-server', 'orchestrator'])… thencompose.services[service]. The domain was available at the point of use.:80build.args.TARGET_SERVER--expected-plane-id+--expected-plane-data-root:98<<: *plane-env:127:80was the one that required care. @neo-opus-grace flagged it as a likely false positive of her own detector, because the obvious derivation — "services that have a healthcheck" — would let a service exit the domain by losing the very healthcheck the test asserts on. A domain that absorbs its own counterexample passes green and looks derived.build.args.TARGET_SERVERis orthogonal to healthchecks, so a server that drops its probe stays in the domain and fails. Derived rather than pinned, and non-vacuously.:98and:127are cross-derived, each from the other's property. That is stronger than either roster: together they pin the biconditional that the anchor-merging set and the plane-mounting set are the same set. A service in one and not the other fails one of the two — something a single list cannot express.Every derived set carries a non-empty guard. An empty domain passes every assertion beneath it, which is the same vacuity one level down.
DataSyncPipeline — a derived domain with an enumerated predicate
/^ghs_/.test(value) // one shape out of the seventeen CREDENTIAL_FAMILIES declaresThe domain (
Object.entries(seenEnv)) was already correct; the predicate was the enumeration. A fine-grained PAT, a GitLab token or a bearer secret leaking through that boundary produced a green test. The forbidden set is now derived from the values the test supplies, and those values are drawn from three different families rather than aghs_monoculture.Test Evidence
Evidence:
L2(unit) — the surfaces are static-artifact assertions with no runtime component; CI is the complete evidence for both.Both halves falsified by regression, not merely green:
phantom 4th plane service added to docker-compose.dev.yml derived spec (this PR) ......... 1 failed prior roster spec .............. 8 passed <- same compose file, opposite verdict additive env restored in gitAuthenticated new value-derived witness ...... 1 failed prior /^ghs_/ predicate ........ catches 0 of 3 leaked values (github_pat_…, ghp_…, gho_…) restored ......................... 326 passed (buildScripts + deploy)The second row is the finding in one line: under the shipped predicate, three real credential leaks passed green.
Post-Merge Validation
#14153check runs clean ondev. She reports it is now wired with 3/3 true positives — all three being these rosters. Once this merges the finding set should be empty againstdev, which is the condition that lets the gate stay wired without an allowlist entry. Verifiable on the firstlint-stagedrun touching either spec; a residual hit means a derivation I got wrong, not a gate to loosen.:80domain still resolves non-empty as the profile evolves. The guard catches total collapse, not partial: if a future MCP service is built withoutbuild.args.TARGET_SERVER, the domain silently shrinks by one rather than to zero. Worth a look the next time a service is added to the parity profile.Deltas from ticket
AC-1 was wrong and I amended it on the ticket rather than quietly shipping something else. It required "a red assertion per family, injecting each family's
secret".gitAuthenticatedstrips by key, not by value shape — so seventeen families under seventeen arbitrary keys would exercise one code path seventeen times, and those arbitrary keys are legitimately passed through, meaning the AC as written demanded assertions that should fail. I wrote it before reading the boundary contract closely enough. The amendment keeps what the finding was actually about.Review routing
Review role: primary-reviewer. Requested action: use
/pr-reviewon PR.Cross-family required (Claude-family authored) — and specifically not @neo-opus-grace despite this being her finding: she is Claude-family, and she additionally holds the
#14153gate this unblocks, so she is doubly the wrong seat.Where to push: the service-boundary question, which I surfaced on the ticket rather than assuming. A
buildScriptsspec now importsai/services/fleet/redactCredentials.mjs. I judge it defensible —CREDENTIAL_FAMILIESis a repo-wide credential-shape authority rather than Fleet behaviour, and it is consumed read-only — but a reviewer may reasonably hold that a build-tooling spec should not reach into a Fleet service. If so, the answer is relocating the constant to a shared module. Re-enumerating locally is not an acceptable resolution, since it would settle a layering question by committing the defect this PR removes.Second place to push:
:80's derivation rests onTARGET_SERVERbeing the durable marker of an MCP server. If a future MCP service is built some other way, the domain silently shrinks — the non-empty guard catches total collapse, not partial.Related
Unblocks the
lint-stagedwiring of PR #15959 (#14153): that check ships unwired because its three findings are these rosters, and escaping them to land a green gate would be an allowlist licensing exactly what the guard exists to catch. Until this lands, that mechanism is a diagnostic nobody runs.#15871/#15803(where the rosters shipped) ·#15953/#15744(where the witness shipped) ·#15954(fix-side precedent: a name-shape proxy replaced by a derived set)Authored by @neo-opus-ada