[malibio]

Summary

• One shared delivery-gates rule (rationalizations with their answers, red flags, what counts as evidence) included by Implementing a Task, Reviewing a Task, Addressing Review Findings and Completing a Task.
• The procedures no longer restate the weakened-check rule in their own words; the rule holds it once.
• Both audiences read identical text (the rule names no tool or command), asserted by a unit test.
• Attribution to addyosmani/agent-skills (MIT, copyright 2025 Addy Osmani) on the four procedures; ADR-092 delivery table updated in nodespace-docs.

Testing

• nodespace-agent suite: 1143 passed.
• Prompt assembly snapshot regenerated: no golden changes (no golden holds these four procedures).
• eval:tasks and the skill-retrieval matrix: pending, noted on the issue.

🤖 Generated with Claude Code

[malibio]

Code Review Summary

Verdict: APPROVE with minor suggestions. Net positive: one shared delivery-gates fragment, included by all four procedures, with the weakened-check rule consolidated into it. Both forms are byte-identical (and a test pins that), so the local and external audiences read the same words.

Pending, not defects: eval:tasks and the skill-retrieval matrix have not been run (machine not calm). Issue #3796 makes "no regression" an acceptance criterion and names #3784 (longer procedures risk the local agent), so these must run before merge.

Acceptance criteria

• One shared fragment (rationalizations, red flags, evidence): met.
• Four procedures include it and stop restating the rule: met for implementing (rule removed from STEP 4 and STEP 6), reviewing (STEP 6 points at the gates), completing. Addressing STEP 3 still carries a one-line form, see below.
• Same text for both audiences: met (identical files, the_delivery_gates_read_the_same_for_both_audiences).
• Attribution names source, licence, copyright: met in all four procedures, and matches the ADR-092 rows in the docs repo.
• Snapshot regenerated, no golden diff: reported by the author.
• eval:tasks / matrix: not yet run.
• quality:fix: not shown.

Findings

Critical Issues

• None.

Suggested Improvements

• [Improvement] packages/agent/src/skill_rules.rs (tests): the new test was inserted between the doc comment of rule_ids_are_unique_across_every_kind ("An include marker names a rule by id alone...") and that function. The orphaned comment now attaches to the_delivery_gates_read_the_same_for_both_audiences, and rule_ids_are_unique_across_every_kind loses its doc. Move the new test above that comment.
• [Improvement] Fragment length against The local agent does not follow the task procedures (eval:tasks fails 4 of 5 scenarios) #3784. The fragment is about 330 words, added to four procedures, and it roughly doubles completing-a-task. Part of it restates steps the procedures already have: "tick as you go / no evidence is not met" (implementing STEP 5, completing's first paragraph), "a passing run with no failing run" and "'Done' is not evidence" (implementing STEP 5, now said twice in one procedure). The issue asked that the fragment not restate steps. Consider dropping the "I will fill in the evidence afterwards" bullet and the "passing run with no failing run" red flag from the shared text, or trimming implementing STEP 5's copy. The eval runs will say whether it matters.
• [Improvement] addressing-review-findings.md STEP 3 still says "A fix that weakens a check is not a fix." The issue asked for procedures to stop restating the rule. Either delete it (the fragment's last line covers it) or keep it deliberately as a pointer ("see the gates below").
• [Improvement] The fragment's "the failing run and then the passing run" evidence line does not fit the reviewing procedure (a reviewer reads the evidence, it does not produce a failing run) or the completing one. It reads fine in implementing and addressing. Acceptable for a single shared fragment, but a half-sentence such as "or the evidence the author left" would stop a reviewer trying to reproduce failing runs.

Nitpicks

• Nit: reviewing-a-task.md STEP 6 now says "weakens a check, in the sense of the gates below"; the fragment never defines "weaken" except by the one-liner at the end. Naming the cases (test, threshold, lint rule) once in the fragment, which it does, is enough; the earlier explicit list helped a small model recognise a "loosened threshold" in a diff. Consider keeping "removes or loosens a test, a threshold or a lint rule" in that step since it is the detection step.
• Nit: packages/agent/src/seeds/rules/agent/delivery-gates.md and skill-md/delivery-gates.md are identical copies; that matches the existing two-form layout, so no action.

[malibio]

Review posted above. Verdict: APPROVE with minor suggestions (eval:tasks and matrix still to run before merge).

[malibio]

Addressed: (1) new test moved above the orphaned doc comment's owner; (3) addressing STEP 3 leftover sentence deleted; (4) evidence line now covers a review (the evidence the implementer left); (2) dropped the duplicated 'passing run with no failing run' red flag. Skipped: dropping the 'fill in the evidence afterwards' bullet (it is the only place a finding with no decision is named; the eval runs decide), and restoring the explicit loosen-a-check wording in reviewing STEP 6 (the fragment now defines it once). Pending before merge: eval:tasks and skill-retrieval matrix runs.

🤖 Generated with Claude Code

[malibio]

Merge queue: #3805 was taken out of the queue — it conflicts with main in: packages/agent/src/seeds/skills/addressing-review-findings.md, packages/agent/src/seeds/skills/reviewing-a-task.md. Merge or rebase origin/main, resolve the conflicts, push, and re-run bun run merge 3805.