#15122 and merged PR #15123 correctly rebalanced routine test execution: exact-head required CI owns normal unit/integration evidence, authors own existing non-CI coverage, and reviewers own named falsifiers. That change modified pr-review and pull-request, but did not modify the downstream unit-test skill.
The residual became concrete during the #16062 review. pr-review pointed to the unit-test reference for canonical placement; loading that skill then activated its author/executor workflow, including mandatory initialization and “Verify: Run the test.” The reviewer duplicated 96 focused tests already covered by the author and exact-head CI. The public follow-up records that the run “adds no independent evidence and is intentionally not counted”: https://github.com/neomjs/neo/pull/16062#pullrequestreview-4791366728
Live latest-open, semantic, local-history, and recent-A2A duplicate sweeps are required immediately before creation; the filing result is recorded below.
The Problem
The pr-review rule is correct at its own boundary but is not sticky across the skill link:
pr-review says not to discover or rerun routine unit/integration tests merely to duplicate green exact-head CI.
The same section links reviewers to .agents/skills/unit-test/references/unit-test.md when tests are added or moved.
unit-test has no review-mode boundary. Its trigger and payload lead directly into heavyweight authoring/execution initialization and an unconditional run step.
This is a cross-skill integration gap. A reviewer who follows both skills can obey each sentence locally and still waste context and execution time globally.
There is one necessary exception: hosted unit CI sets NEO_TEST_SKIP_CI=true. Green CI therefore does not establish behavior inside tests explicitly excluded by that flag, including environment-bound cases such as local-model coverage. Those tests need honest non-CI evidence, but that does not restore a blanket reviewer-rerun rule.
.agents/skills/unit-test/SKILL.md owns entry routing for test authoring, modification, and execution.
.agents/skills/unit-test/references/unit-test.md owns unit-test mechanics, patterns, placement, and commands.
.github/workflows/test.yml is the mechanical CI boundary: the unit suite runs with NEO_TEST_SKIP_CI=true.
Routine unit/integration execution belongs to exact-head required CI.
Exact-head non-CI receipts for NEO_TEST_SKIP_CI behavior belong to the author; reviewers validate/challenge them and run locally only for a named falsifier when their environment has the capability.
Progressive Disclosure outcome: tighten and compress the two existing skills. Do not add a skill, audit payload, static skip registry, or new .mjs classifier.
Structure-map gate: no new code placement is involved. The owning substrate is the existing pr-review and unit-test payloads.
The Fix
Tighten the unit-test skill description and entry routing:
do not trigger solely because a PR diff contains or adds unit tests, or because a reviewer checks canonical placement;
during a PR review, pr-review owns execution-evidence allocation.
Add a compact review-mode boundary before the unit-test payload's mandatory initialization:
exact-head green required CI owns routine unit/integration execution;
NEO_TEST_SKIP_CI coverage requires an exact-head author receipt;
reviewers validate that receipt and locally execute only a named falsifier or capability-specific challenge;
initialization, grounding, implementation, and “run the test” mandates apply when writing, modifying, fixing, or executing tests—not to review-only placement/idiom audits.
Tighten the pr-review cross-link so it invokes only unit-test placement/idiom guidance and does not enter the execution workflow merely because tests are present.
Replace redundant prose rather than accreting another rule. Keep the combined modified surface at or below the 44,006-byte filing baseline.
Contract Ledger Matrix
Target Surface
Source of Authority
Proposed Behavior
Fallback
Docs
Evidence
Routine unit/integration evidence in PR review
pr-review §7.5 + required exact-head CI
Do not rerun CI-equivalent tests
Named reviewer falsifier for a concrete unresolved behavior
Consume placement/idiom guidance without activating execution ceremony
Targeted falsifier remains available
Existing guide/reference
Cross-skill wording audit
Loaded substrate
ADR 0008 + substrate-accretion defense
Rewrite/compress existing payloads only
None
No new payload
wc -c before/after + skill lint
Acceptance Criteria
unit-test/SKILL.md explicitly does not trigger merely because a reviewer inspects a PR containing, adding, or moving unit tests.
The unit-test reference presents a review-mode boundary before mandatory initialization.
Review-only placement/idiom checks do not require Memory Core initialization, core/example grounding, or local test execution.
Routine exact-head green unit/integration CI remains sufficient execution evidence; neither skill tells reviewers to duplicate it.
Both skills name NEO_TEST_SKIP_CI as the mechanical exception to CI-owned evidence.
For NEO_TEST_SKIP_CI coverage, the author owns an exact-head non-CI receipt; the reviewer validates/challenges it and runs only a named falsifier when capability permits.
The pr-review link to unit-test guidance is scoped to placement/idioms and cannot be read as an instruction to enter the author/executor workflow.
Writing, modifying, fixing, or explicitly executing unit tests still activates the full unit-test workflow and canonical commands.
No new skill, audit payload, static list of guarded specs, .mjs classifier, or always-loaded AGENTS rule is added.
The combined pr-review guide + unit-test router/reference surface does not exceed the 44,006-byte filing baseline.
A representative review-only fixture or existing skill-lint assertion pins the reciprocal boundary without adding a new test file.
Decision Record impact
Aligned with ADR 0008: this refines routing between existing map/payload owners and reduces accidental payload activation. No new primitive or ADR amendment is needed.
Out of Scope
Changing which specs use NEO_TEST_SKIP_CI.
Moving CI-excluded tests into hosted CI.
Treating green CI as proof of correctness or approval.
Removing targeted reviewer falsifiers.
Weakening author-side non-CI evidence.
A broad rewrite of the unit-test skill.
Avoided Traps
Duplicate of #15122: rejected; this is its residual downstream-skill integration gap, evidenced after #15123 merged.
Blanket “never run tests locally”: rejected; named falsifiers and CI-excluded capability evidence remain.
Reviewer-owned rerun of every guarded spec: rejected; authors own the non-CI receipt, while reviewers validate/challenge it.
Static NEO_TEST_SKIP_CI inventory: rejected because the list drifts and mixes multiple exclusion reasons.
New review audit payload: rejected because the boundary belongs at the existing cross-skill seam.
Full unit-test initialization for placement review: rejected as the observed friction mechanism.
Context
#15122 and merged PR #15123 correctly rebalanced routine test execution: exact-head required CI owns normal unit/integration evidence, authors own existing non-CI coverage, and reviewers own named falsifiers. That change modified
pr-reviewandpull-request, but did not modify the downstreamunit-testskill.The residual became concrete during the #16062 review.
pr-reviewpointed to the unit-test reference for canonical placement; loading that skill then activated its author/executor workflow, including mandatory initialization and “Verify: Run the test.” The reviewer duplicated 96 focused tests already covered by the author and exact-head CI. The public follow-up records that the run “adds no independent evidence and is intentionally not counted”: https://github.com/neomjs/neo/pull/16062#pullrequestreview-4791366728Live latest-open, semantic, local-history, and recent-A2A duplicate sweeps are required immediately before creation; the filing result is recorded below.
The Problem
The
pr-reviewrule is correct at its own boundary but is not sticky across the skill link:pr-reviewsays not to discover or rerun routine unit/integration tests merely to duplicate green exact-head CI..agents/skills/unit-test/references/unit-test.mdwhen tests are added or moved.unit-testhas no review-mode boundary. Its trigger and payload lead directly into heavyweight authoring/execution initialization and an unconditional run step.This is a cross-skill integration gap. A reviewer who follows both skills can obey each sentence locally and still waste context and execution time globally.
There is one necessary exception: hosted unit CI sets
NEO_TEST_SKIP_CI=true. Green CI therefore does not establish behavior inside tests explicitly excluded by that flag, including environment-bound cases such as local-model coverage. Those tests need honest non-CI evidence, but that does not restore a blanket reviewer-rerun rule.The Architectural Reality
.agents/skills/pr-review/references/pr-review-guide.mdowns reviewer evidence allocation..agents/skills/unit-test/SKILL.mdowns entry routing for test authoring, modification, and execution..agents/skills/unit-test/references/unit-test.mdowns unit-test mechanics, patterns, placement, and commands..github/workflows/test.ymlis the mechanical CI boundary: the unit suite runs withNEO_TEST_SKIP_CI=true.NEO_TEST_SKIP_CIbehavior belong to the author; reviewers validate/challenge them and run locally only for a named falsifier when their environment has the capability.Progressive Disclosure outcome: tighten and compress the two existing skills. Do not add a skill, audit payload, static skip registry, or new
.mjsclassifier.Structure-map gate: no new code placement is involved. The owning substrate is the existing
pr-reviewandunit-testpayloads.The Fix
unit-testskill description and entry routing:pr-reviewowns execution-evidence allocation.NEO_TEST_SKIP_CIcoverage requires an exact-head author receipt;pr-reviewcross-link so it invokes only unit-test placement/idiom guidance and does not enter the execution workflow merely because tests are present.Contract Ledger Matrix
pr-review§7.5 + required exact-head CIpr-reviewguide.github/workflows/test.ymlNEO_TEST_SKIP_CIboundarypr-reviewandunit-testpayloadsunit-test/SKILL.mdpr-reviewremains authoritative during reviewpr-review§7.5wc -cbefore/after + skill lintAcceptance Criteria
unit-test/SKILL.mdexplicitly does not trigger merely because a reviewer inspects a PR containing, adding, or moving unit tests.NEO_TEST_SKIP_CIas the mechanical exception to CI-owned evidence.NEO_TEST_SKIP_CIcoverage, the author owns an exact-head non-CI receipt; the reviewer validates/challenges it and runs only a named falsifier when capability permits.pr-reviewlink to unit-test guidance is scoped to placement/idioms and cannot be read as an instruction to enter the author/executor workflow..mjsclassifier, or always-loaded AGENTS rule is added.pr-reviewguide +unit-testrouter/reference surface does not exceed the 44,006-byte filing baseline.node ai/scripts/lint/lint-skill-manifest.mjs --base origin/devpasses.Decision Record impact
Aligned with ADR 0008: this refines routing between existing map/payload owners and reduces accidental payload activation. No new primitive or ADR amendment is needed.
Out of Scope
NEO_TEST_SKIP_CI.Avoided Traps
NEO_TEST_SKIP_CIinventory: rejected because the list drifts and mixes multiple exclusion reasons.Related
Origin Session ID: 019fa530-53d6-7271-bf05-51497720b29c
Retrieval Hint: "PR review unit-test CI ownership NEO_TEST_SKIP_CI redundant local rerun #16062 #15122"