fix(skills): record settled ADR decisions #98

Merged
jercik merged 13 commits from fix/project-docs-adr-acceptance into main 2026-10-03 22:06:15 +00:00
Owner

ADRs now record acceptance when the user settles a decision, even before implementation. Accepted and statusless records bind; proposed choices remain open. Conflicting plans use the same rule in documentation and grilling sessions.

Keep only current decisions, preserve useful rejected alternatives, repair deleted-record links, and never reuse ADR numbers. Every new or edited ADR receives a whole-record check against relevant implementation and governing decisions, including after mechanical edits. Implementation gaps are reported without rewriting settled intent.

The reviewer integration in j4k/review#94 and the skill pin in j4k/review-runner#25 must use the merged revision; repin the runner after this merges.

ADRs now record acceptance when the user settles a decision, even before implementation. Accepted and statusless records bind; proposed choices remain open. Conflicting plans use the same rule in documentation and grilling sessions. Keep only current decisions, preserve useful rejected alternatives, repair deleted-record links, and never reuse ADR numbers. Every new or edited ADR receives a whole-record check against relevant implementation and governing decisions, including after mechanical edits. Implementation gaps are reported without rewriting settled intent. The reviewer integration in [j4k/review#94](https://code.j4k.dev/j4k/review/pulls/94) and the skill pin in [j4k/review-runner#25](https://code.j4k.dev/j4k/review-runner/pulls/25) must use the merged revision; repin the runner after this merges.
fix(project-docs): accept ADRs with their implementation and keep the context map to relationships
All checks were successful
commit-msg / commitlint (pull_request) Successful in 25s
Node tests / node:test (pull_request) Successful in 2m5s
Review / Review (pull_request_target) Successful in 6m23s
0613134ded

Review 01M41SFZWKA8GGYFCW0ZY7PGEB — head bc0921e42bab9862a5108abda1377c10de421e99

Review — j4k-oss/agent-skills @ 2034125557

Scope: diff against base tree 2bb52ad1ca16
Status: dispatched — coverage complete (4/4 slots terminal)
Facts: current review-wide projection

Computed under:

{
  "abandonment": "abandonment-v1",
  "anchor_recipe": 1,
  "batch_policy": "batch-v1",
  "coverage": "coverage-v3",
  "dispatch_policy": "dispatch-v2",
  "grounder_version": 1,
  "grounding_read_rule": "grounding-read-v1",
  "promotion_policy": "promotion-v1",
  "report": "report-v3",
  "tally": "tally-v1",
  "triage_settle": "triage-settle-v2"
}

Findings (7)

high — ADR loading rule repeats the complete status value set

  • claim: 01M41SRZK1AA7QXAW0CH87CMZE
  • anchor: skills/project-docs/SKILL.md (snippet)
An ADR marked `accepted`, or without a status, is decided and binding; `proposed` means the decision is still open.
  • lens: restated-sets · arm: default
  • verdicts: 1 valid / 0 invalid / 0 uncertain
  • disposition: none

I read the changed ADR loading rule and the ADR format section in skills/project-docs/SKILL.md. The format section defines the valid frontmatter values as ‘Status frontmatter (proposed | accepted)’. The loading rule separately names both values to explain their meanings, so a future edit to the format set can leave the loading procedure silently incomplete. The values and their semantics are in the same accessible skill, and this is not a dated record or an example subset. Put the open/binding meaning beside each value in the ADR format definition, then have the loading rule say to apply that definition; it can separately say that an ADR without status is binding and how to handle outdated values. Refutation would be a separate system-readable schema defining the status values independently; I found none in the subject.

high — Design brief count duplicates the constraint list

  • claim: 01M41SPMTDFYW4XN25RCP5C0F1
  • anchor: skills/reengineer-program/references/compare-designs.md (snippet)
Have three fresh-context sub-agents independently design the scope.
  • lens: restated-sets · arm: default
  • verdicts: 1 valid / 0 invalid / 0 uncertain
  • disposition: none

I read the changed opening instruction and the constraint list directly below it in skills/reengineer-program/references/compare-designs.md. The list defines the briefs: ‘Smallest model,’ ‘Smallest interface,’ and ‘Easiest common case’; the next instruction says to give each designer a different constraint. ‘Three’ copies the size of that list, so adding or removing a design constraint can silently leave the number of agents wrong. The list is accessible in the same file, is exhaustive for this procedure, and is neither a dated record nor a table of contents. Replace the count with ‘Have a fresh-context sub-agent independently design the scope for each constraint below.’ Refutation would require a separate requirement for exactly three agents regardless of how many constraints the list defines; the procedure supplies no such rationale.

high — Completion rule copies the number of verification checks

  • claim: 01M41SQ0VYDZ9TZG3GVQ92RVYJ
  • anchor: skills/reengineer-program/references/implementation.md (snippet)
Completion requires the implementation to satisfy the confirmed design, all three checks to pass, and every review finding to be resolved.
  • lens: restated-sets · arm: default
  • verdicts: 1 valid / 0 invalid / 0 uncertain
  • disposition: none

I read the changed completion sentence in skills/reengineer-program/references/implementation.md, its Verify section, and skills/reengineering/SKILL.md. The shared skill defines the set under ‘Use three checks:’ with entries ‘Retained promises,’ ‘Dropped promises,’ and ‘Replaced or added promises’; the implementation reference repeats that structure as ‘Unchanged promises,’ ‘Dropped promises,’ and ‘Replaced or added promises.’ The changed sentence’s ‘all three checks’ copies the set size. If the shared verification procedure changes, the count can silently misstate the completion gate. The source is accessible to this workflow, and this is a set size rather than a number used in an argument. Say ‘Completion requires the implementation to satisfy the confirmed design, the verification checks specified by the shared reengineering skill to pass, and every review finding to be resolved.’ A separate, fixed three-check contractual requirement would refute this, but I found none beyond the list itself.

high — verify-doc-drift copies the binding ADR status set

  • claim: 01M41SNRHJN7GVSFZ4JS21YZ9H
  • anchor: skills/verify-doc-drift/SKILL.md (snippet)
An `accepted` or status-less ADR is the source of truth for the decision it records: code contradicting it is `code-drift`.
  • lens: restated-sets · arm: default
  • verdicts: 1 valid / 0 invalid / 0 uncertain
  • disposition: none

I read the changed fix-direction paragraph in skills/verify-doc-drift/SKILL.md and the ADR loading and maintenance rules in skills/project-docs/SKILL.md. The latter is the canonical source for this classification: ‘An ADR marked accepted, or without a status, is decided and binding; proposed means the decision is still open.’ The new verify-doc-drift sentence repeats the members of the binding-status set. It currently agrees, but a later change to project-docs can leave this audit skill silently applying code-drift rules to the wrong ADRs. The reader can load the referenced project-docs skill, so the inaccessible-source exception does not apply. Say ‘A binding ADR, as defined by project-docs, is the source of truth for the decision it records: code contradicting it is code-drift.’ The decisive refutation would be another authoritative source for this skill’s independent status classification; I found none in the snapshot.

high — ADR drift rule repeats project-docs maintenance actions

  • claim: 01M41STGVHDAQ8A4Z39KBR3HR6
  • anchor: skills/verify-doc-drift/SKILL.md (snippet)
Changing the decision is the user's call, after which the ADR is edited, replaced, or deleted per `project-docs`; never edit an ADR just to match the code.
  • lens: restated-sets · arm: default
  • verdicts: 1 valid / 0 invalid / 0 uncertain
  • disposition: none

I read the changed fix-direction paragraph in skills/verify-doc-drift/SKILL.md and the ‘Keep only current decisions’ rule in skills/project-docs/SKILL.md. The latter defines the maintenance actions: ‘When a decision changes, edit its ADR to state the new decision. When a new decision replaces it outright, delete the old ADR and add a new one. Delete an ADR whose decision no longer applies.’ The verify-doc-drift sentence copies that action set as ‘edited, replaced, or deleted.’ It currently agrees at a high level, but a change to the canonical maintenance rule can leave the audit skill prescribing stale options. The project-docs skill is directly named and accessible, so the reader does not need the copied list. Say ‘Changing the decision is the user’s call; then maintain the ADR according to project-docs. Never edit an ADR just to match the code.’ A distinct maintenance policy for drift audits would refute this, but the sentence explicitly delegates to project-docs.

medium — Statusless reengineering backlog becomes binding without migration

  • claim: 01M41SN326AB5VY88DAFHFGAQ8
  • anchor: skills/project-docs/SKILL.md (snippet)
- **Don't re-litigate recorded ADRs.** An ADR marked `accepted`, or without a status, is decided and binding; `proposed` means the decision is still open.
  • lens: general-bug · arm: default
  • verdicts: 1 valid / 0 invalid / 0 uncertain
  • disposition: none

I traced the changed status rule through project-docs, reengineer-program, and verify-doc-drift. The prior reengineer-program instruction explicitly treated status-less ADRs as proposed and left them unchanged until the user answered; the new text removes that exception while project-docs now says every statusless ADR is decided and binding. Thus an existing statusless ADR that was intentionally left in the reengineering decision backlog is now taken as an approved requirement: reengineer-program may use it to permit design or implementation, and verify-doc-drift will classify code that disagrees with it as code drift. This conclusion follows from the served before/after text; this snapshot contains no consumer project's ADRs, so the presence of affected backlog records in a particular project is unverified. Preserve the old interpretation for those records or require an explicit migration/confirmation before treating them as binding.

medium — ADR maintenance refers to a skill that this audit never loads

  • claim: 01M41SR8B4BRF263JVRJ39D11C
  • anchor: skills/verify-doc-drift/SKILL.md (snippet)
after which the ADR is edited, replaced, or deleted per `project-docs`
  • lens: writing-quality · arm: default
  • verdicts: 1 valid / 0 invalid / 0 uncertain
  • disposition: none

I read the changed ADR guidance in verify-doc-drift/SKILL.md, the maintenance rules in project-docs/SKILL.md, and the skill-packaging standard. The audit skill delegates the decision-change procedure to project-docs, but its frontmatter has no axskills.requires dependency and its body never calls the Skill tool to load it. That procedure contains consequential choices: edit an existing ADR versus delete and replace it, update links after deletion, and preserve the user's decision rather than rewrite it to match drift. An agent invoked only for this audit can reach the changed sentence without those rules and make an incomplete ADR change, such as deleting an old ADR while leaving links to it. The packaging standard says to declare hard dependencies and call the skill at the point of use. Declare project-docs as a dependency and load it before ADR maintenance, while keeping the user's decision gate. A delivery contract that always supplies and loads project-docs for this skill would refute the concern; neither guarantee appears in this file.

Other claims

  • grounding-pending (0)
  • ungrounded (0)
  • rejected (0)
  • duplicate-of (0)
  • unadjudicated (0)

Coverage

Coverage pass: 01M41SFZY6AF097K8066D4S79S
Accounting: complete
Slot health: healthy

lens part arm unit status runs loss
general-bug whole default claims-emitted 1 no
writing-quality whole default claims-emitted 1 no
test-trimming whole default no-claims 1 no
restated-sets whole default claims-emitted 1 no
<!-- review:summary --> **Review** `01M41SFZWKA8GGYFCW0ZY7PGEB` — head `bc0921e42bab9862a5108abda1377c10de421e99` # Review — j4k-oss/agent-skills @ 2034125557e6 Scope: diff against base tree `2bb52ad1ca16` Status: dispatched — coverage complete (4/4 slots terminal) Facts: current review-wide projection Computed under: ```json { "abandonment": "abandonment-v1", "anchor_recipe": 1, "batch_policy": "batch-v1", "coverage": "coverage-v3", "dispatch_policy": "dispatch-v2", "grounder_version": 1, "grounding_read_rule": "grounding-read-v1", "promotion_policy": "promotion-v1", "report": "report-v3", "tally": "tally-v1", "triage_settle": "triage-settle-v2" } ``` ## Findings (7) ### high — ADR loading rule repeats the complete status value set - claim: `01M41SRZK1AA7QXAW0CH87CMZE` - anchor: `skills/project-docs/SKILL.md` (snippet) ``` An ADR marked `accepted`, or without a status, is decided and binding; `proposed` means the decision is still open. ``` - lens: restated-sets · arm: default - verdicts: 1 valid / 0 invalid / 0 uncertain - disposition: none > I read the changed ADR loading rule and the ADR format section in `skills/project-docs/SKILL.md`. The format section defines the valid frontmatter values as ‘**Status** frontmatter (`proposed | accepted`)’. The loading rule separately names both values to explain their meanings, so a future edit to the format set can leave the loading procedure silently incomplete. The values and their semantics are in the same accessible skill, and this is not a dated record or an example subset. Put the open/binding meaning beside each value in the ADR format definition, then have the loading rule say to apply that definition; it can separately say that an ADR without status is binding and how to handle outdated values. Refutation would be a separate system-readable schema defining the status values independently; I found none in the subject. ### high — Design brief count duplicates the constraint list - claim: `01M41SPMTDFYW4XN25RCP5C0F1` - anchor: `skills/reengineer-program/references/compare-designs.md` (snippet) ``` Have three fresh-context sub-agents independently design the scope. ``` - lens: restated-sets · arm: default - verdicts: 1 valid / 0 invalid / 0 uncertain - disposition: none > I read the changed opening instruction and the constraint list directly below it in `skills/reengineer-program/references/compare-designs.md`. The list defines the briefs: ‘Smallest model,’ ‘Smallest interface,’ and ‘Easiest common case’; the next instruction says to give each designer a different constraint. ‘Three’ copies the size of that list, so adding or removing a design constraint can silently leave the number of agents wrong. The list is accessible in the same file, is exhaustive for this procedure, and is neither a dated record nor a table of contents. Replace the count with ‘Have a fresh-context sub-agent independently design the scope for each constraint below.’ Refutation would require a separate requirement for exactly three agents regardless of how many constraints the list defines; the procedure supplies no such rationale. ### high — Completion rule copies the number of verification checks - claim: `01M41SQ0VYDZ9TZG3GVQ92RVYJ` - anchor: `skills/reengineer-program/references/implementation.md` (snippet) ``` Completion requires the implementation to satisfy the confirmed design, all three checks to pass, and every review finding to be resolved. ``` - lens: restated-sets · arm: default - verdicts: 1 valid / 0 invalid / 0 uncertain - disposition: none > I read the changed completion sentence in `skills/reengineer-program/references/implementation.md`, its Verify section, and `skills/reengineering/SKILL.md`. The shared skill defines the set under ‘Use three checks:’ with entries ‘Retained promises,’ ‘Dropped promises,’ and ‘Replaced or added promises’; the implementation reference repeats that structure as ‘Unchanged promises,’ ‘Dropped promises,’ and ‘Replaced or added promises.’ The changed sentence’s ‘all three checks’ copies the set size. If the shared verification procedure changes, the count can silently misstate the completion gate. The source is accessible to this workflow, and this is a set size rather than a number used in an argument. Say ‘Completion requires the implementation to satisfy the confirmed design, the verification checks specified by the shared `reengineering` skill to pass, and every review finding to be resolved.’ A separate, fixed three-check contractual requirement would refute this, but I found none beyond the list itself. ### high — verify-doc-drift copies the binding ADR status set - claim: `01M41SNRHJN7GVSFZ4JS21YZ9H` - anchor: `skills/verify-doc-drift/SKILL.md` (snippet) ``` An `accepted` or status-less ADR is the source of truth for the decision it records: code contradicting it is `code-drift`. ``` - lens: restated-sets · arm: default - verdicts: 1 valid / 0 invalid / 0 uncertain - disposition: none > I read the changed fix-direction paragraph in `skills/verify-doc-drift/SKILL.md` and the ADR loading and maintenance rules in `skills/project-docs/SKILL.md`. The latter is the canonical source for this classification: ‘An ADR marked `accepted`, or without a status, is decided and binding; `proposed` means the decision is still open.’ The new verify-doc-drift sentence repeats the members of the binding-status set. It currently agrees, but a later change to project-docs can leave this audit skill silently applying code-drift rules to the wrong ADRs. The reader can load the referenced project-docs skill, so the inaccessible-source exception does not apply. Say ‘A binding ADR, as defined by `project-docs`, is the source of truth for the decision it records: code contradicting it is `code-drift`.’ The decisive refutation would be another authoritative source for this skill’s independent status classification; I found none in the snapshot. ### high — ADR drift rule repeats project-docs maintenance actions - claim: `01M41STGVHDAQ8A4Z39KBR3HR6` - anchor: `skills/verify-doc-drift/SKILL.md` (snippet) ``` Changing the decision is the user's call, after which the ADR is edited, replaced, or deleted per `project-docs`; never edit an ADR just to match the code. ``` - lens: restated-sets · arm: default - verdicts: 1 valid / 0 invalid / 0 uncertain - disposition: none > I read the changed fix-direction paragraph in `skills/verify-doc-drift/SKILL.md` and the ‘Keep only current decisions’ rule in `skills/project-docs/SKILL.md`. The latter defines the maintenance actions: ‘When a decision changes, edit its ADR to state the new decision. When a new decision replaces it outright, delete the old ADR and add a new one. Delete an ADR whose decision no longer applies.’ The verify-doc-drift sentence copies that action set as ‘edited, replaced, or deleted.’ It currently agrees at a high level, but a change to the canonical maintenance rule can leave the audit skill prescribing stale options. The project-docs skill is directly named and accessible, so the reader does not need the copied list. Say ‘Changing the decision is the user’s call; then maintain the ADR according to `project-docs`. Never edit an ADR just to match the code.’ A distinct maintenance policy for drift audits would refute this, but the sentence explicitly delegates to project-docs. ### medium — Statusless reengineering backlog becomes binding without migration - claim: `01M41SN326AB5VY88DAFHFGAQ8` - anchor: `skills/project-docs/SKILL.md` (snippet) ``` - **Don't re-litigate recorded ADRs.** An ADR marked `accepted`, or without a status, is decided and binding; `proposed` means the decision is still open. ``` - lens: general-bug · arm: default - verdicts: 1 valid / 0 invalid / 0 uncertain - disposition: none > I traced the changed status rule through `project-docs`, `reengineer-program`, and `verify-doc-drift`. The prior `reengineer-program` instruction explicitly treated status-less ADRs as proposed and left them unchanged until the user answered; the new text removes that exception while `project-docs` now says every statusless ADR is decided and binding. Thus an existing statusless ADR that was intentionally left in the reengineering decision backlog is now taken as an approved requirement: `reengineer-program` may use it to permit design or implementation, and `verify-doc-drift` will classify code that disagrees with it as code drift. This conclusion follows from the served before/after text; this snapshot contains no consumer project's ADRs, so the presence of affected backlog records in a particular project is unverified. Preserve the old interpretation for those records or require an explicit migration/confirmation before treating them as binding. ### medium — ADR maintenance refers to a skill that this audit never loads - claim: `01M41SR8B4BRF263JVRJ39D11C` - anchor: `skills/verify-doc-drift/SKILL.md` (snippet) ``` after which the ADR is edited, replaced, or deleted per `project-docs` ``` - lens: writing-quality · arm: default - verdicts: 1 valid / 0 invalid / 0 uncertain - disposition: none > I read the changed ADR guidance in `verify-doc-drift/SKILL.md`, the maintenance rules in `project-docs/SKILL.md`, and the skill-packaging standard. The audit skill delegates the decision-change procedure to `project-docs`, but its frontmatter has no `axskills.requires` dependency and its body never calls the Skill tool to load it. That procedure contains consequential choices: edit an existing ADR versus delete and replace it, update links after deletion, and preserve the user's decision rather than rewrite it to match drift. An agent invoked only for this audit can reach the changed sentence without those rules and make an incomplete ADR change, such as deleting an old ADR while leaving links to it. The packaging standard says to declare hard dependencies and call the skill at the point of use. Declare `project-docs` as a dependency and load it before ADR maintenance, while keeping the user's decision gate. A delivery contract that always supplies and loads `project-docs` for this skill would refute the concern; neither guarantee appears in this file. ## Other claims - grounding-pending (0) - ungrounded (0) - rejected (0) - duplicate-of (0) - unadjudicated (0) ## Coverage Coverage pass: 01M41SFZY6AF097K8066D4S79S Accounting: complete Slot health: healthy | lens | part | arm | unit status | runs | loss | | --- | --- | --- | --- | --- | --- | | general-bug | whole | default | claims-emitted | 1 | no | | writing-quality | whole | default | claims-emitted | 1 | no | | test-trimming | whole | default | no-claims | 1 | no | | restated-sets | whole | default | claims-emitted | 1 | no |
@ -52,3 +52,3 @@
Before planning or designing on top of an existing codebase:
1. Check the repo root. If `CONTEXT-MAP.md` exists, read it, identify which contexts the task touches, and read their `CONTEXT.md` files. Otherwise read the root `CONTEXT.md` if present.
1. Check the repo root. If `CONTEXT-MAP.md` exists, read it, list every tracked `CONTEXT.md`, the root one included (`git ls-files ':(glob)**/CONTEXT.md'`), identify which contexts the task touches, and read their `CONTEXT.md` files. Otherwise read the root `CONTEXT.md` if present.

medium — Context discovery skips untracked glossaries in the working tree
lens writing-quality · arm default · tally 1 valid / 0 invalid / 0 uncertain

I read the context-map example, the loading procedure, and the lazy creation rule in this skill. The new instruction uses git ls-files ':(glob)**/CONTEXT.md' as its context inventory, while the map now omits the context list. In a temporary Git repo I staged a root CONTEXT.md and left src/CONTEXT.md untracked; running that exact command printed only CONTEXT.md. A newly created context glossary, which this skill asks agents to create as soon as a term is settled, can therefore be present in the working tree yet absent from discovery before it is staged. The agent may plan with the wrong glossary and skip that context's ADR directory. The writing standard says to use a cheap authoritative lookup and make workflow evidence reliable. Discover CONTEXT.md files from the working tree, including untracked files, then use the resulting locations to select context docs; this preserves the intended lazy structure and avoids adding a duplicate directory list to the map. This affects a multi-context repo with an untracked glossary; the subject tree does not establish how often that worktree state occurs.

claim 01M3XVA54BMXDT8QGHHMRMAMZH of review 01M3XV50BTQCWEGQD4AF64TVWF

<!-- review:claim:01M3XVA54BMXDT8QGHHMRMAMZH --> **medium** — Context discovery skips untracked glossaries in the working tree lens `writing-quality` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain > I read the context-map example, the loading procedure, and the lazy creation rule in this skill. The new instruction uses `git ls-files ':(glob)**/CONTEXT.md'` as its context inventory, while the map now omits the context list. In a temporary Git repo I staged a root `CONTEXT.md` and left `src/CONTEXT.md` untracked; running that exact command printed only `CONTEXT.md`. A newly created context glossary, which this skill asks agents to create as soon as a term is settled, can therefore be present in the working tree yet absent from discovery before it is staged. The agent may plan with the wrong glossary and skip that context's ADR directory. The writing standard says to use a cheap authoritative lookup and make workflow evidence reliable. Discover `CONTEXT.md` files from the working tree, including untracked files, then use the resulting locations to select context docs; this preserves the intended lazy structure and avoids adding a duplicate directory list to the map. This affects a multi-context repo with an untracked glossary; the subject tree does not establish how often that worktree state occurs. claim `01M3XVA54BMXDT8QGHHMRMAMZH` of review `01M3XV50BTQCWEGQD4AF64TVWF`

superseded by review 01M3XVM7DM1RXJMNJVKDT8YYR1 for head b7a6d1aaacf26262ba8c3cfb9a0047215287fc98

<!-- review:superseded:01M3XVM7DM1RXJMNJVKDT8YYR1 --> superseded by review `01M3XVM7DM1RXJMNJVKDT8YYR1` for head `b7a6d1aaacf26262ba8c3cfb9a0047215287fc98`
Author
Owner

Already fixed in e7d1864, which removed the git ls-files discovery step and restored the context list in CONTEXT-MAP.md; loading reads the contexts from the map again.

<!-- gh-feedback:reply-to:101440 --> Already fixed in e7d1864, which removed the `git ls-files` discovery step and restored the context list in `CONTEXT-MAP.md`; loading reads the contexts from the map again.
jercik marked this conversation as resolved
@ -58,3 +58,3 @@
- **Use the glossary's canonical terms** in everything you produce — plans, code, commit messages. When the user's wording conflicts with a defined term, flag the mismatch instead of silently adopting either side.
- **Don't re-litigate recorded ADRs.** A status-less ADR counts as accepted; only an ADR marked `proposed` is still open, and a `deprecated` or `superseded` one no longer binds — its successor, where one exists, governs instead. If the plan contradicts a binding ADR, name it ("this conflicts with ADR-0007") and either adjust the plan or supersede the ADR — never silently override a recorded decision.
- **Don't re-litigate recorded ADRs.** A status-less ADR counts as accepted, and one marked `proposed` is decided but not yet built; a `deprecated` or `superseded` one no longer binds — its successor, where one exists, governs instead. If the plan contradicts a binding ADR, name it ("this conflicts with ADR-0007") and either adjust the plan or supersede the ADR — never silently override a recorded decision.

medium — proposed has conflicting meanings across documentation skills
lens writing-quality · arm default · tally 1 valid / 0 invalid / 0 uncertain

I read the changed project-docs skill and its consumers. This passage now defines a proposed ADR as a binding decision awaiting implementation. The new maintenance rule also requires recording an unbuilt decision as proposed. But skills/verify-doc-drift/SKILL.md says a proposed ADR is an open question, while an accepted ADR is the source of truth; it directs the audit to fix a proposed ADR's description when code disagrees. An agent following both skills can therefore treat a settled, unbuilt decision as an open proposal and rewrite its intended behavior to match the current code. reengineer-program explicitly supplies its own ADR precedence, but verify-doc-drift does not. The writing standard's one-term-one-concept guidance applies here. Use accepted for a settled decision even if implementation is pending, with implementation state recorded separately, or update the dependent skill to use the new status contract consistently. That preserves the distinction between settled decisions and unfinished work. The decisive check is whether every consumer of project-docs treats proposed as binding; the named consumer currently does not.

claim 01M3XV82Q857XA3W14T0EGSDT5 of review 01M3XV50BTQCWEGQD4AF64TVWF

<!-- review:claim:01M3XV82Q857XA3W14T0EGSDT5 --> **medium** — `proposed` has conflicting meanings across documentation skills lens `writing-quality` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain > I read the changed project-docs skill and its consumers. This passage now defines a `proposed` ADR as a binding decision awaiting implementation. The new maintenance rule also requires recording an unbuilt decision as `proposed`. But `skills/verify-doc-drift/SKILL.md` says a `proposed` ADR is an open question, while an `accepted` ADR is the source of truth; it directs the audit to fix a proposed ADR's description when code disagrees. An agent following both skills can therefore treat a settled, unbuilt decision as an open proposal and rewrite its intended behavior to match the current code. `reengineer-program` explicitly supplies its own ADR precedence, but `verify-doc-drift` does not. The writing standard's one-term-one-concept guidance applies here. Use `accepted` for a settled decision even if implementation is pending, with implementation state recorded separately, or update the dependent skill to use the new status contract consistently. That preserves the distinction between settled decisions and unfinished work. The decisive check is whether every consumer of project-docs treats `proposed` as binding; the named consumer currently does not. claim `01M3XV82Q857XA3W14T0EGSDT5` of review `01M3XV50BTQCWEGQD4AF64TVWF`

superseded by review 01M3XVM7DM1RXJMNJVKDT8YYR1 for head b7a6d1aaacf26262ba8c3cfb9a0047215287fc98

<!-- review:superseded:01M3XVM7DM1RXJMNJVKDT8YYR1 --> superseded by review `01M3XVM7DM1RXJMNJVKDT8YYR1` for head `b7a6d1aaacf26262ba8c3cfb9a0047215287fc98`
Author
Owner

Already fixed in a02ba51: proposed again means an open decision in project-docs, matching verify-doc-drift's "open question" reading, and a settled decision is accepted.

<!-- gh-feedback:reply-to:101439 --> Already fixed in a02ba51: `proposed` again means an open decision in project-docs, matching verify-doc-drift's "open question" reading, and a settled decision is `accepted`.
jercik marked this conversation as resolved
@ -146,2 +143,3 @@
- **Update inline, don't batch.** When the user settles a term or a decision mid-session, write it down right then. Deferred documentation doesn't happen. A plan you propose is not a decision until the user accepts it.
- **Update inline, don't batch.** When the user settles a term or a decision mid-session, write it down right then. Deferred documentation doesn't happen. A plan you suggest is not a decision until the user accepts it.
- **Accept an ADR with its implementation.** Record a decision that is not yet built as `proposed`. The change that builds it, and no earlier one, sets the ADR to `accepted` before it merges and marks each older record the ADR supersedes or amends; an amended record keeps its status and gains a note naming the amending ADR.

medium — Delayed supersession leaves contradictory ADRs binding during planning
lens writing-quality · arm default · tally 1 valid / 0 invalid / 0 uncertain

I traced this new maintenance rule against the earlier Don't re-litigate recorded ADRs rule in the same skill. The earlier rule calls a proposed ADR a decision that should not be re-litigated, while a status-less or accepted ADR remains binding until marked superseded. If a user decides to replace accepted ADR A with unbuilt ADR B, this passage makes B proposed but postpones marking A superseded until B's implementation change. Between those events, a later planning session is told to respect both contradictory records and has no rule for choosing one. This is static reasoning from the two passages; the tree does not show a live pair of ADRs in that state. Mark the old ADR's decision relationship when the replacement is settled, or explicitly state which ADR governs during the implementation gap and align the meaning of proposed with that choice. This preserves the useful distinction between the decision date and implementation state while giving the reader one governing decision. The writing standard calls for one term per concept and a clear rule at the point of choice.

claim 01M3XVAW7EMNM3XQSGN54W8D5W of review 01M3XV50BTQCWEGQD4AF64TVWF

<!-- review:claim:01M3XVAW7EMNM3XQSGN54W8D5W --> **medium** — Delayed supersession leaves contradictory ADRs binding during planning lens `writing-quality` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain > I traced this new maintenance rule against the earlier `Don't re-litigate recorded ADRs` rule in the same skill. The earlier rule calls a `proposed` ADR a decision that should not be re-litigated, while a status-less or accepted ADR remains binding until marked superseded. If a user decides to replace accepted ADR A with unbuilt ADR B, this passage makes B `proposed` but postpones marking A superseded until B's implementation change. Between those events, a later planning session is told to respect both contradictory records and has no rule for choosing one. This is static reasoning from the two passages; the tree does not show a live pair of ADRs in that state. Mark the old ADR's decision relationship when the replacement is settled, or explicitly state which ADR governs during the implementation gap and align the meaning of `proposed` with that choice. This preserves the useful distinction between the decision date and implementation state while giving the reader one governing decision. The writing standard calls for one term per concept and a clear rule at the point of choice. claim `01M3XVAW7EMNM3XQSGN54W8D5W` of review `01M3XV50BTQCWEGQD4AF64TVWF`

superseded by review 01M3XVM7DM1RXJMNJVKDT8YYR1 for head b7a6d1aaacf26262ba8c3cfb9a0047215287fc98

<!-- review:superseded:01M3XVM7DM1RXJMNJVKDT8YYR1 --> superseded by review `01M3XVM7DM1RXJMNJVKDT8YYR1` for head `b7a6d1aaacf26262ba8c3cfb9a0047215287fc98`
Author
Owner

Already fixed in a02ba51 and c672626: proposed is open again and does not bind, so the accepted predecessor is the one governing decision until the change that builds the replacement edits or deletes it. No supersession step remains.

<!-- gh-feedback:reply-to:101441 --> Already fixed in a02ba51 and c672626: `proposed` is open again and does not bind, so the accepted predecessor is the one governing decision until the change that builds the replacement edits or deletes it. No supersession step remains.
jercik marked this conversation as resolved
fix(skills): align reengineer-program and verify-doc-drift with the proposed ADR status
All checks were successful
commit-msg / commitlint (pull_request) Successful in 25s
Node tests / node:test (pull_request) Successful in 1m31s
Review / Review (pull_request_target) Successful in 6m53s
b7a6d1aaac
@ -52,3 +52,3 @@
Before planning or designing on top of an existing codebase:
1. Check the repo root. If `CONTEXT-MAP.md` exists, read it, identify which contexts the task touches, and read their `CONTEXT.md` files. Otherwise read the root `CONTEXT.md` if present.
1. Check the repo root. If `CONTEXT-MAP.md` exists, read it, list every tracked `CONTEXT.md`, the root one included (`git ls-files ':(glob)**/CONTEXT.md'`), identify which contexts the task touches, and read their `CONTEXT.md` files. Otherwise read the root `CONTEXT.md` if present.

medium — Include untracked context glossaries when loading project docs
lens writing-quality · arm default · tally 1 valid / 0 invalid / 0 uncertain

I read the complete project-docs loading and lazy-creation rules and the callers in grill-with-docs and improve-codebase-architecture. The new instruction uses git ls-files and explicitly limits discovery to tracked CONTEXT.md files. A context glossary just created under the lazy-creation rule, or supplied as an untracked working-tree file, is therefore absent from the discovery list until staged or committed. Because the context map no longer inventories contexts, a later session can omit that glossary and plan with missing domain terms. The writing standard asks for observable, checkable evidence and for instructions that work with a dirty working tree. Include existing untracked nonignored CONTEXT.md files in discovery, or state and justify a deliberate tracked-only boundary. The claim would be refuted if another loading step found those working-tree files; the reviewed skill and its callers have none.

claim 01M3XVVDQ0JYC53YMFHS818ZZ9 of review 01M3XVM7DM1RXJMNJVKDT8YYR1

<!-- review:claim:01M3XVVDQ0JYC53YMFHS818ZZ9 --> **medium** — Include untracked context glossaries when loading project docs lens `writing-quality` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain > I read the complete project-docs loading and lazy-creation rules and the callers in grill-with-docs and improve-codebase-architecture. The new instruction uses git ls-files and explicitly limits discovery to tracked CONTEXT.md files. A context glossary just created under the lazy-creation rule, or supplied as an untracked working-tree file, is therefore absent from the discovery list until staged or committed. Because the context map no longer inventories contexts, a later session can omit that glossary and plan with missing domain terms. The writing standard asks for observable, checkable evidence and for instructions that work with a dirty working tree. Include existing untracked nonignored CONTEXT.md files in discovery, or state and justify a deliberate tracked-only boundary. The claim would be refuted if another loading step found those working-tree files; the reviewed skill and its callers have none. claim `01M3XVVDQ0JYC53YMFHS818ZZ9` of review `01M3XVM7DM1RXJMNJVKDT8YYR1`

superseded by review 01M3XWHXMV98SN26Q201S1D7BP for head e7d186400ced1a3495e7d4c2b4b9d44d775ec3cb

<!-- review:superseded:01M3XWHXMV98SN26Q201S1D7BP --> superseded by review `01M3XWHXMV98SN26Q201S1D7BP` for head `e7d186400ced1a3495e7d4c2b4b9d44d775ec3cb`
Author
Owner

Already fixed in e7d1864, which removed the git ls-files discovery step and restored the context list in CONTEXT-MAP.md; loading reads the contexts from the map again.

<!-- gh-feedback:reply-to:101506 --> Already fixed in e7d1864, which removed the `git ls-files` discovery step and restored the context list in `CONTEXT-MAP.md`; loading reads the contexts from the map again.
jercik marked this conversation as resolved
@ -58,3 +58,3 @@
- **Use the glossary's canonical terms** in everything you produce — plans, code, commit messages. When the user's wording conflicts with a defined term, flag the mismatch instead of silently adopting either side.
- **Don't re-litigate recorded ADRs.** A status-less ADR counts as accepted; only an ADR marked `proposed` is still open, and a `deprecated` or `superseded` one no longer binds — its successor, where one exists, governs instead. If the plan contradicts a binding ADR, name it ("this conflicts with ADR-0007") and either adjust the plan or supersede the ADR — never silently override a recorded decision.
- **Don't re-litigate recorded ADRs.** A status-less ADR counts as accepted, and one marked `proposed` is decided but not yet built; a `deprecated` or `superseded` one no longer binds — its successor, where one exists, governs instead. If the plan contradicts a binding ADR, name it ("this conflicts with ADR-0007") and either adjust the plan or supersede the ADR — never silently override a recorded decision.

medium — Project-docs treats new open ADRs as binding decisions
lens general-bug · arm default · tally 1 valid / 0 invalid / 0 uncertain

I traced the new ADR status rules through reengineer-program, project-docs, and grill-with-docs. Reengineer-program now creates status: open records before asking the user and says they only state the current behavior; those records are its decision backlog. Project-docs' changed loading rule lists only status-less and proposed as active cases and deprecated/superseded as nonbinding; its leading directive says not to re-litigate recorded ADRs and to adjust or supersede a conflicting plan. It never identifies status: open as an unanswered question. A later planning or grill-with-docs session, which applies project-docs directly rather than reengineer-program's override, can treat an unconfirmed description of the old behavior as a binding decision and force a new plan to preserve or supersede it. Project-docs should explicitly classify open ADRs as undecided and ask about them. This is a static cross-skill trace; exercising a downstream project with an open ADR would establish whether an agent actually makes that misclassification.

claim 01M3XVQVXKCJS7M299V8VHC557 of review 01M3XVM7DM1RXJMNJVKDT8YYR1

<!-- review:claim:01M3XVQVXKCJS7M299V8VHC557 --> **medium** — Project-docs treats new open ADRs as binding decisions lens `general-bug` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain > I traced the new ADR status rules through reengineer-program, project-docs, and grill-with-docs. Reengineer-program now creates `status: open` records before asking the user and says they only state the current behavior; those records are its decision backlog. Project-docs' changed loading rule lists only status-less and proposed as active cases and deprecated/superseded as nonbinding; its leading directive says not to re-litigate recorded ADRs and to adjust or supersede a conflicting plan. It never identifies `status: open` as an unanswered question. A later planning or grill-with-docs session, which applies project-docs directly rather than reengineer-program's override, can treat an unconfirmed description of the old behavior as a binding decision and force a new plan to preserve or supersede it. Project-docs should explicitly classify open ADRs as undecided and ask about them. This is a static cross-skill trace; exercising a downstream project with an open ADR would establish whether an agent actually makes that misclassification. claim `01M3XVQVXKCJS7M299V8VHC557` of review `01M3XVM7DM1RXJMNJVKDT8YYR1`

superseded by review 01M3XWHXMV98SN26Q201S1D7BP for head e7d186400ced1a3495e7d4c2b4b9d44d775ec3cb

<!-- review:superseded:01M3XWHXMV98SN26Q201S1D7BP --> superseded by review `01M3XWHXMV98SN26Q201S1D7BP` for head `e7d186400ced1a3495e7d4c2b4b9d44d775ec3cb`
Author
Owner

Already fixed in 80588f6, which reverted the status: open records; reengineer-program again marks open ADRs status: proposed, which project-docs treats as still open.

<!-- gh-feedback:reply-to:101505 --> Already fixed in 80588f6, which reverted the `status: open` records; reengineer-program again marks open ADRs `status: proposed`, which project-docs treats as still open.
jercik marked this conversation as resolved
@ -46,3 +46,1 @@
- **Open (`status: proposed`):** state the situation and current behavior. Status-less ADRs count as proposed and stay unchanged until answered. Superseded or deprecated ADRs do not settle promises still in code.
- **Keep (`status: accepted`):** state the situation, obligation, stakeholder goal, or risk and the chosen promise. A deferred keep names the owner who must decide.
- **Drop (`status: accepted`):** state the negative promise and reason, such as "We do not support concurrent writers because each deployment has one writer." For "don't care, take the smallest," record only the negative promise. Record drops even when `project-docs`' ADR test excludes them.
- **Open (`status: open`):** state the situation and current behavior. Superseded or deprecated ADRs do not settle promises still in code.

medium — Previously open status-less ADRs are treated as settled on resume
lens general-bug · arm default · tally 1 valid / 0 invalid / 0 uncertain

I read the complete reengineer-program and project-docs skills and compared this change with the served diff. The old reengineer-program rule explicitly treated status-less ADRs as proposed and left them unanswered; this change replaces its open classification with only status: open. The current project-docs loading rule says a status-less ADR counts as accepted, and reengineer-program now says only open ADRs form the decision backlog while decided ADRs remain settled. A project resumed with an unresolved status-less ADR created under the previous rule can therefore skip the user question and treat the recorded current behavior as a confirmed promise, then pass it to design or implementation. Existing status-less open records need a migration rule or must continue to be recognized as undecided. This is a static instruction trace; I could not inspect downstream project ADRs in this snapshot, so an inventory showing no such records among users would refute the migration impact.

claim 01M3XVQ2X4K3R9NDPA03HTK9SW of review 01M3XVM7DM1RXJMNJVKDT8YYR1

<!-- review:claim:01M3XVQ2X4K3R9NDPA03HTK9SW --> **medium** — Previously open status-less ADRs are treated as settled on resume lens `general-bug` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain > I read the complete reengineer-program and project-docs skills and compared this change with the served diff. The old reengineer-program rule explicitly treated status-less ADRs as proposed and left them unanswered; this change replaces its open classification with only `status: open`. The current project-docs loading rule says a status-less ADR counts as accepted, and reengineer-program now says only open ADRs form the decision backlog while decided ADRs remain settled. A project resumed with an unresolved status-less ADR created under the previous rule can therefore skip the user question and treat the recorded current behavior as a confirmed promise, then pass it to design or implementation. Existing status-less open records need a migration rule or must continue to be recognized as undecided. This is a static instruction trace; I could not inspect downstream project ADRs in this snapshot, so an inventory showing no such records among users would refute the migration impact. claim `01M3XVQ2X4K3R9NDPA03HTK9SW` of review `01M3XVM7DM1RXJMNJVKDT8YYR1`

superseded by review 01M3XWHXMV98SN26Q201S1D7BP for head e7d186400ced1a3495e7d4c2b4b9d44d775ec3cb

<!-- review:superseded:01M3XWHXMV98SN26Q201S1D7BP --> superseded by review `01M3XWHXMV98SN26Q201S1D7BP` for head `e7d186400ced1a3495e7d4c2b4b9d44d775ec3cb`
Author
Owner

Declining: a status-less ADR counting as accepted is the user's decision for this PR. The statuses are proposed (open) and accepted (decided), and a status-less ADR is accepted. project-docs on main already said so; reengineer-program's "status-less ADRs count as proposed" override contradicted it. reengineer-program did not create status-less question ADRs either: on main it gave every ADR status frontmatter and created each open root as status: proposed, which still marks an open ADR.

<!-- gh-feedback:reply-to:101507 --> Declining: a status-less ADR counting as accepted is the user's decision for this PR. The statuses are `proposed` (open) and `accepted` (decided), and a status-less ADR is accepted. project-docs on main already said so; reengineer-program's "status-less ADRs count as proposed" override contradicted it. reengineer-program did not create status-less question ADRs either: on main it gave every ADR `status` frontmatter and created each open root as `status: proposed`, which still marks an open ADR.
@ -49,0 +47,4 @@
- **Keep:** state the situation, obligation, stakeholder goal, or risk and the chosen promise. A deferred keep names the owner who must decide.
- **Drop:** state the negative promise and reason, such as "We do not support concurrent writers because each deployment has one writer." For "don't care, take the smallest," record only the negative promise. Record drops even when `project-docs`' ADR test excludes them.
Mark a keep or drop `accepted` when the code already reflects it; otherwise mark it `proposed` until the change that builds it merges. Both are decided ADRs.

medium — Existing proposed ADRs become decided without a user answer
lens general-bug · arm default · tally 1 valid / 0 invalid / 0 uncertain

I compared the complete reengineer-program skill against its served diff and read the project-docs rules it calls. Before this change, reengineer-program instructed agents to create an open ADR for each unrecorded root with status: proposed, describing the situation and current behavior, and to keep it proposed until answered. The new rule instead treats proposed as a decided keep or drop awaiting implementation, and says only status: open records remain in the question backlog. A resumed project with old proposed ADRs will therefore treat unanswered roots as settled, omit them from the interview, and may implement or preserve the old behavior without confirmation. The status transition needs a migration or a way to distinguish legacy open proposed ADRs from newly decided proposed ADRs. This is a static trace through the instructions; I could not inspect downstream project ADRs here, so absence of legacy proposed records would refute impact for a particular project.

claim 01M3XVRP4CRMXFQXAASC94BV76 of review 01M3XVM7DM1RXJMNJVKDT8YYR1

<!-- review:claim:01M3XVRP4CRMXFQXAASC94BV76 --> **medium** — Existing proposed ADRs become decided without a user answer lens `general-bug` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain > I compared the complete reengineer-program skill against its served diff and read the project-docs rules it calls. Before this change, reengineer-program instructed agents to create an open ADR for each unrecorded root with `status: proposed`, describing the situation and current behavior, and to keep it proposed until answered. The new rule instead treats `proposed` as a decided keep or drop awaiting implementation, and says only `status: open` records remain in the question backlog. A resumed project with old proposed ADRs will therefore treat unanswered roots as settled, omit them from the interview, and may implement or preserve the old behavior without confirmation. The status transition needs a migration or a way to distinguish legacy open proposed ADRs from newly decided proposed ADRs. This is a static trace through the instructions; I could not inspect downstream project ADRs here, so absence of legacy proposed records would refute impact for a particular project. claim `01M3XVRP4CRMXFQXAASC94BV76` of review `01M3XVM7DM1RXJMNJVKDT8YYR1`

superseded by review 01M3XWHXMV98SN26Q201S1D7BP for head e7d186400ced1a3495e7d4c2b4b9d44d775ec3cb

<!-- review:superseded:01M3XWHXMV98SN26Q201S1D7BP --> superseded by review `01M3XWHXMV98SN26Q201S1D7BP` for head `e7d186400ced1a3495e7d4c2b4b9d44d775ec3cb`
Author
Owner

Already fixed in 80588f6, which reverted b7a6d1a; reengineer-program again marks an unanswered ADR Open (status: proposed), so existing proposed ADRs stay in the decision backlog.

<!-- gh-feedback:reply-to:101508 --> Already fixed in 80588f6, which reverted b7a6d1a; reengineer-program again marks an unanswered ADR `Open (status: proposed)`, so existing proposed ADRs stay in the decision backlog.
jercik marked this conversation as resolved
fix(project-docs): keep the context map's list of contexts
Some checks failed
commit-msg / commitlint (pull_request) Successful in 18s
Node tests / node:test (pull_request) Successful in 2m7s
Review / Review (pull_request_target) Failing after 6m36s
e7d186400c
jercik changed title from fix(project-docs): accept ADRs with their implementation and keep the context map to relationships to fix(project-docs): accept ADRs with their implementation 2026-10-02 08:44:06 +00:00
@ -58,3 +58,3 @@
- **Use the glossary's canonical terms** in everything you produce — plans, code, commit messages. When the user's wording conflicts with a defined term, flag the mismatch instead of silently adopting either side.
- **Don't re-litigate recorded ADRs.** A status-less ADR counts as accepted; only an ADR marked `proposed` is still open, and a `deprecated` or `superseded` one no longer binds — its successor, where one exists, governs instead. If the plan contradicts a binding ADR, name it ("this conflicts with ADR-0007") and either adjust the plan or supersede the ADR — never silently override a recorded decision.
- **Don't re-litigate recorded ADRs.** A status-less ADR counts as accepted, and one marked `proposed` is decided but not yet built; a `deprecated` or `superseded` one no longer binds — its successor, where one exists, governs instead. If the plan contradicts a binding ADR, name it ("this conflicts with ADR-0007") and either adjust the plan or supersede the ADR — never silently override a recorded decision.

medium — Existing statusless question ADRs become settled without an answer
lens general-bug · arm default · tally 1 valid / 0 invalid / 0 uncertain

I read the full project-docs and reengineer-program skills and the served diff. The prior reengineer-program rule explicitly said status-less ADRs count as proposed and remain in the decision backlog until answered. This revision removes that rule, says only status: open ADRs are the backlog, and calls project-docs, whose new loading rule says a status-less ADR counts as accepted. On resuming a repository with an unanswered statusless ADR created or interpreted under the previous rule, the agent now treats its current-behavior statement as a settled requirement. The 'all ADRs settled' design gate can pass without asking the owner, and a rebuild can preserve or implement an unapproved promise. This is a static workflow trace; I could not inspect downstream repositories for an instance. Preserve the legacy undecided interpretation until such ADRs are explicitly answered or migrated.

claim 01M3XWRDVN83FNAPVQTPHE8QK1 of review 01M3XWHXMV98SN26Q201S1D7BP

<!-- review:claim:01M3XWRDVN83FNAPVQTPHE8QK1 --> **medium** — Existing statusless question ADRs become settled without an answer lens `general-bug` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain > I read the full project-docs and reengineer-program skills and the served diff. The prior reengineer-program rule explicitly said status-less ADRs count as proposed and remain in the decision backlog until answered. This revision removes that rule, says only `status: open` ADRs are the backlog, and calls project-docs, whose new loading rule says a status-less ADR counts as accepted. On resuming a repository with an unanswered statusless ADR created or interpreted under the previous rule, the agent now treats its current-behavior statement as a settled requirement. The 'all ADRs settled' design gate can pass without asking the owner, and a rebuild can preserve or implement an unapproved promise. This is a static workflow trace; I could not inspect downstream repositories for an instance. Preserve the legacy undecided interpretation until such ADRs are explicitly answered or migrated. claim `01M3XWRDVN83FNAPVQTPHE8QK1` of review `01M3XWHXMV98SN26Q201S1D7BP`

superseded by review 01M3XYGR3KHZXEV5EJVPMTXCDT for head a02ba510acbbff8582b9f9c4833c656b9dfff285

<!-- review:superseded:01M3XYGR3KHZXEV5EJVPMTXCDT --> superseded by review `01M3XYGR3KHZXEV5EJVPMTXCDT` for head `a02ba510acbbff8582b9f9c4833c656b9dfff285`

superseded by review 01M3XYGR3KHZXEV5EJVPMTXCDT for head a02ba510acbbff8582b9f9c4833c656b9dfff285

<!-- review:superseded:01M3XYGR3KHZXEV5EJVPMTXCDT --> superseded by review `01M3XYGR3KHZXEV5EJVPMTXCDT` for head `a02ba510acbbff8582b9f9c4833c656b9dfff285`
Author
Owner

Declining: a status-less ADR counting as accepted is the user's decision for this PR. The statuses are proposed (open) and accepted (decided), and a status-less ADR is accepted. project-docs on main already said so; reengineer-program's "status-less ADRs count as proposed" override contradicted it. reengineer-program did not create status-less question ADRs either: on main it gave every ADR status frontmatter and created each open root as status: proposed, which still marks an open ADR.

<!-- gh-feedback:reply-to:101631 --> Declining: a status-less ADR counting as accepted is the user's decision for this PR. The statuses are `proposed` (open) and `accepted` (decided), and a status-less ADR is accepted. project-docs on main already said so; reengineer-program's "status-less ADRs count as proposed" override contradicted it. reengineer-program did not create status-less question ADRs either: on main it gave every ADR `status` frontmatter and created each open root as `status: proposed`, which still marks an open ADR.
@ -121,3 +121,3 @@
That's it. An ADR can be a single paragraph — the value is in recording _that_ a decision was made and _why_, not in filling out sections. Add optional sections only when they earn their place:
- **Status** frontmatter (`proposed | accepted | deprecated | superseded by ADR-NNNN`) — when decisions get revisited
- **Status** frontmatter (`proposed | accepted | deprecated | superseded by ADR-NNNN`) — when a decision is not yet built or gets revisited

medium — Define the open ADR status in the shared documentation contract
lens writing-quality · arm default · tally 1 valid / 0 invalid / 0 uncertain

I read the changed project-docs guidance, the full reengineer-program skill, and the changed verify-doc-drift guidance. Reengineer-program now requires status: open for every unrecorded root and calls those records its decision backlog, but this shared ADR format omits open; its loading rule says recorded ADRs are binding except deprecated or superseded ones. A later project-docs user can therefore treat an unresolved open record as a settled decision, while the doc-drift workflow defines only proposed and accepted handling. Add open to the shared status list and state that it is an unresolved, nonbinding question; align the audit guidance with that meaning. This preserves the new distinction between undecided, decided-but-unbuilt, and implemented ADRs. This is a static cross-skill reading; an explicit shared rule giving open that meaning would refute the gap.

claim 01M3XWNTBGK78RESH76KFWFWCB of review 01M3XWHXMV98SN26Q201S1D7BP

<!-- review:claim:01M3XWNTBGK78RESH76KFWFWCB --> **medium** — Define the open ADR status in the shared documentation contract lens `writing-quality` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain > I read the changed project-docs guidance, the full reengineer-program skill, and the changed verify-doc-drift guidance. Reengineer-program now requires `status: open` for every unrecorded root and calls those records its decision backlog, but this shared ADR format omits `open`; its loading rule says recorded ADRs are binding except deprecated or superseded ones. A later project-docs user can therefore treat an unresolved open record as a settled decision, while the doc-drift workflow defines only proposed and accepted handling. Add `open` to the shared status list and state that it is an unresolved, nonbinding question; align the audit guidance with that meaning. This preserves the new distinction between undecided, decided-but-unbuilt, and implemented ADRs. This is a static cross-skill reading; an explicit shared rule giving `open` that meaning would refute the gap. claim `01M3XWNTBGK78RESH76KFWFWCB` of review `01M3XWHXMV98SN26Q201S1D7BP`

superseded by review 01M3XYGR3KHZXEV5EJVPMTXCDT for head a02ba510acbbff8582b9f9c4833c656b9dfff285

<!-- review:superseded:01M3XYGR3KHZXEV5EJVPMTXCDT --> superseded by review `01M3XYGR3KHZXEV5EJVPMTXCDT` for head `a02ba510acbbff8582b9f9c4833c656b9dfff285`

superseded by review 01M3XYGR3KHZXEV5EJVPMTXCDT for head a02ba510acbbff8582b9f9c4833c656b9dfff285

<!-- review:superseded:01M3XYGR3KHZXEV5EJVPMTXCDT --> superseded by review `01M3XYGR3KHZXEV5EJVPMTXCDT` for head `a02ba510acbbff8582b9f9c4833c656b9dfff285`
Author
Owner

Already fixed in 80588f6, which reverted the status: open records; the shared status list (proposed | accepted) covers every status the skills write.

<!-- gh-feedback:reply-to:101629 --> Already fixed in 80588f6, which reverted the `status: open` records; the shared status list (`proposed | accepted`) covers every status the skills write.
jercik marked this conversation as resolved
@ -146,2 +146,3 @@
- **Update inline, don't batch.** When the user settles a term or a decision mid-session, write it down right then. Deferred documentation doesn't happen. A plan you propose is not a decision until the user accepts it.
- **Update inline, don't batch.** When the user settles a term or a decision mid-session, write it down right then. Deferred documentation doesn't happen. A plan you suggest is not a decision until the user accepts it.
- **Accept an ADR with its implementation.** Record a decision that is not yet built as `proposed`. The change that builds it, and no earlier one, sets the ADR to `accepted` before it merges and marks each older record the ADR supersedes or amends; an amended record keeps its status and gains a note naming the amending ADR.

medium — Specify which ADR governs while a decided replacement awaits implementation
lens writing-quality · arm default · tally 1 valid / 0 invalid / 0 uncertain

I read the project-docs loading and maintenance rules together with the changed reengineer-program and verify-doc-drift guidance. Project-docs says proposed means decided but unbuilt and tells readers to hold recorded ADRs as binding inputs. This new rule waits until the implementation change to mark an older accepted ADR superseded. If an accepted ADR requires behavior A and a newly decided proposed ADR replaces it with B, both remain active during the planning interval, and the instruction to adjust a plan that contradicts a binding ADR gives no precedence. An agent can reject the B implementation as conflicting with A, or follow A despite the newer decision. State explicitly that the proposed successor governs future work while the accepted predecessor describes the code still deployed, then retire the predecessor with the implementation; this preserves the intended current-versus-planned distinction. This is a static conflict in the stated lifecycle; a precedence rule for that interval elsewhere in the reviewed guidance would refute it.

claim 01M3XWPW8BT8ZP7SEKT3HF1KV7 of review 01M3XWHXMV98SN26Q201S1D7BP

<!-- review:claim:01M3XWPW8BT8ZP7SEKT3HF1KV7 --> **medium** — Specify which ADR governs while a decided replacement awaits implementation lens `writing-quality` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain > I read the project-docs loading and maintenance rules together with the changed reengineer-program and verify-doc-drift guidance. Project-docs says proposed means decided but unbuilt and tells readers to hold recorded ADRs as binding inputs. This new rule waits until the implementation change to mark an older accepted ADR superseded. If an accepted ADR requires behavior A and a newly decided proposed ADR replaces it with B, both remain active during the planning interval, and the instruction to adjust a plan that contradicts a binding ADR gives no precedence. An agent can reject the B implementation as conflicting with A, or follow A despite the newer decision. State explicitly that the proposed successor governs future work while the accepted predecessor describes the code still deployed, then retire the predecessor with the implementation; this preserves the intended current-versus-planned distinction. This is a static conflict in the stated lifecycle; a precedence rule for that interval elsewhere in the reviewed guidance would refute it. claim `01M3XWPW8BT8ZP7SEKT3HF1KV7` of review `01M3XWHXMV98SN26Q201S1D7BP`

superseded by review 01M3XYGR3KHZXEV5EJVPMTXCDT for head a02ba510acbbff8582b9f9c4833c656b9dfff285

<!-- review:superseded:01M3XYGR3KHZXEV5EJVPMTXCDT --> superseded by review `01M3XYGR3KHZXEV5EJVPMTXCDT` for head `a02ba510acbbff8582b9f9c4833c656b9dfff285`

superseded by review 01M3XYGR3KHZXEV5EJVPMTXCDT for head a02ba510acbbff8582b9f9c4833c656b9dfff285

<!-- review:superseded:01M3XYGR3KHZXEV5EJVPMTXCDT --> superseded by review `01M3XYGR3KHZXEV5EJVPMTXCDT` for head `a02ba510acbbff8582b9f9c4833c656b9dfff285`
Author
Owner

Already fixed in a02ba51 and c672626: proposed is open again and does not bind, so the accepted predecessor is the one governing decision until the change that builds the replacement edits or deletes it. No supersession step remains.

<!-- gh-feedback:reply-to:101630 --> Already fixed in a02ba51 and c672626: `proposed` is open again and does not bind, so the accepted predecessor is the one governing decision until the change that builds the replacement edits or deletes it. No supersession step remains.
jercik marked this conversation as resolved
jercik changed title from fix(project-docs): accept ADRs with their implementation to fix(project-docs): ship an ADR accepted with the change that builds it 2026-10-02 09:18:21 +00:00
This reverts commit b7a6d1aaac.
fix(project-docs): keep standard ADR status meanings
Some checks failed
commit-msg / commitlint (pull_request) Successful in 18s
Node tests / node:test (pull_request) Successful in 1m36s
Review / Review (pull_request_target) Failing after 7m40s
a02ba510ac
@ -145,6 +145,7 @@ What qualifies:
## Maintenance rules
- **Update inline, don't batch.** When the user settles a term or a decision mid-session, write it down right then. Deferred documentation doesn't happen. A plan you propose is not a decision until the user accepts it.
- **Ship an ADR accepted with the change that builds it.** That change sets the ADR to `accepted` before it merges and marks each older record it supersedes or amends; an amended record keeps its status and gains a note naming the amending ADR.

medium — Clarify when a draft ADR becomes accepted
lens writing-quality · arm default · tally 1 valid / 0 invalid / 0 uncertain

I read the full project-docs skill and traced this maintenance rule against its ADR format and loading rules. The preceding rule says a proposed plan is not a decision until the user accepts it; the format permits proposed, and the loading rule treats accepted (or a status-less ADR) as binding. Here, “the change that builds it” can refer to the change creating the ADR itself, while “That change sets the ADR to accepted before it merges” then tells an agent to accept an open draft merely to merge its documentation. That can turn an unresolved choice into a binding input for later planning. The writing standard calls for leading with the action and attaching conditions to the action they govern. Say explicitly that the trigger is the implementation change for a user-approved decision, and that an ADR recording an unresolved choice stays proposed; retain the useful requirement to update superseded or amended records in that change. This is a static reading of the instruction; I could not observe an agent follow it. An explicit definition of “the change that builds it” as an implementation change would refute the ambiguity.

claim 01M3XYS034F59QPTYBHAEPXDXN of review 01M3XYGR3KHZXEV5EJVPMTXCDT

<!-- review:claim:01M3XYS034F59QPTYBHAEPXDXN --> **medium** — Clarify when a draft ADR becomes accepted lens `writing-quality` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain > I read the full project-docs skill and traced this maintenance rule against its ADR format and loading rules. The preceding rule says a proposed plan is not a decision until the user accepts it; the format permits `proposed`, and the loading rule treats `accepted` (or a status-less ADR) as binding. Here, “the change that builds it” can refer to the change creating the ADR itself, while “That change sets the ADR to `accepted` before it merges” then tells an agent to accept an open draft merely to merge its documentation. That can turn an unresolved choice into a binding input for later planning. The writing standard calls for leading with the action and attaching conditions to the action they govern. Say explicitly that the trigger is the implementation change for a user-approved decision, and that an ADR recording an unresolved choice stays proposed; retain the useful requirement to update superseded or amended records in that change. This is a static reading of the instruction; I could not observe an agent follow it. An explicit definition of “the change that builds it” as an implementation change would refute the ambiguity. claim `01M3XYS034F59QPTYBHAEPXDXN` of review `01M3XYGR3KHZXEV5EJVPMTXCDT`

superseded by review 01M3Y07MB5A1RCDDP61X4SJCHF for head c672626bd054c406bcbf7766008c5dee6a50e672

<!-- review:superseded:01M3Y07MB5A1RCDDP61X4SJCHF --> superseded by review `01M3Y07MB5A1RCDDP61X4SJCHF` for head `c672626bd054c406bcbf7766008c5dee6a50e672`
Author
Owner

Already fixed in 71043340d9 via #102: acceptance occurs when the user settles the decision, even before implementation; implementation completion is a separate check.

<!-- gh-feedback:reply-to:101868 --> Already fixed in 71043340d911e11dff997d7fd7af7733b0d468f6 via #102: acceptance occurs when the user settles the decision, even before implementation; implementation completion is a separate check.
jercik marked this conversation as resolved
@ -145,6 +145,7 @@ What qualifies:
## Maintenance rules
- **Update inline, don't batch.** When the user settles a term or a decision mid-session, write it down right then. Deferred documentation doesn't happen. A plan you propose is not a decision until the user accepts it.
- **Ship an ADR accepted with the change that builds it.** That change sets the ADR to `accepted` before it merges and marks each older record it supersedes or amends; an amended record keeps its status and gains a note naming the amending ADR.

medium — Clarify when a draft ADR becomes accepted
lens writing-quality · arm default · tally 1 valid / 0 invalid / 0 uncertain

I read the full project-docs skill and traced this maintenance rule against its ADR format and loading rules. The preceding rule says a proposed plan is not a decision until the user accepts it; the format permits proposed, and the loading rule treats accepted (or a status-less ADR) as binding. Here, “the change that builds it” can refer to the change creating the ADR itself, while “That change sets the ADR to accepted before it merges” then tells an agent to accept an open draft merely to merge its documentation. That can turn an unresolved choice into a binding input for later planning. The writing standard calls for leading with the action and attaching conditions to the action they govern. Say explicitly that the trigger is the implementation change for a user-approved decision, and that an ADR recording an unresolved choice stays proposed; retain the useful requirement to update superseded or amended records in that change. This is a static reading of the instruction; I could not observe an agent follow it. An explicit definition of “the change that builds it” as an implementation change would refute the ambiguity.

claim 01M3XYS034F59QPTYBHAEPXDXN of review 01M3XYGR3KHZXEV5EJVPMTXCDT

<!-- review:claim:01M3XYS034F59QPTYBHAEPXDXN --> **medium** — Clarify when a draft ADR becomes accepted lens `writing-quality` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain > I read the full project-docs skill and traced this maintenance rule against its ADR format and loading rules. The preceding rule says a proposed plan is not a decision until the user accepts it; the format permits `proposed`, and the loading rule treats `accepted` (or a status-less ADR) as binding. Here, “the change that builds it” can refer to the change creating the ADR itself, while “That change sets the ADR to `accepted` before it merges” then tells an agent to accept an open draft merely to merge its documentation. That can turn an unresolved choice into a binding input for later planning. The writing standard calls for leading with the action and attaching conditions to the action they govern. Say explicitly that the trigger is the implementation change for a user-approved decision, and that an ADR recording an unresolved choice stays proposed; retain the useful requirement to update superseded or amended records in that change. This is a static reading of the instruction; I could not observe an agent follow it. An explicit definition of “the change that builds it” as an implementation change would refute the ambiguity. claim `01M3XYS034F59QPTYBHAEPXDXN` of review `01M3XYGR3KHZXEV5EJVPMTXCDT`

superseded by review 01M3Y07MB5A1RCDDP61X4SJCHF for head c672626bd054c406bcbf7766008c5dee6a50e672

<!-- review:superseded:01M3Y07MB5A1RCDDP61X4SJCHF --> superseded by review `01M3Y07MB5A1RCDDP61X4SJCHF` for head `c672626bd054c406bcbf7766008c5dee6a50e672`
Author
Owner

Already fixed in 71043340d9 via #102: acceptance occurs when the user settles the decision, even before implementation; implementation completion is a separate check.

<!-- gh-feedback:reply-to:101868 --> Already fixed in 71043340d911e11dff997d7fd7af7733b0d468f6 via #102: acceptance occurs when the user settles the decision, even before implementation; implementation completion is a separate check.
jercik marked this conversation as resolved
fix(skills): keep only current ADRs
All checks were successful
commit-msg / commitlint (pull_request) Successful in 22s
Node tests / node:test (pull_request) Successful in 1m31s
Review / Review (pull_request_target) Successful in 6m54s
c672626bd0
jercik changed title from fix(project-docs): ship an ADR accepted with the change that builds it to fix(skills): keep only current ADRs 2026-10-02 09:48:24 +00:00
@ -58,3 +58,3 @@
- **Use the glossary's canonical terms** in everything you produce — plans, code, commit messages. When the user's wording conflicts with a defined term, flag the mismatch instead of silently adopting either side.
- **Don't re-litigate recorded ADRs.** A status-less ADR counts as accepted; only an ADR marked `proposed` is still open, and a `deprecated` or `superseded` one no longer binds — its successor, where one exists, governs instead. If the plan contradicts a binding ADR, name it ("this conflicts with ADR-0007") and either adjust the plan or supersede the ADR — never silently override a recorded decision.
- **Don't re-litigate recorded ADRs.** A status-less ADR counts as accepted; only an ADR marked `proposed` is still open. An ADR with any other status, such as `superseded` or `deprecated`, is outdated and does not bind: delete it per the maintenance rules. If the plan contradicts an accepted ADR, name it ("this conflicts with ADR-0007"), then either adjust the plan or, on the user's decision, change the ADR per the maintenance rules — never silently override a recorded decision.

medium — Clarify that accepted ADRs still bind
lens writing-quality · arm default · tally 1 valid / 0 invalid / 0 uncertain

I read the full project-docs skill and its reengineer-program and verify-doc-drift callers. This sentence follows “only an ADR marked proposed is still open,” so “any other status” includes accepted and instructs the reader to treat it as outdated and delete it. The same paragraph says accepted ADRs constrain plans, and verify-doc-drift calls them the source of truth. An agent following the broad wording could discard a live decision or ignore it during planning. This is a static wording conflict; no execution was involved. State the cases explicitly: accepted and status-less ADRs bind, proposed ADRs remain open, and superseded/deprecated ADRs are removed under the maintenance rule. That preserves the new current-decisions policy while excluding accepted records from deletion. A rule explicitly classifying accepted as binding in this sentence would refute the concern.

claim 01M3Y0C3YESESRCDPX1GKZBJ2B of review 01M3Y07MB5A1RCDDP61X4SJCHF

<!-- review:claim:01M3Y0C3YESESRCDPX1GKZBJ2B --> **medium** — Clarify that accepted ADRs still bind lens `writing-quality` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain > I read the full project-docs skill and its reengineer-program and verify-doc-drift callers. This sentence follows “only an ADR marked `proposed` is still open,” so “any other status” includes `accepted` and instructs the reader to treat it as outdated and delete it. The same paragraph says accepted ADRs constrain plans, and verify-doc-drift calls them the source of truth. An agent following the broad wording could discard a live decision or ignore it during planning. This is a static wording conflict; no execution was involved. State the cases explicitly: `accepted` and status-less ADRs bind, `proposed` ADRs remain open, and `superseded`/`deprecated` ADRs are removed under the maintenance rule. That preserves the new current-decisions policy while excluding accepted records from deletion. A rule explicitly classifying `accepted` as binding in this sentence would refute the concern. claim `01M3Y0C3YESESRCDPX1GKZBJ2B` of review `01M3Y07MB5A1RCDDP61X4SJCHF`

superseded by review 01M3Y3EMSCR0EWR8CFG2FV7Z2Q for head d2e1564536a5f4cd367bf7e660375f3ec10bcf02

<!-- review:superseded:01M3Y3EMSCR0EWR8CFG2FV7Z2Q --> superseded by review `01M3Y3EMSCR0EWR8CFG2FV7Z2Q` for head `d2e1564536a5f4cd367bf7e660375f3ec10bcf02`
Author
Owner

Already fixed in 71043340d9 via merged followup #102: the loading rule explicitly classifies accepted and statusless ADRs as binding, proposed as open, and only other statuses as outdated. Rechecked the live source.

<!-- gh-feedback:reply-to:102019 --> Already fixed in 71043340d911e11dff997d7fd7af7733b0d468f6 via merged followup #102: the loading rule explicitly classifies accepted and statusless ADRs as binding, proposed as open, and only other statuses as outdated. Rechecked the live source.
jercik marked this conversation as resolved
@ -145,6 +145,7 @@ What qualifies:
## Maintenance rules
- **Update inline, don't batch.** When the user settles a term or a decision mid-session, write it down right then. Deferred documentation doesn't happen. A plan you propose is not a decision until the user accepts it.
- **Ship an ADR accepted with the change that builds it.** That change sets the ADR to `accepted` before it merges and marks each older record it supersedes or amends; an amended record keeps its status and gains a note naming the amending ADR.

medium — Clarify when a draft ADR becomes accepted
lens writing-quality · arm default · tally 1 valid / 0 invalid / 0 uncertain

I read the full project-docs skill and traced this maintenance rule against its ADR format and loading rules. The preceding rule says a proposed plan is not a decision until the user accepts it; the format permits proposed, and the loading rule treats accepted (or a status-less ADR) as binding. Here, “the change that builds it” can refer to the change creating the ADR itself, while “That change sets the ADR to accepted before it merges” then tells an agent to accept an open draft merely to merge its documentation. That can turn an unresolved choice into a binding input for later planning. The writing standard calls for leading with the action and attaching conditions to the action they govern. Say explicitly that the trigger is the implementation change for a user-approved decision, and that an ADR recording an unresolved choice stays proposed; retain the useful requirement to update superseded or amended records in that change. This is a static reading of the instruction; I could not observe an agent follow it. An explicit definition of “the change that builds it” as an implementation change would refute the ambiguity.

claim 01M3XYS034F59QPTYBHAEPXDXN of review 01M3XYGR3KHZXEV5EJVPMTXCDT

<!-- review:claim:01M3XYS034F59QPTYBHAEPXDXN --> **medium** — Clarify when a draft ADR becomes accepted lens `writing-quality` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain > I read the full project-docs skill and traced this maintenance rule against its ADR format and loading rules. The preceding rule says a proposed plan is not a decision until the user accepts it; the format permits `proposed`, and the loading rule treats `accepted` (or a status-less ADR) as binding. Here, “the change that builds it” can refer to the change creating the ADR itself, while “That change sets the ADR to `accepted` before it merges” then tells an agent to accept an open draft merely to merge its documentation. That can turn an unresolved choice into a binding input for later planning. The writing standard calls for leading with the action and attaching conditions to the action they govern. Say explicitly that the trigger is the implementation change for a user-approved decision, and that an ADR recording an unresolved choice stays proposed; retain the useful requirement to update superseded or amended records in that change. This is a static reading of the instruction; I could not observe an agent follow it. An explicit definition of “the change that builds it” as an implementation change would refute the ambiguity. claim `01M3XYS034F59QPTYBHAEPXDXN` of review `01M3XYGR3KHZXEV5EJVPMTXCDT`

superseded by review 01M3Y07MB5A1RCDDP61X4SJCHF for head c672626bd054c406bcbf7766008c5dee6a50e672

<!-- review:superseded:01M3Y07MB5A1RCDDP61X4SJCHF --> superseded by review `01M3Y07MB5A1RCDDP61X4SJCHF` for head `c672626bd054c406bcbf7766008c5dee6a50e672`
Author
Owner

Already fixed in 71043340d9 via #102: acceptance occurs when the user settles the decision, even before implementation; implementation completion is a separate check.

<!-- gh-feedback:reply-to:101869 --> Already fixed in 71043340d911e11dff997d7fd7af7733b0d468f6 via #102: acceptance occurs when the user settles the decision, even before implementation; implementation completion is a separate check.
jercik marked this conversation as resolved
fix(project-docs): flag outdated ADRs instead of deleting them on sight
All checks were successful
commit-msg / commitlint (pull_request) Successful in 24s
Node tests / node:test (pull_request) Successful in 2m1s
Review / Review (pull_request_target) Successful in 5m23s
d2e1564536
@ -146,3 +146,3 @@
- **Update inline, don't batch.** When the user settles a term or a decision mid-session, write it down right then. Deferred documentation doesn't happen. A plan you propose is not a decision until the user accepts it.
- **Supersede living decisions; delete dead ones.** When a decision is reversed but the area it governs still exists, write a new ADR and mark the old one `superseded by ADR-NNNN` — the old rationale still explains why the code looked the way it did. But when a refactor removes the subject of a decision entirely, edit or delete the ADR: a record about code that no longer exists only misleads, and version control remembers. Leave numbering gaps as-is after a deletion — renumbering breaks `superseded by` references.
- **Ship an ADR accepted with the change that builds it.** That change sets a `proposed` ADR to `accepted` before it merges and edits or deletes each older ADR whose decision it changes, per the next rule.

low — project-docs gives two incompatible rules for when an ADR becomes accepted (on user settlement vs. on the merge that builds it)
lens general-bug · arm default · tally 1 valid / 0 invalid / 0 uncertain

Examined: skills/project-docs/SKILL.md (ADR format and Maintenance rules), and the skills that defer to it: grill-with-docs/SKILL.md, verify-doc-drift/SKILL.md, reengineer-program/SKILL.md.

The change narrows the optional Status field to proposed | accepted and describes it as used "when a decision is recorded before it is settled", i.e. proposed means not yet settled by the user. The maintenance rule right above the new one says "When the user settles a term or a decision mid-session, write it down right then" and "A plan you propose is not a decision until the user accepts it", so acceptance is a user act. The newly added rule (anchored) instead ties the proposed -> accepted flip to the code change that builds the decision: "That change sets a proposed ADR to accepted before it merges".

These disagree for the common case this skill family produces: a decision the user settles in a grill-with-docs session (which says it updates ADRs "inline as decisions crystallise") but that nobody has built yet. Under the Status description it should be written accepted (or status-less, which also "counts as accepted"); under the new ship rule it stays proposed until the building change merges. The two choices have different downstream consequences in other skills:

  • verify-doc-drift: an accepted or status-less ADR contradicted by code is reported as code-drift, so an accepted-but-unbuilt decision gets flagged as drift; a proposed ADR is instead "an open question: correct its statement of current behaviour if the code contradicts it", so the audit would rewrite the settled-but-unbuilt ADR's context to describe the old code.
  • project-docs' own loading rule: "only an ADR marked proposed is still open", so keeping a user-settled decision proposed per the ship rule invites re-litigating it.

reengineer-program sidesteps this only because it declares its own ADR rules take precedence and accepts on the user's answer ("Replace proposed text with the accepted decision when answered"; "Recording decisions without changing code is a valid outcome"), which itself contradicts the new ship rule for anyone reading project-docs alone.

This is static reading of the instruction text; no agent run was performed. A fix is to state one trigger, e.g. "set accepted when the user settles it; the change that builds it must not merge while its ADR is still proposed", or to say explicitly that unbuilt settled decisions stay proposed and update the Status description and verify-doc-drift's proposed handling to match.

claim 01M3Y3JB71C911P0WEN71CSYNZ of review 01M3Y3EMSCR0EWR8CFG2FV7Z2Q

<!-- review:claim:01M3Y3JB71C911P0WEN71CSYNZ --> **low** — project-docs gives two incompatible rules for when an ADR becomes `accepted` (on user settlement vs. on the merge that builds it) lens `general-bug` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain > Examined: skills/project-docs/SKILL.md (ADR format and Maintenance rules), and the skills that defer to it: grill-with-docs/SKILL.md, verify-doc-drift/SKILL.md, reengineer-program/SKILL.md. > > The change narrows the optional Status field to `proposed | accepted` and describes it as used "when a decision is recorded before it is settled", i.e. `proposed` means not yet settled by the user. The maintenance rule right above the new one says "When the user settles a term or a decision mid-session, write it down right then" and "A plan you propose is not a decision until the user accepts it", so acceptance is a user act. The newly added rule (anchored) instead ties the `proposed` -> `accepted` flip to the code change that builds the decision: "That change sets a `proposed` ADR to `accepted` before it merges". > > These disagree for the common case this skill family produces: a decision the user settles in a grill-with-docs session (which says it updates ADRs "inline as decisions crystallise") but that nobody has built yet. Under the Status description it should be written `accepted` (or status-less, which also "counts as accepted"); under the new ship rule it stays `proposed` until the building change merges. The two choices have different downstream consequences in other skills: > - verify-doc-drift: an `accepted` or status-less ADR contradicted by code is reported as `code-drift`, so an accepted-but-unbuilt decision gets flagged as drift; a `proposed` ADR is instead "an open question: correct its statement of current behaviour if the code contradicts it", so the audit would rewrite the settled-but-unbuilt ADR's context to describe the old code. > - project-docs' own loading rule: "only an ADR marked `proposed` is still open", so keeping a user-settled decision `proposed` per the ship rule invites re-litigating it. > > reengineer-program sidesteps this only because it declares its own ADR rules take precedence and accepts on the user's answer ("Replace proposed text with the accepted decision when answered"; "Recording decisions without changing code is a valid outcome"), which itself contradicts the new ship rule for anyone reading project-docs alone. > > This is static reading of the instruction text; no agent run was performed. A fix is to state one trigger, e.g. "set `accepted` when the user settles it; the change that builds it must not merge while its ADR is still `proposed`", or to say explicitly that unbuilt settled decisions stay `proposed` and update the Status description and verify-doc-drift's `proposed` handling to match. claim `01M3Y3JB71C911P0WEN71CSYNZ` of review `01M3Y3EMSCR0EWR8CFG2FV7Z2Q`
Author
Owner

Already fixed in 71043340d9 via merged followup #102: Accept when settled now explicitly accepts on the user decision before implementation and distinguishes acceptance from implementation completion. Rechecked the live source; the contradictory ship-time trigger is gone.

<!-- gh-feedback:reply-to:102381 --> Already fixed in 71043340d911e11dff997d7fd7af7733b0d468f6 via merged followup #102: Accept when settled now explicitly accepts on the user decision before implementation and distinguishes acceptance from implementation completion. Rechecked the live source; the contradictory ship-time trigger is gone.
jercik marked this conversation as resolved
@ -147,2 +147,3 @@
- **Update inline, don't batch.** When the user settles a term or a decision mid-session, write it down right then. Deferred documentation doesn't happen. A plan you propose is not a decision until the user accepts it.
- **Supersede living decisions; delete dead ones.** When a decision is reversed but the area it governs still exists, write a new ADR and mark the old one `superseded by ADR-NNNN` — the old rationale still explains why the code looked the way it did. But when a refactor removes the subject of a decision entirely, edit or delete the ADR: a record about code that no longer exists only misleads, and version control remembers. Leave numbering gaps as-is after a deletion — renumbering breaks `superseded by` references.
- **Ship an ADR accepted with the change that builds it.** That change sets a `proposed` ADR to `accepted` before it merges and edits or deletes each older ADR whose decision it changes, per the next rule.
- **Keep only current decisions.** When a decision changes, edit its ADR to state the new decision. When a new decision replaces it outright, delete the old ADR and add a new one. Delete an ADR whose decision no longer applies. After deleting an ADR, update or remove every link to it. Never keep an outdated ADR marked superseded or deprecated, or annotated with a note pointing to another ADR, and never write "amends ADR-X" or "supersedes ADR-X" in the newer one: each ADR states its own decision, and version control keeps the history. When the reason the old approach was dropped is worth keeping, record it under the current ADR's Considered Options.

low — "Keep only current decisions" offers edit-in-place vs delete-and-renumber with no usable criterion between "changes" and "replaces outright"
lens writing-quality · arm default · tally 1 valid / 0 invalid / 0 uncertain

What I examined: the "Keep only current decisions" maintenance rule in skills/project-docs/SKILL.md, the ADR numbering rule in the same file, and the skills that defer to it. reengineer-program says "edit or replace the ADR per project-docs", implementation.md says "a new or edited ADR", and verify-doc-drift says "the ADR is edited, replaced, or deleted per project-docs".

What the text says: the rule gives two different procedures for a changed decision. One is to edit in place and keep the number. The other is to delete and add, which takes a new number, leaves a gap, and requires "update or remove every link to it". The choice depends on whether the decision "changes" or whether "a new decision replaces it outright". The text never says what separates the two. Every caller hands this choice back to project-docs, so this rule is the only place it gets made.

What goes wrong: a decision that changes is, in effect, replaced by a new decision. An agent cannot tell which procedure applies, so it picks one at random. The two outcomes differ in ways that matter later. With an in-place edit, existing references to ADR-NNNN in commits, PR titles, and conversations now point at a decision they never meant. reengineer-program's implementation.md depends on those references: "Every subtracting commit's subject names each executed ADR, such as ADR-0007", and unexecuted drops are found by "inspect[ing] subtraction commits naming the ADR". If a drop ADR is edited into a different decision under the same number, an old commit can make the new decision look executed. Delete-and-add avoids that but adds the link-update work. The skill's precise-language guidance asks for one term per concept and an operational criterion. "Changes" versus "replaces outright" is a label that changes no behaviour, because the reader cannot apply it.

Proposed correction: state the criterion in observable terms. One option: "Edit an ADR in place when the decision keeps its subject and only its details change (a limit, a named tool). When the chosen option itself changes, delete the old ADR and add one under a new number, so commits that cite the old number still refer to the decision they executed." This keeps both procedures and the history-in-git principle, and tells the agent which one to use.

What would refute this: a passage elsewhere that defines when to edit and when to replace. I found none in project-docs or in the four skills that defer to it.

claim 01M3Y3KC6JVHWZ9HXHWHZVC25X of review 01M3Y3EMSCR0EWR8CFG2FV7Z2Q

<!-- review:claim:01M3Y3KC6JVHWZ9HXHWHZVC25X --> **low** — "Keep only current decisions" offers edit-in-place vs delete-and-renumber with no usable criterion between "changes" and "replaces outright" lens `writing-quality` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain > What I examined: the "Keep only current decisions" maintenance rule in skills/project-docs/SKILL.md, the ADR numbering rule in the same file, and the skills that defer to it. reengineer-program says "edit or replace the ADR per `project-docs`", implementation.md says "a new or edited ADR", and verify-doc-drift says "the ADR is edited, replaced, or deleted per `project-docs`". > > What the text says: the rule gives two different procedures for a changed decision. One is to edit in place and keep the number. The other is to delete and add, which takes a new number, leaves a gap, and requires "update or remove every link to it". The choice depends on whether the decision "changes" or whether "a new decision replaces it outright". The text never says what separates the two. Every caller hands this choice back to project-docs, so this rule is the only place it gets made. > > What goes wrong: a decision that changes is, in effect, replaced by a new decision. An agent cannot tell which procedure applies, so it picks one at random. The two outcomes differ in ways that matter later. With an in-place edit, existing references to ADR-NNNN in commits, PR titles, and conversations now point at a decision they never meant. reengineer-program's implementation.md depends on those references: "Every subtracting commit's subject names each executed ADR, such as `ADR-0007`", and unexecuted drops are found by "inspect[ing] subtraction commits naming the ADR". If a drop ADR is edited into a different decision under the same number, an old commit can make the new decision look executed. Delete-and-add avoids that but adds the link-update work. The skill's precise-language guidance asks for one term per concept and an operational criterion. "Changes" versus "replaces outright" is a label that changes no behaviour, because the reader cannot apply it. > > Proposed correction: state the criterion in observable terms. One option: "Edit an ADR in place when the decision keeps its subject and only its details change (a limit, a named tool). When the chosen option itself changes, delete the old ADR and add one under a new number, so commits that cite the old number still refer to the decision they executed." This keeps both procedures and the history-in-git principle, and tells the agent which one to use. > > What would refute this: a passage elsewhere that defines when to edit and when to replace. I found none in project-docs or in the four skills that defer to it. > claim `01M3Y3KC6JVHWZ9HXHWHZVC25X` of review `01M3Y3EMSCR0EWR8CFG2FV7Z2Q`
Author
Owner

Declining: the edit-or-replace split is the user's decision for this PR. A changed decision edits its ADR, and a decision replaced outright gets a new ADR while the old one is deleted. The proposed criterion would turn most changes into delete-and-add, which reverses that default. Nothing is renumbered: a new ADR takes the next unused number, and a deleted one leaves a gap (ADR format section). An old commit citing ADR-0007 after an in-place edit follows from the same decision: ADRs state only the current decision, and git keeps the version that commit executed.

<!-- gh-feedback:reply-to:102382 --> Declining: the edit-or-replace split is the user's decision for this PR. A changed decision edits its ADR, and a decision replaced outright gets a new ADR while the old one is deleted. The proposed criterion would turn most changes into delete-and-add, which reverses that default. Nothing is renumbered: a new ADR takes the next unused number, and a deleted one leaves a gap (ADR format section). An old commit citing `ADR-0007` after an in-place edit follows from the same decision: ADRs state only the current decision, and git keeps the version that commit executed.

superseded by review 01M41KHRHF0D8BVKBP18YFCNQD for head 71043340d911e11dff997d7fd7af7733b0d468f6

<!-- review:superseded:01M41KHRHF0D8BVKBP18YFCNQD --> superseded by review `01M41KHRHF0D8BVKBP18YFCNQD` for head `71043340d911e11dff997d7fd7af7733b0d468f6`
@ -57,3 +57,3 @@
Replace proposed text with the accepted decision when answered. Delete proposed ADRs only when they duplicate another root; retain accepted negatives after their code disappears. Use the same format for design choices, implementation gaps, and review findings. ADRs plus git carry resume state; execution history belongs in commits.
Accepted ADRs remain settled across sessions. Supersede one when a fact undermines its reason, a witness appears for a don't-care drop, or a deferred keep's owner answers. Proposed and status-less ADRs are the decision backlog.
Accepted ADRs remain settled across sessions. When a fact undermines one's reason or a witness appears for a don't-care drop, put it to the user; when the user or a deferred keep's owner answers, edit or replace the ADR per `project-docs`. Proposed ADRs are the decision backlog; status-less ADRs count as accepted.

low — reengineer-program restates project-docs' status-less default, creating a second home that its precedence clause would let drift into an override
lens writing-quality · arm default · tally 1 valid / 0 invalid / 0 uncertain

What I examined: skills/reengineer-program/SKILL.md, which declares project-docs in axskills.requires and tells the agent to "Call the Skill tool with project-docs" and "Follow its loading procedure; this skill's ADR rules take precedence". I also read skills/project-docs/SKILL.md, whose loading bullet says "A status-less ADR counts as accepted; only an ADR marked proposed is still open."

What the text says: this change removed reengineer-program's old override ("Status-less ADRs count as proposed and stay unchanged until answered") and added "status-less ADRs count as accepted" to the end of the backlog paragraph. That is now the same rule project-docs already states, and project-docs is a declared dependency that this skill always loads first.

What goes wrong: the clause is a no-op. writing-for-agents says to "call declared skill dependencies instead of duplicating their instructions", because a fact kept in two homes drifts. The risk is concrete here. This skill says its ADR rules "take precedence", so a reader will treat every ADR statement in it as a deliberate local override. If project-docs later changes its status-less default, the copy here will silently override it. That is the same kind of divergence this change just had to fix. The clause also sits in a paragraph about the backlog, not next to the Open/Keep/Drop status definitions, so it does not work as a local definition either.

Proposed correction: change the sentence to "Proposed ADRs are the decision backlog." and let project-docs carry the status-less default. If reengineer-program really does need to pin that default against future project-docs changes, then present it explicitly as an override beside the "Open (status: proposed)" bullet. Either way, the backlog meaning is kept.

What would refute this: evidence that reengineer-program runs without project-docs loaded. Its metadata and its "Ground the scope" step both require loading it.

claim 01M3Y3KPWDZ9ECKE0X7S2SXXGD of review 01M3Y3EMSCR0EWR8CFG2FV7Z2Q

<!-- review:claim:01M3Y3KPWDZ9ECKE0X7S2SXXGD --> **low** — reengineer-program restates project-docs' status-less default, creating a second home that its precedence clause would let drift into an override lens `writing-quality` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain > What I examined: skills/reengineer-program/SKILL.md, which declares `project-docs` in `axskills.requires` and tells the agent to "Call the Skill tool with `project-docs`" and "Follow its loading procedure; this skill's ADR rules take precedence". I also read skills/project-docs/SKILL.md, whose loading bullet says "A status-less ADR counts as accepted; only an ADR marked `proposed` is still open." > > What the text says: this change removed reengineer-program's old override ("Status-less ADRs count as proposed and stay unchanged until answered") and added "status-less ADRs count as accepted" to the end of the backlog paragraph. That is now the same rule project-docs already states, and project-docs is a declared dependency that this skill always loads first. > > What goes wrong: the clause is a no-op. writing-for-agents says to "call declared skill dependencies instead of duplicating their instructions", because a fact kept in two homes drifts. The risk is concrete here. This skill says its ADR rules "take precedence", so a reader will treat every ADR statement in it as a deliberate local override. If project-docs later changes its status-less default, the copy here will silently override it. That is the same kind of divergence this change just had to fix. The clause also sits in a paragraph about the backlog, not next to the Open/Keep/Drop status definitions, so it does not work as a local definition either. > > Proposed correction: change the sentence to "Proposed ADRs are the decision backlog." and let project-docs carry the status-less default. If reengineer-program really does need to pin that default against future project-docs changes, then present it explicitly as an override beside the "Open (`status: proposed`)" bullet. Either way, the backlog meaning is kept. > > What would refute this: evidence that reengineer-program runs without project-docs loaded. Its metadata and its "Ground the scope" step both require loading it. > claim `01M3Y3KPWDZ9ECKE0X7S2SXXGD` of review `01M3Y3EMSCR0EWR8CFG2FV7Z2Q`
Author
Owner

Already fixed in 71043340d9 via merged followup #102: reengineer-program now says only Proposed ADRs are the decision backlog; the duplicate statusless default was removed. Project-docs remains its declared, loaded dependency.

<!-- gh-feedback:reply-to:102383 --> Already fixed in 71043340d911e11dff997d7fd7af7733b0d468f6 via merged followup #102: reengineer-program now says only Proposed ADRs are the decision backlog; the duplicate statusless default was removed. Project-docs remains its declared, loaded dependency.
jercik marked this conversation as resolved
Author
Owner

Replying to review comment #102381

Real, and deliberately deferred under the review round gate. This is round 6 on this PR (five fix pushes since it opened), where only security, data-corruption, or data-loss bugs are fixed in place. Reviewed at d2e1564.

Follow-up fix, in skills/project-docs/SKILL.md under Maintenance rules: make the user's settlement the one trigger for accepted, and keep the build rule as a merge gate. Replace the "Ship an ADR accepted with the change that builds it" bullet with: "Mark an ADR accepted when the user settles it. The change that builds a decision never merges with its ADR still proposed, and it edits or deletes each older ADR whose decision it changes, per the next rule." verify-doc-drift's proposed handling then needs no change.

> Replying to review comment #102381 Real, and deliberately deferred under the review round gate. This is round 6 on this PR (five fix pushes since it opened), where only security, data-corruption, or data-loss bugs are fixed in place. Reviewed at d2e1564. Follow-up fix, in `skills/project-docs/SKILL.md` under Maintenance rules: make the user's settlement the one trigger for `accepted`, and keep the build rule as a merge gate. Replace the "Ship an ADR accepted with the change that builds it" bullet with: "**Mark an ADR accepted when the user settles it.** The change that builds a decision never merges with its ADR still `proposed`, and it edits or deletes each older ADR whose decision it changes, per the next rule." verify-doc-drift's `proposed` handling then needs no change.
Author
Owner

Replying to review comment #102019

Real, and deliberately deferred under the review round gate. This is round 6 on this PR, where only security, data-corruption, or data-loss bugs are fixed in place. d2e1564 replaced the on-sight deletion with "flag it, and fix it … only when that is the task or the user agrees", so an accepted ADR can no longer be deleted through this sentence. The wording itself remains: "any other status" follows "only an ADR marked proposed is still open", so it can be read as including accepted.

Follow-up fix, in skills/project-docs/SKILL.md under "Don't re-litigate recorded ADRs": change "An ADR with any other status, such as superseded or deprecated," to "An ADR with any status other than accepted or proposed, such as superseded or deprecated,".

> Replying to review comment #102019 Real, and deliberately deferred under the review round gate. This is round 6 on this PR, where only security, data-corruption, or data-loss bugs are fixed in place. d2e1564 replaced the on-sight deletion with "flag it, and fix it … only when that is the task or the user agrees", so an accepted ADR can no longer be deleted through this sentence. The wording itself remains: "any other status" follows "only an ADR marked `proposed` is still open", so it can be read as including `accepted`. Follow-up fix, in `skills/project-docs/SKILL.md` under "Don't re-litigate recorded ADRs": change "An ADR with any other status, such as `superseded` or `deprecated`," to "An ADR with any status other than `accepted` or `proposed`, such as `superseded` or `deprecated`,".
Author
Owner

Replying to review comment #101868

Real, and deliberately deferred under the review round gate (round 6 on this PR). c672626 made proposed mean a decision "recorded before it is settled", but "Ship an ADR accepted with the change that builds it" still reads as the acceptance trigger. The current head re-raises the same ambiguity as #102381. The follow-up fix named there settles this one too: in skills/project-docs/SKILL.md, accept an ADR when the user settles it, and bar the change that builds it from merging while the ADR is still proposed.

> Replying to review comment #101868 Real, and deliberately deferred under the review round gate (round 6 on this PR). c672626 made `proposed` mean a decision "recorded before it is settled", but "Ship an ADR accepted with the change that builds it" still reads as the acceptance trigger. The current head re-raises the same ambiguity as #102381. The follow-up fix named there settles this one too: in `skills/project-docs/SKILL.md`, accept an ADR when the user settles it, and bar the change that builds it from merging while the ADR is still `proposed`.
Author
Owner

Replying to review comment #101869

Duplicate posting of claim 01M3XYS034F59QPTYBHAEPXDXN, handled at #101868: real, deferred under the round gate (round 6 on this PR) to the follow-up fix named at #102381, in skills/project-docs/SKILL.md.

> Replying to review comment #101869 Duplicate posting of claim `01M3XYS034F59QPTYBHAEPXDXN`, handled at #101868: real, deferred under the round gate (round 6 on this PR) to the follow-up fix named at #102381, in `skills/project-docs/SKILL.md`.
Author
Owner

Replying to review comment #102383

Real, and deliberately deferred under the review round gate. This is round 6 on this PR, where only security, data-corruption, or data-loss bugs are fixed in place. Reviewed at d2e1564.

Follow-up fix, in skills/reengineer-program/SKILL.md (line 59): change "Proposed ADRs are the decision backlog; status-less ADRs count as accepted." to "Proposed ADRs are the decision backlog." project-docs, which this skill loads first, owns the status-less default.

> Replying to review comment #102383 Real, and deliberately deferred under the review round gate. This is round 6 on this PR, where only security, data-corruption, or data-loss bugs are fixed in place. Reviewed at d2e1564. Follow-up fix, in `skills/reengineer-program/SKILL.md` (line 59): change "Proposed ADRs are the decision backlog; status-less ADRs count as accepted." to "Proposed ADRs are the decision backlog." project-docs, which this skill loads first, owns the status-less default.
fix: accept settled ADR decisions and verify every added or edited record (#102)
All checks were successful
commit-msg / commitlint (pull_request) Successful in 17s
Node tests / node:test (pull_request) Successful in 1m19s
Review / Review (pull_request_target) Successful in 8m41s
71043340d9
Follows [#98](#98) by accepting settled decisions before implementation and verifying every added or edited ADR against its relevant evidence.

Merge this follow-up into #98 before integrating #98 with `main`.

Reviewed-on: #102
@ -28,3 +28,3 @@
**Cross-reference with code.** When the user states how something works, check whether the code agrees, and surface any contradiction: "Your code cancels entire Orders, but you just said partial cancellation is possible — which is right?"
**Name ADR conflicts.** When the plan contradicts a recorded ADR, say so explicitly — "this conflicts with ADR-0007" — and force the choice: adjust the plan, or supersede the ADR per the `project-docs` skill. Never let a plan silently override a recorded decision.
**Name ADR conflicts.** When the plan contradicts an accepted ADR, say so explicitly — "this conflicts with ADR-0007" — and force the choice: adjust the plan, or change the ADR per the `project-docs` skill. Never let a plan silently override a recorded decision.

high — Grilling instruction copies ADR conflict choices

I read the changed conflict instruction in skills/grill-with-docs/SKILL.md and its explicit dependency, skills/project-docs/SKILL.md. Project-docs defines the resolution set: If the plan contradicts an accepted ADR, name it ("this conflicts with ADR-0007"), then either adjust the plan or, on the user's decision, change the ADR per the maintenance rules. Grill-with-docs repeats those two actions as adjust the plan, or change the ADR. The copy currently agrees, but if the canonical conflict procedure changes, the grilling instruction can keep presenting an obsolete or incomplete choice. This is a live instruction, the dependency is available to its reader, and the two actions are presented as the choice rather than examples; no exception applies. Replace the action list with name the conflict and follow the conflict-resolution rule in project-docs, while retaining the grilling-specific requirement to surface the conflict. The quoted project-docs rule defines the set; no execution test is needed.

lens restated-sets · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41KTMJ6EPCTFE5M0X7ZYYND of review 01M41KHRHF0D8BVKBP18YFCNQD

<!-- review:claim:01M41KTMJ6EPCTFE5M0X7ZYYND --> **high** — Grilling instruction copies ADR conflict choices > I read the changed conflict instruction in skills/grill-with-docs/SKILL.md and its explicit dependency, skills/project-docs/SKILL.md. Project-docs defines the resolution set: `If the plan contradicts an accepted ADR, name it ("this conflicts with ADR-0007"), then either adjust the plan or, on the user's decision, change the ADR per the maintenance rules`. Grill-with-docs repeats those two actions as `adjust the plan, or change the ADR`. The copy currently agrees, but if the canonical conflict procedure changes, the grilling instruction can keep presenting an obsolete or incomplete choice. This is a live instruction, the dependency is available to its reader, and the two actions are presented as the choice rather than examples; no exception applies. Replace the action list with `name the conflict and follow the conflict-resolution rule in project-docs`, while retaining the grilling-specific requirement to surface the conflict. The quoted project-docs rule defines the set; no execution test is needed. lens `restated-sets` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41KTMJ6EPCTFE5M0X7ZYYND` of review `01M41KHRHF0D8BVKBP18YFCNQD`
Author
Owner

Fixed in babf98cbbc while repairing the binding-ADR guard: grill-with-docs names the conflict and delegates resolution to its declared and explicitly loaded project-docs dependency.

<!-- gh-feedback:reply-to:109915 --> Fixed in babf98cbbc3f37c673af24c448ccd89d9750191f while repairing the binding-ADR guard: grill-with-docs names the conflict and delegates resolution to its declared and explicitly loaded project-docs dependency.
jercik marked this conversation as resolved
@ -58,3 +58,3 @@
- **Use the glossary's canonical terms** in everything you produce — plans, code, commit messages. When the user's wording conflicts with a defined term, flag the mismatch instead of silently adopting either side.
- **Don't re-litigate recorded ADRs.** A status-less ADR counts as accepted; only an ADR marked `proposed` is still open, and a `deprecated` or `superseded` one no longer binds — its successor, where one exists, governs instead. If the plan contradicts a binding ADR, name it ("this conflicts with ADR-0007") and either adjust the plan or supersede the ADR — never silently override a recorded decision.
- **Don't re-litigate recorded ADRs.** An ADR marked `accepted`, or without a status, is decided and binding; `proposed` means the decision is still open. Other statuses, including `superseded` and `deprecated`, are outdated and do not bind: flag them and use the maintenance rules when that cleanup is authorized. If the plan contradicts an accepted ADR, name it ("this conflicts with ADR-0007"), then either adjust the plan or, on the user's decision, change the ADR per the maintenance rules — never silently override a recorded decision.

medium — Conflict rule omits binding ADRs without a status

I read the full project-docs, grill-with-docs, and verify-doc-drift skills. This paragraph declares an ADR without a status binding, but its operative conflict rule says to name a conflict only with an accepted ADR. The changed grill-with-docs conflict instruction uses the same narrower phrase, while verify-doc-drift explicitly treats an accepted or status-less ADR as authoritative. An agent interviewing a plan against a status-less ADR can therefore treat that decision as outside the conflict check and fail to surface the contradiction, despite the first sentence. Use binding ADR in the conflict instruction and in grill-with-docs, referring to the accepted-or-status-less definition here; this preserves the distinction from proposed and outdated ADRs. The decisive counterevidence would be a convention in these skills that accepted ADR includes an ADR with no status; the explicit contrast in verify-doc-drift points the other way. This is a static wording trace, not a runtime reproduction.

lens writing-quality · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41KPHNVTAJEHTG2049AVGTK of review 01M41KHRHF0D8BVKBP18YFCNQD

<!-- review:claim:01M41KPHNVTAJEHTG2049AVGTK --> **medium** — Conflict rule omits binding ADRs without a status > I read the full project-docs, grill-with-docs, and verify-doc-drift skills. This paragraph declares an ADR without a status binding, but its operative conflict rule says to name a conflict only with an accepted ADR. The changed grill-with-docs conflict instruction uses the same narrower phrase, while verify-doc-drift explicitly treats an accepted or status-less ADR as authoritative. An agent interviewing a plan against a status-less ADR can therefore treat that decision as outside the conflict check and fail to surface the contradiction, despite the first sentence. Use binding ADR in the conflict instruction and in grill-with-docs, referring to the accepted-or-status-less definition here; this preserves the distinction from proposed and outdated ADRs. The decisive counterevidence would be a convention in these skills that accepted ADR includes an ADR with no status; the explicit contrast in verify-doc-drift points the other way. This is a static wording trace, not a runtime reproduction. lens `writing-quality` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41KPHNVTAJEHTG2049AVGTK` of review `01M41KHRHF0D8BVKBP18YFCNQD`
Author
Owner

Fixed in babf98cbbc: both project-docs and grill-with-docs now check conflicts against binding ADRs, covering explicit accepted and statusless records. The shared rule retains the user decision gate; accepted-but-unimplemented targets, proposed choices, and whole-record verification are unchanged.

<!-- gh-feedback:reply-to:109919 --> Fixed in babf98cbbc3f37c673af24c448ccd89d9750191f: both project-docs and grill-with-docs now check conflicts against binding ADRs, covering explicit accepted and statusless records. The shared rule retains the user decision gate; accepted-but-unimplemented targets, proposed choices, and whole-record verification are unchanged.
jercik marked this conversation as resolved
@ -31,3 +31,3 @@
Have a fresh-context sub-agent attack the result from both directions: find situations the old code handles that the rebuild omits without an accepted decision, and promises the rebuild makes that no accepted decision covers. Mechanism-only differences and replacements chosen by an accepted ADR are not findings.
Put each uncovered situation to the user and record the answer in an added or superseding ADR before adding, removing, or keeping its promise. Completion requires the implementation to satisfy the confirmed design, all three checks to pass, and every review finding to be resolved.
Put each uncovered situation to the user and record the answer in a new or edited ADR before adding, removing, or keeping its promise. Completion requires the implementation to satisfy the confirmed design, all three checks to pass, and every review finding to be resolved.

high — Completion rule repeats the verification-check count

I read the changed completion sentence and the Verify section of this file, then checked skills/reengineering/SKILL.md, which reengineer-program invokes. The latter defines the set with Use three checks: followed by Retained promises, Dropped promises, and Replaced or added promises; this reference also defines its verification contracts under Check three distinct contracts: with entries for unchanged, dropped, and replaced or added promises. The changed completion sentence copies the size as all three checks. If a verification contract is added or removed, this count can silently disagree with the steps the agent must run. No exception applies: this is a current procedural instruction, the reader can open the defining skill and section, and the number is only the set's size, not a capacity argument. Replace all three checks to pass with the verification checks defined above to pass (or point to the defining reengineering skill). The quoted definitions establish the set; no execution behavior is needed to decide this claim.

lens restated-sets · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41KQ42VEQ6TN8VHVXFBWXTP of review 01M41KHRHF0D8BVKBP18YFCNQD

<!-- review:claim:01M41KQ42VEQ6TN8VHVXFBWXTP --> **high** — Completion rule repeats the verification-check count > I read the changed completion sentence and the Verify section of this file, then checked skills/reengineering/SKILL.md, which reengineer-program invokes. The latter defines the set with `Use three checks:` followed by `Retained promises`, `Dropped promises`, and `Replaced or added promises`; this reference also defines its verification contracts under `Check three distinct contracts:` with entries for unchanged, dropped, and replaced or added promises. The changed completion sentence copies the size as `all three checks`. If a verification contract is added or removed, this count can silently disagree with the steps the agent must run. No exception applies: this is a current procedural instruction, the reader can open the defining skill and section, and the number is only the set's size, not a capacity argument. Replace `all three checks to pass` with `the verification checks defined above to pass` (or point to the defining reengineering skill). The quoted definitions establish the set; no execution behavior is needed to decide this claim. lens `restated-sets` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41KQ42VEQ6TN8VHVXFBWXTP` of review `01M41KHRHF0D8BVKBP18YFCNQD`
jercik marked this conversation as resolved
@ -35,3 +35,3 @@
## Fix direction and missing features
Fix docs to match code by default. Change code only for a confirmed **code-drift** where the doc is the intended source of truth. A third case hides inside "incorrect": the doc describes a capability that _should_ exist but doesn't (an endpoint, a flag). Removing the claim and _implementing_ the feature are both valid — surface the fork to the user, and if the claim is removed, flag the missing feature as separate work rather than silently dropping it. ADRs are decision records, not behaviour docs. A `proposed` ADR is an open question: correct its statement of current behaviour if the code contradicts it, never delete it as stale. An `accepted` ADR is the source of truth for the decision it records: code contradicting it is `code-drift`, and changing the decision means superseding the ADR per `project-docs`, never editing it to match the code.
Fix docs to match code by default. Change code only for a confirmed **code-drift** where the doc is the intended source of truth. A third case hides inside "incorrect": the doc describes a capability that _should_ exist but doesn't (an endpoint, a flag). Removing the claim and _implementing_ the feature are both valid — surface the fork to the user, and if the claim is removed, flag the missing feature as separate work rather than silently dropping it. ADRs are decision records, not behaviour docs. A `proposed` ADR is an open question: correct its statement of current behaviour if the code contradicts it, never delete it as stale. An `accepted` or status-less ADR is the source of truth for the decision it records: code contradicting it is `code-drift`. Changing the decision is the user's call, after which the ADR is edited, replaced, or deleted per `project-docs`; never edit an ADR just to match the code.

high — Drift-audit instructions copy the ADR maintenance actions

I read the changed ADR paragraph in skills/verify-doc-drift/SKILL.md and the canonical maintenance rule in skills/project-docs/SKILL.md. The latter defines the actions: When a decision changes, edit its ADR to state the new decision. When a new decision replaces it outright, delete the old ADR and add a new one. Delete an ADR whose decision no longer applies. The changed sentence repeats that action set as edited, replaced, or deleted, even while citing project-docs. It currently agrees in substance, but the next maintenance-rule change can leave this workflow with a silent, incomplete instruction about what the user may do. This is a current skill instruction, the agent can open project-docs, and the actions are presented as the available set rather than examples; no exception applies. Say Changing the decision is the user's call; then follow the ADR maintenance rules in project-docs. Never edit an ADR just to match the code. The source quote establishes the actions; no runtime check is needed.

lens restated-sets · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41KQXQZFY6J5J09EF4P2GJM of review 01M41KHRHF0D8BVKBP18YFCNQD

<!-- review:claim:01M41KQXQZFY6J5J09EF4P2GJM --> **high** — Drift-audit instructions copy the ADR maintenance actions > I read the changed ADR paragraph in skills/verify-doc-drift/SKILL.md and the canonical maintenance rule in skills/project-docs/SKILL.md. The latter defines the actions: `When a decision changes, edit its ADR to state the new decision. When a new decision replaces it outright, delete the old ADR and add a new one. Delete an ADR whose decision no longer applies.` The changed sentence repeats that action set as `edited, replaced, or deleted`, even while citing project-docs. It currently agrees in substance, but the next maintenance-rule change can leave this workflow with a silent, incomplete instruction about what the user may do. This is a current skill instruction, the agent can open project-docs, and the actions are presented as the available set rather than examples; no exception applies. Say `Changing the decision is the user's call; then follow the ADR maintenance rules in project-docs. Never edit an ADR just to match the code.` The source quote establishes the actions; no runtime check is needed. lens `restated-sets` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41KQXQZFY6J5J09EF4P2GJM` of review `01M41KHRHF0D8BVKBP18YFCNQD`

high — Drift-audit instructions copy ADR status classes

I read the changed ADR-status guidance in skills/verify-doc-drift/SKILL.md and the binding-status rule in skills/project-docs/SKILL.md. Project-docs defines the status classes: An ADR marked \accepted`, or without a status, is decided and binding; `proposed` means the decision is still open. Other statuses, including `superseded` and `deprecated`, are outdated and do not bind. The drift-audit paragraph copies the accepted/status-less and proposed members in its own status split. It currently agrees, but a change to the canonical ADR status rule can make the audit classify a document using stale members. This is a current agent instruction whose reader can open project-docs, and the status names are cases, not examples; no exception applies. Refer to the source to identify open and binding ADRs, then retain the audit-specific behavior: Follow project-docs to determine whether an ADR is open or binding. Correct an open ADR's statement of current behavior when contradicted by code; classify code that contradicts a binding ADR as code-drift.` The source quote establishes the set; the claim is based on static comparison.

lens restated-sets · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41KREHBVG8512KGVNCKZFPE of review 01M41KHRHF0D8BVKBP18YFCNQD

<!-- review:claim:01M41KREHBVG8512KGVNCKZFPE --> **high** — Drift-audit instructions copy ADR status classes > I read the changed ADR-status guidance in skills/verify-doc-drift/SKILL.md and the binding-status rule in skills/project-docs/SKILL.md. Project-docs defines the status classes: `An ADR marked \`accepted\`, or without a status, is decided and binding; \`proposed\` means the decision is still open. Other statuses, including \`superseded\` and \`deprecated\`, are outdated and do not bind`. The drift-audit paragraph copies the accepted/status-less and proposed members in its own status split. It currently agrees, but a change to the canonical ADR status rule can make the audit classify a document using stale members. This is a current agent instruction whose reader can open project-docs, and the status names are cases, not examples; no exception applies. Refer to the source to identify open and binding ADRs, then retain the audit-specific behavior: `Follow project-docs to determine whether an ADR is open or binding. Correct an open ADR's statement of current behavior when contradicted by code; classify code that contradicts a binding ADR as code-drift.` The source quote establishes the set; the claim is based on static comparison. lens `restated-sets` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41KREHBVG8512KGVNCKZFPE` of review `01M41KHRHF0D8BVKBP18YFCNQD`
Author
Owner

The proposed removal assumes project-docs is available to this reader. At 71043340, verify-doc-drift has no metadata.axskills.requires and never calls project-docs. writing-for-agents (One Idea, One Place) explicitly permits short required material from undeclared skills that readers may not have; skill-packaging says delivery guarantees availability only for declared dependencies. The local maintenance summary supplies that portable guidance and explicitly leaves the decision with the user. The actions currently agree; no conflicting maintenance behavior is demonstrated. Deleting them without separately changing the dependency/loading contract is unsupported.

<!-- gh-feedback:reply-to:109917 --> The proposed removal assumes project-docs is available to this reader. At 71043340, verify-doc-drift has no metadata.axskills.requires and never calls project-docs. writing-for-agents (One Idea, One Place) explicitly permits short required material from undeclared skills that readers may not have; skill-packaging says delivery guarantees availability only for declared dependencies. The local maintenance summary supplies that portable guidance and explicitly leaves the decision with the user. The actions currently agree; no conflicting maintenance behavior is demonstrated. Deleting them without separately changing the dependency/loading contract is unsupported.
Author
Owner

At 71043340 verify-doc-drift neither declares project-docs in metadata.axskills.requires nor invokes it. writing-for-agents explicitly permits short required material from undeclared skills that readers may not have; skill-packaging says catalog discovery may omit capabilities and delivery requires declared dependencies. The local proposed/accepted/statusless split supplies the audit-specific correction direction for those readers and matches the canonical contract. Replacing it with an unguaranteed lookup would lose that guidance. No present classification conflict is shown.

<!-- gh-feedback:reply-to:109918 --> At 71043340 verify-doc-drift neither declares project-docs in metadata.axskills.requires nor invokes it. writing-for-agents explicitly permits short required material from undeclared skills that readers may not have; skill-packaging says catalog discovery may omit capabilities and delivery requires declared dependencies. The local proposed/accepted/statusless split supplies the audit-specific correction direction for those readers and matches the canonical contract. Replacing it with an unguaranteed lookup would lose that guidance. No present classification conflict is shown.

superseded by review 01M41MPNRQQVAHPRDQJBK6VM0Q for head babf98cbbc3f37c673af24c448ccd89d9750191f

<!-- review:superseded:01M41MPNRQQVAHPRDQJBK6VM0Q --> superseded by review `01M41MPNRQQVAHPRDQJBK6VM0Q` for head `babf98cbbc3f37c673af24c448ccd89d9750191f`
fix(skills): check conflicts against every binding ADR
All checks were successful
commit-msg / commitlint (pull_request) Successful in 23s
Node tests / node:test (pull_request) Successful in 1m36s
Review / Review (pull_request_target) Successful in 8m26s
babf98cbbc
jercik changed title from fix(skills): keep only current ADRs to fix(skills): ADRs should reflect settled decisions 2026-10-03 19:45:18 +00:00
Author
Owner

Replying to review comment #109916

Tracked in #104: completion refers to the verification checks defined above instead of copying their count. The one-line maintenance change is based on the current default branch and preserves the checks themselves. This source PR is not claimed fixed by the unmerged follow-up.

> Replying to review comment #109916 Tracked in https://code.j4k.dev/j4k-oss/agent-skills/pulls/104: completion refers to the verification checks defined above instead of copying their count. The one-line maintenance change is based on the current default branch and preserves the checks themselves. This source PR is not claimed fixed by the unmerged follow-up.
@ -58,3 +58,3 @@
- **Use the glossary's canonical terms** in everything you produce — plans, code, commit messages. When the user's wording conflicts with a defined term, flag the mismatch instead of silently adopting either side.
- **Don't re-litigate recorded ADRs.** A status-less ADR counts as accepted; only an ADR marked `proposed` is still open, and a `deprecated` or `superseded` one no longer binds — its successor, where one exists, governs instead. If the plan contradicts a binding ADR, name it ("this conflicts with ADR-0007") and either adjust the plan or supersede the ADR — never silently override a recorded decision.
- **Don't re-litigate recorded ADRs.** An ADR marked `accepted`, or without a status, is decided and binding; `proposed` means the decision is still open. Other statuses, including `superseded` and `deprecated`, are outdated and do not bind: flag them and use the maintenance rules when that cleanup is authorized. If the plan contradicts a binding ADR, name it ("this conflicts with ADR-0007"), then either adjust the plan or, on the user's decision, change the ADR per the maintenance rules — never silently override a recorded decision.

medium — Status-less binding ADRs are omitted from reengineering requirements

I read the ADR loading and format rules here and traced their consumers in reengineer-program/SKILL.md and its compare-designs.md and implementation.md references. This changed sentence makes an ADR without status binding, and the ADR format still says status frontmatter is optional. Yet reengineer-program says completion requires every promise to trace to an accepted ADR, while compare-designs.md gives designers only “accepted ADRs” as requirements and implementation.md gives implementers only “accepted ADRs in scope” as authority. A project with a status-less ADR can therefore have a binding decision omitted from design and implementation briefs, or be pushed to re-decide an already settled promise. The writing guidance calls for one term per concept and one home for an instruction; use the binding-ADR definition consistently in those downstream handoffs, or explicitly convert status-less records before the handoffs. This preserves the intended binding status without dropping an existing decision. The omission follows from the text; I could not observe a downstream run on a project with a status-less ADR.

lens writing-quality · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41MW4TW2R1GN0PTQRB9AFKC of review 01M41MPNRQQVAHPRDQJBK6VM0Q

<!-- review:claim:01M41MW4TW2R1GN0PTQRB9AFKC --> **medium** — Status-less binding ADRs are omitted from reengineering requirements > I read the ADR loading and format rules here and traced their consumers in reengineer-program/SKILL.md and its compare-designs.md and implementation.md references. This changed sentence makes an ADR without status binding, and the ADR format still says status frontmatter is optional. Yet reengineer-program says completion requires every promise to trace to an accepted ADR, while compare-designs.md gives designers only “accepted ADRs” as requirements and implementation.md gives implementers only “accepted ADRs in scope” as authority. A project with a status-less ADR can therefore have a binding decision omitted from design and implementation briefs, or be pushed to re-decide an already settled promise. The writing guidance calls for one term per concept and one home for an instruction; use the binding-ADR definition consistently in those downstream handoffs, or explicitly convert status-less records before the handoffs. This preserves the intended binding status without dropping an existing decision. The omission follows from the text; I could not observe a downstream run on a project with a status-less ADR. lens `writing-quality` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41MW4TW2R1GN0PTQRB9AFKC` of review `01M41MPNRQQVAHPRDQJBK6VM0Q`
Author
Owner

Fixed in e1a7d3f05d. Reengineering completion, resumed authority, the generated future rule, designer and implementer briefs, and the replacement-review exception now use binding ADRs through the loaded project-docs definition. New Keep/Drop records and newly answered proposals still explicitly become status: accepted; no settled decision is reopened.

<!-- gh-feedback:reply-to:110193 --> Fixed in e1a7d3f05db3e0bf1345e7c84d8f175475c19447. Reengineering completion, resumed authority, the generated future rule, designer and implementer briefs, and the replacement-review exception now use binding ADRs through the loaded project-docs definition. New Keep/Drop records and newly answered proposals still explicitly become status: accepted; no settled decision is reopened.
jercik marked this conversation as resolved
@ -31,3 +31,3 @@
Have a fresh-context sub-agent attack the result from both directions: find situations the old code handles that the rebuild omits without an accepted decision, and promises the rebuild makes that no accepted decision covers. Mechanism-only differences and replacements chosen by an accepted ADR are not findings.
Put each uncovered situation to the user and record the answer in an added or superseding ADR before adding, removing, or keeping its promise. Completion requires the implementation to satisfy the confirmed design, all three checks to pass, and every review finding to be resolved.
Put each uncovered situation to the user and record the answer in a new or edited ADR before adding, removing, or keeping its promise. Completion requires the implementation to satisfy the confirmed design, all three checks to pass, and every review finding to be resolved.

high — Completion rule repeats the count of verification checks

I read the full implementation reference and the adjacent Verify section. The source list immediately above this sentence defines the verification set as ‘Unchanged promises’, ‘Dropped promises’, and ‘Replaced or added promises’. The completion sentence then repeats its size as ‘all three checks’. If a contract is added or removed, the completion rule can silently retain the old count and mislead the implementing agent. This is a live workflow instruction, not a dated record, a table of contents, or text needed by a reader who cannot see the source: the list is in the same file. Replace the count with ‘the checks above’ (or simply ‘the verification checks’). The decisive evidence would be another authoritative definition that fixes the number independent of this list; I found none in this reference.

lens restated-sets · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41MYBSN6VXTSCFDCRSHM0KQ of review 01M41MPNRQQVAHPRDQJBK6VM0Q

<!-- review:claim:01M41MYBSN6VXTSCFDCRSHM0KQ --> **high** — Completion rule repeats the count of verification checks > I read the full implementation reference and the adjacent Verify section. The source list immediately above this sentence defines the verification set as ‘Unchanged promises’, ‘Dropped promises’, and ‘Replaced or added promises’. The completion sentence then repeats its size as ‘all three checks’. If a contract is added or removed, the completion rule can silently retain the old count and mislead the implementing agent. This is a live workflow instruction, not a dated record, a table of contents, or text needed by a reader who cannot see the source: the list is in the same file. Replace the count with ‘the checks above’ (or simply ‘the verification checks’). The decisive evidence would be another authoritative definition that fixes the number independent of this list; I found none in this reference. lens `restated-sets` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41MYBSN6VXTSCFDCRSHM0KQ` of review `01M41MPNRQQVAHPRDQJBK6VM0Q`
jercik marked this conversation as resolved
@ -35,3 +35,3 @@
## Fix direction and missing features
Fix docs to match code by default. Change code only for a confirmed **code-drift** where the doc is the intended source of truth. A third case hides inside "incorrect": the doc describes a capability that _should_ exist but doesn't (an endpoint, a flag). Removing the claim and _implementing_ the feature are both valid — surface the fork to the user, and if the claim is removed, flag the missing feature as separate work rather than silently dropping it. ADRs are decision records, not behaviour docs. A `proposed` ADR is an open question: correct its statement of current behaviour if the code contradicts it, never delete it as stale. An `accepted` ADR is the source of truth for the decision it records: code contradicting it is `code-drift`, and changing the decision means superseding the ADR per `project-docs`, never editing it to match the code.
Fix docs to match code by default. Change code only for a confirmed **code-drift** where the doc is the intended source of truth. A third case hides inside "incorrect": the doc describes a capability that _should_ exist but doesn't (an endpoint, a flag). Removing the claim and _implementing_ the feature are both valid — surface the fork to the user, and if the claim is removed, flag the missing feature as separate work rather than silently dropping it. ADRs are decision records, not behaviour docs. A `proposed` ADR is an open question: correct its statement of current behaviour if the code contradicts it, never delete it as stale. An `accepted` or status-less ADR is the source of truth for the decision it records: code contradicting it is `code-drift`. Changing the decision is the user's call, after which the ADR is edited, replaced, or deleted per `project-docs`; never edit an ADR just to match the code.

high — ADR maintenance actions are copied into verify-doc-drift

I read this full skill and the referenced skills/project-docs/SKILL.md. Its maintenance rule defines the ADR outcomes: ‘When a decision changes, edit its ADR to state the new decision. When a new decision replaces it outright, delete the old ADR and add a new one. Delete an ADR whose decision no longer applies.’ The changed verify-doc-drift sentence enumerates those outcomes again as ‘edited, replaced, or deleted’ even while pointing to project-docs. The copy agrees today, but a change to that maintenance rule can leave this independent list stale and steer an audit to the wrong ADR action. No exception applies: this is a live skill, the source is explicitly named, and the action list is not needed as a table of contents or a dated record. Keep the audit-specific rule and say: ‘Changing the decision is the user's call; then maintain its ADR per project-docs, never merely to match code drift.’ A contrary finding would require evidence that project-docs is inaccessible to the intended agent despite this skill already directing it there; the tree has no such constraint.

lens restated-sets · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41MZGA77DNVW75H3Y71PHA8 of review 01M41MPNRQQVAHPRDQJBK6VM0Q

<!-- review:claim:01M41MZGA77DNVW75H3Y71PHA8 --> **high** — ADR maintenance actions are copied into verify-doc-drift > I read this full skill and the referenced `skills/project-docs/SKILL.md`. Its maintenance rule defines the ADR outcomes: ‘When a decision changes, edit its ADR to state the new decision. When a new decision replaces it outright, delete the old ADR and add a new one. Delete an ADR whose decision no longer applies.’ The changed verify-doc-drift sentence enumerates those outcomes again as ‘edited, replaced, or deleted’ even while pointing to `project-docs`. The copy agrees today, but a change to that maintenance rule can leave this independent list stale and steer an audit to the wrong ADR action. No exception applies: this is a live skill, the source is explicitly named, and the action list is not needed as a table of contents or a dated record. Keep the audit-specific rule and say: ‘Changing the decision is the user's call; then maintain its ADR per `project-docs`, never merely to match code drift.’ A contrary finding would require evidence that `project-docs` is inaccessible to the intended agent despite this skill already directing it there; the tree has no such constraint. lens `restated-sets` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41MZGA77DNVW75H3Y71PHA8` of review `01M41MPNRQQVAHPRDQJBK6VM0Q`

high — Verify-doc-drift copies ADR status members from project-docs

I read skills/verify-doc-drift/SKILL.md and the source policy in skills/project-docs/SKILL.md. The latter says: ‘An ADR marked accepted, or without a status, is decided and binding; proposed means the decision is still open. Other statuses, including superseded and deprecated, are outdated and do not bind’. This changed audit instruction separately names the status members and repeats their open/binding behavior. It currently omits the outdated-status case, and future status changes would require another edit here. This is a live instruction rather than a dated record; it already refers the reader to project-docs, so the inaccessible-source exception has no support in the tree. Keep the audit-specific behavior without copying status members: ‘Use project-docs to determine whether an ADR is open, binding, or outdated. Correct an inaccurate current-behavior description in an open ADR while leaving the decision open; treat code that contradicts a binding decision as code-drift.’ The decisive refutation would be evidence that these are distinct status sets or that the reader cannot open project-docs; neither appears in the reviewed files.

lens restated-sets · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41N046QS7TG2BES0A5REX7C of review 01M41MPNRQQVAHPRDQJBK6VM0Q

<!-- review:claim:01M41N046QS7TG2BES0A5REX7C --> **high** — Verify-doc-drift copies ADR status members from project-docs > I read `skills/verify-doc-drift/SKILL.md` and the source policy in `skills/project-docs/SKILL.md`. The latter says: ‘An ADR marked `accepted`, or without a status, is decided and binding; `proposed` means the decision is still open. Other statuses, including `superseded` and `deprecated`, are outdated and do not bind’. This changed audit instruction separately names the status members and repeats their open/binding behavior. It currently omits the outdated-status case, and future status changes would require another edit here. This is a live instruction rather than a dated record; it already refers the reader to `project-docs`, so the inaccessible-source exception has no support in the tree. Keep the audit-specific behavior without copying status members: ‘Use `project-docs` to determine whether an ADR is open, binding, or outdated. Correct an inaccurate current-behavior description in an open ADR while leaving the decision open; treat code that contradicts a binding decision as `code-drift`.’ The decisive refutation would be evidence that these are distinct status sets or that the reader cannot open `project-docs`; neither appears in the reviewed files. lens `restated-sets` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41N046QS7TG2BES0A5REX7C` of review `01M41MPNRQQVAHPRDQJBK6VM0Q`
Author
Owner

The tree does contain the delivery boundary: verify-doc-drift has no metadata.axskills.requires and no Call the Skill tool with project-docs invocation. writing-for-agents, One Idea One Place, allows short required material from undeclared skills the reader may not have. Its skill-packaging reference states that catalog discovery may omit capabilities and only declared dependencies guarantee delivery. A textual per-project-docs reference does not establish that runtime availability. This portable local summary currently agrees with the source and keeps the user decision gate; simply removing it is not a supported fix.

<!-- gh-feedback:reply-to:110191 --> The tree does contain the delivery boundary: verify-doc-drift has no metadata.axskills.requires and no Call the Skill tool with project-docs invocation. writing-for-agents, One Idea One Place, allows short required material from undeclared skills the reader may not have. Its skill-packaging reference states that catalog discovery may omit capabilities and only declared dependencies guarantee delivery. A textual per-project-docs reference does not establish that runtime availability. This portable local summary currently agrees with the source and keeps the user decision gate; simply removing it is not a supported fix.
Author
Owner

The inaccessible-source counterevidence is the actual skill delivery contract: verify-doc-drift declares no project-docs dependency and never invokes it. writing-for-agents permits short required material from undeclared skills; its packaging reference says catalog discovery may omit a capability and declared dependencies guarantee delivery. Merely naming project-docs does not load or deliver it. This local split keeps the audit-specific correction direction available; no current contradictory classification is reproduced. Introducing a hard dependency would be a separate delivery-contract change, not evidence that this deletion is safe.

<!-- gh-feedback:reply-to:110192 --> The inaccessible-source counterevidence is the actual skill delivery contract: verify-doc-drift declares no project-docs dependency and never invokes it. writing-for-agents permits short required material from undeclared skills; its packaging reference says catalog discovery may omit a capability and declared dependencies guarantee delivery. Merely naming project-docs does not load or deliver it. This local split keeps the audit-specific correction direction available; no current contradictory classification is reproduced. Introducing a hard dependency would be a separate delivery-contract change, not evidence that this deletion is safe.

superseded by review 01M41NG4S8JHH19TW7JCMXNQCH for head e1a7d3f05db3e0bf1345e7c84d8f175475c19447

<!-- review:superseded:01M41NG4S8JHH19TW7JCMXNQCH --> superseded by review `01M41NG4S8JHH19TW7JCMXNQCH` for head `e1a7d3f05db3e0bf1345e7c84d8f175475c19447`
Author
Owner

Replying to review comment #110190

Tracked in #104, which already owns the same count-reference finding from #109916. The independent one-line change preserves every verification contract. It is not yet merged into this PR.

> Replying to review comment #110190 Tracked in https://code.j4k.dev/j4k-oss/agent-skills/pulls/104, which already owns the same count-reference finding from #109916. The independent one-line change preserves every verification contract. It is not yet merged into this PR.
fix(skills): pass binding ADRs to reengineering agents
Some checks failed
commit-msg / commitlint (pull_request) Failing after 15s
Node tests / node:test (pull_request) Successful in 1m37s
Review / Review (pull_request_target) Has been cancelled
e1a7d3f05d
@ -57,3 +57,3 @@
Replace proposed text with the accepted decision when answered. Delete proposed ADRs only when they duplicate another root; retain accepted negatives after their code disappears. Use the same format for design choices, implementation gaps, and review findings. ADRs plus git carry resume state; execution history belongs in commits.
Accepted ADRs remain settled across sessions. Supersede one when a fact undermines its reason, a witness appears for a don't-care drop, or a deferred keep's owner answers. Proposed and status-less ADRs are the decision backlog.
Binding ADRs remain settled across sessions. When a fact undermines one's reason or a witness appears for a don't-care drop, put it to the user; when the user or a deferred keep's owner answers, edit or replace the ADR per `project-docs`. Proposed ADRs are the decision backlog.

high — Reengineering skill repeats ADR maintenance choices

I read this skill's inventory, decision-recording, and repository-rule paragraphs and the canonical maintenance rule in skills/project-docs/SKILL.md. The source says, "When a decision changes, edit its ADR to state the new decision. When a new decision replaces it outright, delete the old ADR and add a new one." This changed paragraph repeats those outcomes as "edit or replace the ADR". The inventory paragraph similarly says an answer "rewrites or replaces" existing ADRs, and the repository-rule paragraph repeats "editing or replacing its ADR". All three are copies of the same maintenance choice already defined in project-docs. They agree at this snapshot, but a change to the canonical rule can make agents following this program-specific skill apply stale instructions. The agent loads project-docs earlier in this file, so it can open the source; the passages are not dated records or examples. Keep the program-specific trigger and decision authority, but refer each action to the maintenance rules in project-docs without listing methods. This is a static comparison; a separate program-specific rule defining different ADR actions would refute the claim, but this skill explicitly defers to project-docs for these actions.

lens restated-sets · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41NRXZQSZQYXFZ00MFN6TQJ of review 01M41NG4S8JHH19TW7JCMXNQCH

<!-- review:claim:01M41NRXZQSZQYXFZ00MFN6TQJ --> **high** — Reengineering skill repeats ADR maintenance choices > I read this skill's inventory, decision-recording, and repository-rule paragraphs and the canonical maintenance rule in skills/project-docs/SKILL.md. The source says, "When a decision changes, edit its ADR to state the new decision. When a new decision replaces it outright, delete the old ADR and add a new one." This changed paragraph repeats those outcomes as "edit or replace the ADR". The inventory paragraph similarly says an answer "rewrites or replaces" existing ADRs, and the repository-rule paragraph repeats "editing or replacing its ADR". All three are copies of the same maintenance choice already defined in project-docs. They agree at this snapshot, but a change to the canonical rule can make agents following this program-specific skill apply stale instructions. The agent loads project-docs earlier in this file, so it can open the source; the passages are not dated records or examples. Keep the program-specific trigger and decision authority, but refer each action to the maintenance rules in project-docs without listing methods. This is a static comparison; a separate program-specific rule defining different ADR actions would refute the claim, but this skill explicitly defers to project-docs for these actions. lens `restated-sets` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41NRXZQSZQYXFZ00MFN6TQJ` of review `01M41NG4S8JHH19TW7JCMXNQCH`

superseded by review 01M41PW2V1NMN8WKZDND4XDBEB for head 8e30deffaacb6c045b09ace982a6db57d95bd269

<!-- review:superseded:01M41PW2V1NMN8WKZDND4XDBEB --> superseded by review `01M41PW2V1NMN8WKZDND4XDBEB` for head `8e30deffaacb6c045b09ace982a6db57d95bd269`
jercik marked this conversation as resolved
@ -32,2 +31,3 @@
Have a fresh-context sub-agent attack the result from both directions: find situations the old code handles that the rebuild omits without an accepted decision, and promises the rebuild makes that no accepted decision covers. Mechanism-only differences and replacements chosen by a binding ADR are not findings.
Put each uncovered situation to the user and record the answer in an added or superseding ADR before adding, removing, or keeping its promise. Completion requires the implementation to satisfy the confirmed design, all three checks to pass, and every review finding to be resolved.
Put each uncovered situation to the user and record the answer in a new or edited ADR before adding, removing, or keeping its promise. Completion requires the implementation to satisfy the confirmed design, all three checks to pass, and every review finding to be resolved.

high — Completion criterion restates the shared verification-check count

I read this implementation guide and the shared skills/reengineering/SKILL.md verification section. The shared source defines the set with "Use three checks:" and the entries "- Retained promises: demonstrate that their outcomes and required compatibility still hold.", "- Dropped promises: demonstrate that the obligation and its unnecessary footprint are gone.", and "- Replaced or added promises: test the chosen outcome, not equivalence to the old implementation." This guide copies the same verification categories in its "## Verify" list and the changed completion sentence restates their number as "all three checks". The copies agree now, but a change to the shared checks can silently leave this completion criterion and local list stale. The shared skill is accessible to the agent via this guide's instruction to load it, so neither access nor a dated-record exception applies. Replace the count with "the verification checks in the shared reengineering skill" and refer to that source for the set; retain program-specific verification detail without an exhaustive duplicate list. This is a static comparison of the two files; a different canonical verification source would refute it, and none appeared in the files read.

lens restated-sets · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41NQP4VEY4WS7PS45Q7TB66 of review 01M41NG4S8JHH19TW7JCMXNQCH

<!-- review:claim:01M41NQP4VEY4WS7PS45Q7TB66 --> **high** — Completion criterion restates the shared verification-check count > I read this implementation guide and the shared skills/reengineering/SKILL.md verification section. The shared source defines the set with "Use three checks:" and the entries "- **Retained promises:** demonstrate that their outcomes and required compatibility still hold.", "- **Dropped promises:** demonstrate that the obligation and its unnecessary footprint are gone.", and "- **Replaced or added promises:** test the chosen outcome, not equivalence to the old implementation." This guide copies the same verification categories in its "## Verify" list and the changed completion sentence restates their number as "all three checks". The copies agree now, but a change to the shared checks can silently leave this completion criterion and local list stale. The shared skill is accessible to the agent via this guide's instruction to load it, so neither access nor a dated-record exception applies. Replace the count with "the verification checks in the shared reengineering skill" and refer to that source for the set; retain program-specific verification detail without an exhaustive duplicate list. This is a static comparison of the two files; a different canonical verification source would refute it, and none appeared in the files read. lens `restated-sets` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41NQP4VEY4WS7PS45Q7TB66` of review `01M41NG4S8JHH19TW7JCMXNQCH`
jercik marked this conversation as resolved
@ -35,3 +35,3 @@
## Fix direction and missing features
Fix docs to match code by default. Change code only for a confirmed **code-drift** where the doc is the intended source of truth. A third case hides inside "incorrect": the doc describes a capability that _should_ exist but doesn't (an endpoint, a flag). Removing the claim and _implementing_ the feature are both valid — surface the fork to the user, and if the claim is removed, flag the missing feature as separate work rather than silently dropping it. ADRs are decision records, not behaviour docs. A `proposed` ADR is an open question: correct its statement of current behaviour if the code contradicts it, never delete it as stale. An `accepted` ADR is the source of truth for the decision it records: code contradicting it is `code-drift`, and changing the decision means superseding the ADR per `project-docs`, never editing it to match the code.
Fix docs to match code by default. Change code only for a confirmed **code-drift** where the doc is the intended source of truth. A third case hides inside "incorrect": the doc describes a capability that _should_ exist but doesn't (an endpoint, a flag). Removing the claim and _implementing_ the feature are both valid — surface the fork to the user, and if the claim is removed, flag the missing feature as separate work rather than silently dropping it. ADRs are decision records, not behaviour docs. A `proposed` ADR is an open question: correct its statement of current behaviour if the code contradicts it, never delete it as stale. An `accepted` or status-less ADR is the source of truth for the decision it records: code contradicting it is `code-drift`. Changing the decision is the user's call, after which the ADR is edited, replaced, or deleted per `project-docs`; never edit an ADR just to match the code.

high — Doc-drift guidance repeats the ADR maintenance options

I read this skill's "Fix direction and missing features" section and the canonical maintenance rule in skills/project-docs/SKILL.md. That rule defines the actions: "When a decision changes, edit its ADR to state the new decision. When a new decision replaces it outright, delete the old ADR and add a new one. Delete an ADR whose decision no longer applies." The changed sentence here lists those same alternatives as "edited, replaced, or deleted" before pointing back to project-docs. It currently agrees, but any change to the canonical maintenance rule requires this copy to change too, and a stale instruction would send a doc-drift audit down the wrong repair path. The agent is explicitly directed to project-docs; this is neither an inaccessible-source prompt nor a dated record. Keep the unique instruction that the user decides and that the ADR must not be edited merely to match code, but replace the action list with "follow the maintenance rules in project-docs." Static source comparison establishes the duplicate; it would be refuted if another file were the actual authority for ADR maintenance, but this sentence itself names project-docs as that authority.

lens restated-sets · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41NR92A46Q6QQMWWJBGACPJ of review 01M41NG4S8JHH19TW7JCMXNQCH

<!-- review:claim:01M41NR92A46Q6QQMWWJBGACPJ --> **high** — Doc-drift guidance repeats the ADR maintenance options > I read this skill's "Fix direction and missing features" section and the canonical maintenance rule in skills/project-docs/SKILL.md. That rule defines the actions: "When a decision changes, edit its ADR to state the new decision. When a new decision replaces it outright, delete the old ADR and add a new one. Delete an ADR whose decision no longer applies." The changed sentence here lists those same alternatives as "edited, replaced, or deleted" before pointing back to project-docs. It currently agrees, but any change to the canonical maintenance rule requires this copy to change too, and a stale instruction would send a doc-drift audit down the wrong repair path. The agent is explicitly directed to project-docs; this is neither an inaccessible-source prompt nor a dated record. Keep the unique instruction that the user decides and that the ADR must not be edited merely to match code, but replace the action list with "follow the maintenance rules in project-docs." Static source comparison establishes the duplicate; it would be refuted if another file were the actual authority for ADR maintenance, but this sentence itself names project-docs as that authority. lens `restated-sets` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41NR92A46Q6QQMWWJBGACPJ` of review `01M41NG4S8JHH19TW7JCMXNQCH`
Author
Owner

At 1610808, verify-doc-drift declares no project-docs dependency and does not invoke it. The writing-for-agents portability rule permits short required material from undeclared skills that readers may not have; axskills guarantees delivery only for declared dependencies. A prose reference alone is not proof of loading or availability. This audit-specific mapping agrees with the canonical maintenance policy and preserves the user-authority boundary; no present contradiction was demonstrated. Removing it would leave the standalone audit reader without the repair choices. The existing source and packaging evidence therefore do not support this finding.

<!-- gh-feedback:reply-to:110345 --> At 1610808, verify-doc-drift declares no project-docs dependency and does not invoke it. The writing-for-agents portability rule permits short required material from undeclared skills that readers may not have; axskills guarantees delivery only for declared dependencies. A prose reference alone is not proof of loading or availability. This audit-specific mapping agrees with the canonical maintenance policy and preserves the user-authority boundary; no present contradiction was demonstrated. Removing it would leave the standalone audit reader without the repair choices. The existing source and packaging evidence therefore do not support this finding.

superseded by review 01M41PW2V1NMN8WKZDND4XDBEB for head 8e30deffaacb6c045b09ace982a6db57d95bd269

<!-- review:superseded:01M41PW2V1NMN8WKZDND4XDBEB --> superseded by review `01M41PW2V1NMN8WKZDND4XDBEB` for head `8e30deffaacb6c045b09ace982a6db57d95bd269`
jercik changed title from fix(skills): ADRs should reflect settled decisions to fix(skills): record settled ADR decisions 2026-10-03 20:17:17 +00:00
fix(skills): honor binding ADRs throughout implementation
Some checks failed
commit-msg / commitlint (pull_request) Successful in 30s
Node tests / node:test (pull_request) Successful in 2m6s
Review / Review (pull_request_target) Has been cancelled
1610808ae9
Author
Owner

Replying to review comment #101438

The medium claim 01M41NQ78J88Y5AJW0P67JW3CW (statusless drops at the subtraction/review gates, review 01M41NG4S8JHH19TW7JCMXNQCH) is fixed in 1610808ae9. Subtraction excludes removal/replacement settled by a binding ADR, and the review checks omissions and new promises against binding ADRs. The consumer audit also covers retained negative ADRs and generated repository rules. New choices still receive status: accepted; accepted design and accepted fallout retain their ordinary user-approval meaning.

This reports that source fix only. The report's unadjudicated pointer claim and separate duplication suggestions still require their own disposition; the summary is not yet acknowledged as complete.

> Replying to review comment #101438 The medium claim `01M41NQ78J88Y5AJW0P67JW3CW` (statusless drops at the subtraction/review gates, review `01M41NG4S8JHH19TW7JCMXNQCH`) is fixed in 1610808ae92e61621d4bbbd08ea073e406bd7d43. Subtraction excludes removal/replacement settled by a binding ADR, and the review checks omissions and new promises against binding ADRs. The consumer audit also covers retained negative ADRs and generated repository rules. New choices still receive `status: accepted`; accepted design and accepted fallout retain their ordinary user-approval meaning. This reports that source fix only. The report's unadjudicated pointer claim and separate duplication suggestions still require their own disposition; the summary is not yet acknowledged as complete.
fix(skills): load governing ADRs from outdated pointers
All checks were successful
commit-msg / commitlint (pull_request) Successful in 20s
Node tests / node:test (pull_request) Successful in 2m6s
Review / Review (pull_request_target) Successful in 8m24s
8e30deffaa
Author
Owner

Replying to review comment #101438

The unadjudicated medium claim 01M41NP1WF93J55D4Q7TJ885VG, 'Follow existing superseded ADR pointers to the governing decision', identified a removed loading guarantee. Default e7d02b52 says an outdated ADR's successor governs instead. ADR2's verification gate applies to added or edited records, so it does not cover read-only planning through improve-codebase-architecture.

8e30deffaa restores bounded direct-pointer reading in project-docs and applies the current record's own status rules. Proposed records remain open, outdated records remain nonbinding, and cleanup still requires authorization. No historical chain or archive audit is added. This is the source repair; the report's unadjudicated bucket remains an incomplete service result until a fresh current-head cycle supplies complete evidence.

> Replying to review comment #101438 The unadjudicated medium claim `01M41NP1WF93J55D4Q7TJ885VG`, 'Follow existing superseded ADR pointers to the governing decision', identified a removed loading guarantee. Default e7d02b52 says an outdated ADR's successor governs instead. ADR2's verification gate applies to added or edited records, so it does not cover read-only planning through `improve-codebase-architecture`. 8e30deffaacb6c045b09ace982a6db57d95bd269 restores bounded direct-pointer reading in `project-docs` and applies the current record's own status rules. Proposed records remain open, outdated records remain nonbinding, and cleanup still requires authorization. No historical chain or archive audit is added. This is the source repair; the report's unadjudicated bucket remains an incomplete service result until a fresh current-head cycle supplies complete evidence.
Author
Owner

Replying to review comment #110344

The repeated completion count is tracked in #104. That independently based follow-up refers to the verification checks defined above; it has not merged into this PR.

The local verification procedure also adds program-specific obligations absent from the shared category list: kept-surface snapshots and surviving tests, and rejection or unrepresentability of dropped situations. Those details remain useful implementation instructions. Matching category labels alone do not establish that deleting this procedure preserves its requirements, so the follow-up retains those details.

> Replying to review comment #110344 The repeated completion count is tracked in https://code.j4k.dev/j4k-oss/agent-skills/pulls/104. That independently based follow-up refers to the verification checks defined above; it has not merged into this PR. The local verification procedure also adds program-specific obligations absent from the shared category list: kept-surface snapshots and surviving tests, and rejection or unrepresentability of dropped situations. Those details remain useful implementation instructions. Matching category labels alone do not establish that deleting this procedure preserves its requirements, so the follow-up retains those details.
@ -58,3 +58,3 @@
- **Use the glossary's canonical terms** in everything you produce — plans, code, commit messages. When the user's wording conflicts with a defined term, flag the mismatch instead of silently adopting either side.
- **Don't re-litigate recorded ADRs.** A status-less ADR counts as accepted; only an ADR marked `proposed` is still open, and a `deprecated` or `superseded` one no longer binds — its successor, where one exists, governs instead. If the plan contradicts a binding ADR, name it ("this conflicts with ADR-0007") and either adjust the plan or supersede the ADR — never silently override a recorded decision.
- **Don't re-litigate recorded ADRs.** An ADR marked `accepted`, or without a status, is decided and binding; `proposed` means the decision is still open. Other statuses, including `superseded` and `deprecated`, are outdated and do not bind: flag them and use the maintenance rules when that cleanup is authorized. Read any current ADR an outdated record points to and apply these status rules. If the plan contradicts a binding ADR, name it ("this conflicts with ADR-0007"), then either adjust the plan or, on the user's decision, change the ADR per the maintenance rules — never silently override a recorded decision.

medium — Unknown ADR statuses are incorrectly treated as obsolete

I read the full project-docs skill and its consumers in grill-with-docs, reengineer-program, and verify-doc-drift. This rule loads ADRs from an existing project, but the new blanket classification makes an ADR with a project-specific active status such as implemented nonbinding. An agent following it could disregard an operative decision and plan a contradictory change without asking the user. The status format below constrains newly written ADRs; it does not establish that every existing project's other status means obsolete. The writing standard calls for a rule narrow enough to avoid false exceptions. Name the known inactive markers (superseded and deprecated), and have the reader establish the meaning of an unfamiliar status before deciding whether that ADR binds. This preserves the accepted/status-less rule and the cleanup path for known obsolete records. The decisive check would be an existing repo's ADR status convention; this generic skill provides none.

lens writing-quality · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41Q2FA8AXHGFEBYE4VJJYKT of review 01M41PW2V1NMN8WKZDND4XDBEB

<!-- review:claim:01M41Q2FA8AXHGFEBYE4VJJYKT --> **medium** — Unknown ADR statuses are incorrectly treated as obsolete > I read the full `project-docs` skill and its consumers in `grill-with-docs`, `reengineer-program`, and `verify-doc-drift`. This rule loads ADRs from an existing project, but the new blanket classification makes an ADR with a project-specific active status such as `implemented` nonbinding. An agent following it could disregard an operative decision and plan a contradictory change without asking the user. The status format below constrains newly written ADRs; it does not establish that every existing project's other status means obsolete. The writing standard calls for a rule narrow enough to avoid false exceptions. Name the known inactive markers (`superseded` and `deprecated`), and have the reader establish the meaning of an unfamiliar status before deciding whether that ADR binds. This preserves the accepted/status-less rule and the cleanup path for known obsolete records. The decisive check would be an existing repo's ADR status convention; this generic skill provides none. lens `writing-quality` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41Q2FA8AXHGFEBYE4VJJYKT` of review `01M41PW2V1NMN8WKZDND4XDBEB`
Author
Owner

This asks for an additional status convention rather than demonstrating a defect in the agreed ADR contract. The canonical format defines proposed and accepted, explicitly makes statusless decisions binding, and requires other statuses to be flagged rather than silently treated as authority. The user settled that classification in this change. The finding names a hypothetical implemented convention but supplies no project or governing convention showing it applies here. Explicit user directions still override these defaults. Without a concrete incompatible convention, changing this classification would reopen the approved policy rather than repair its implementation.

<!-- gh-feedback:reply-to:110621 --> This asks for an additional status convention rather than demonstrating a defect in the agreed ADR contract. The canonical format defines proposed and accepted, explicitly makes statusless decisions binding, and requires other statuses to be flagged rather than silently treated as authority. The user settled that classification in this change. The finding names a hypothetical implemented convention but supplies no project or governing convention showing it applies here. Explicit user directions still override these defaults. Without a concrete incompatible convention, changing this classification would reopen the approved policy rather than repair its implementation.

superseded by review 01M41SFZWKA8GGYFCW0ZY7PGEB for head bc0921e42bab9862a5108abda1377c10de421e99

<!-- review:superseded:01M41SFZWKA8GGYFCW0ZY7PGEB --> superseded by review `01M41SFZWKA8GGYFCW0ZY7PGEB` for head `bc0921e42bab9862a5108abda1377c10de421e99`
@ -58,2 +57,3 @@
Replace proposed text with the accepted decision when answered. Delete proposed ADRs only when they duplicate another root; retain binding negative ADRs after their code disappears. Use the same format for design choices, implementation gaps, and review findings. ADRs plus git carry resume state; execution history belongs in commits.
Accepted ADRs remain settled across sessions. Supersede one when a fact undermines its reason, a witness appears for a don't-care drop, or a deferred keep's owner answers. Proposed and status-less ADRs are the decision backlog.
Binding ADRs remain settled across sessions. When a fact undermines one's reason or a witness appears for a don't-care drop, put it to the user; when the user or a deferred keep's owner answers, edit or replace the ADR per `project-docs`. Proposed ADRs are the decision backlog.

medium — Existing status-less backlog ADRs become binding without a decision

I read the full reengineer-program and project-docs skills and their diff. The previous reengineer-program rule explicitly said “Status-less ADRs count as proposed” and “Proposed and status-less ADRs are the decision backlog”; its own ADR rules took precedence over project-docs. This revision removes that exception, while project-docs now says a status-less ADR is binding. A project continued from the earlier workflow can therefore contain an unanswered, status-less root ADR that the new workflow treats as settled. The interview can skip that decision, and the design or implementer can take it as a requirement. Add a migration check when loading pre-existing status-less reengineering ADRs: determine whether each was actually decided and mark unresolved ones proposed before using binding ADRs as authority. This keeps the new explicit-status rule for future ADRs. I could not inspect downstream projects; an existing status-less ADR created as a backlog item would establish the concrete occurrence.

lens writing-quality · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41Q6GRA0Z1MA2ZAZ981TQRW of review 01M41PW2V1NMN8WKZDND4XDBEB

<!-- review:claim:01M41Q6GRA0Z1MA2ZAZ981TQRW --> **medium** — Existing status-less backlog ADRs become binding without a decision > I read the full `reengineer-program` and `project-docs` skills and their diff. The previous `reengineer-program` rule explicitly said “Status-less ADRs count as proposed” and “Proposed and status-less ADRs are the decision backlog”; its own ADR rules took precedence over `project-docs`. This revision removes that exception, while `project-docs` now says a status-less ADR is binding. A project continued from the earlier workflow can therefore contain an unanswered, status-less root ADR that the new workflow treats as settled. The interview can skip that decision, and the design or implementer can take it as a requirement. Add a migration check when loading pre-existing status-less reengineering ADRs: determine whether each was actually decided and mark unresolved ones `proposed` before using binding ADRs as authority. This keeps the new explicit-status rule for future ADRs. I could not inspect downstream projects; an existing status-less ADR created as a backlog item would establish the concrete occurrence. lens `writing-quality` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41Q6GRA0Z1MA2ZAZ981TQRW` of review `01M41PW2V1NMN8WKZDND4XDBEB`
Author
Owner

The old source at e7d02b52 required status frontmatter on every newly recorded ADR, created each unrecorded root as proposed before the first question, and parked unrelated promises as proposed. It did not generate new statusless backlog records. The old statusless interpretation conflicted with canonical project-docs; removing that exception and treating statusless ADRs as binding is the expressly settled change. Existing proposed records remain open. No actual unanswered statusless downstream ADR was supplied, so a blanket migration that re-interviews statusless decisions would reverse that choice based on an unverified legacy-data hypothesis. A named record with evidence of an unresolved decision would warrant scoped reconciliation; this finding provides none.

<!-- gh-feedback:reply-to:110622 --> The old source at e7d02b52 required status frontmatter on every newly recorded ADR, created each unrecorded root as proposed before the first question, and parked unrelated promises as proposed. It did not generate new statusless backlog records. The old statusless interpretation conflicted with canonical project-docs; removing that exception and treating statusless ADRs as binding is the expressly settled change. Existing proposed records remain open. No actual unanswered statusless downstream ADR was supplied, so a blanket migration that re-interviews statusless decisions would reverse that choice based on an unverified legacy-data hypothesis. A named record with evidence of an unresolved decision would warrant scoped reconciliation; this finding provides none.

superseded by review 01M41SFZWKA8GGYFCW0ZY7PGEB for head bc0921e42bab9862a5108abda1377c10de421e99

<!-- review:superseded:01M41SFZWKA8GGYFCW0ZY7PGEB --> superseded by review `01M41SFZWKA8GGYFCW0ZY7PGEB` for head `bc0921e42bab9862a5108abda1377c10de421e99`
@ -32,2 +31,3 @@
Have a fresh-context sub-agent attack the result from both directions: find situations the old code handles that the rebuild omits without a binding ADR, and promises the rebuild makes that no binding ADR covers. Mechanism-only differences and replacements chosen by a binding ADR are not findings.
Put each uncovered situation to the user and record the answer in an added or superseding ADR before adding, removing, or keeping its promise. Completion requires the implementation to satisfy the confirmed design, all three checks to pass, and every review finding to be resolved.
Put each uncovered situation to the user and record the answer in a new or edited ADR before adding, removing, or keeping its promise. Completion requires the implementation to satisfy the confirmed design, all three checks to pass, and every review finding to be resolved.

high — Completion condition restates the shared verification checklist size

I read this implementation reference, skills/reengineer-program/SKILL.md, and the shared skills/reengineering/SKILL.md. The program skill explicitly calls the shared reengineering skill. Its Verify section is the defining source: Use three checks: followed by Retained promises, Dropped promises, and Replaced or added promises. This reference repeats that three-member checklist under its own Check three distinct contracts: heading, then hard-codes the size again as all three checks in the anchored completion condition. This is a static trace, not a run. If the shared verification set gains or loses a check, this completion condition and its local checklist can silently direct an implementer to use the wrong number or an incomplete set. No exception applies: the shared source is readable by the intended agent, the statement is current procedural guidance rather than a dated record, and the number is the set size, not a capacity argument. Refer to the verification checks in skills/reengineering/SKILL.md in the completion condition and keep any implementation-specific method detail without presenting another complete list or count. The claim would be refuted if the shared skill were not the authoritative checklist; the explicit call to that skill and matching category entries establish that relationship.

lens restated-sets · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41Q3PXT3NF16JAX08QTBMVF of review 01M41PW2V1NMN8WKZDND4XDBEB

<!-- review:claim:01M41Q3PXT3NF16JAX08QTBMVF --> **high** — Completion condition restates the shared verification checklist size > I read this implementation reference, `skills/reengineer-program/SKILL.md`, and the shared `skills/reengineering/SKILL.md`. The program skill explicitly calls the shared reengineering skill. Its Verify section is the defining source: `Use three checks:` followed by `Retained promises`, `Dropped promises`, and `Replaced or added promises`. This reference repeats that three-member checklist under its own `Check three distinct contracts:` heading, then hard-codes the size again as `all three checks` in the anchored completion condition. This is a static trace, not a run. If the shared verification set gains or loses a check, this completion condition and its local checklist can silently direct an implementer to use the wrong number or an incomplete set. No exception applies: the shared source is readable by the intended agent, the statement is current procedural guidance rather than a dated record, and the number is the set size, not a capacity argument. Refer to the verification checks in `skills/reengineering/SKILL.md` in the completion condition and keep any implementation-specific method detail without presenting another complete list or count. The claim would be refuted if the shared skill were not the authoritative checklist; the explicit call to that skill and matching category entries establish that relationship. lens `restated-sets` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41Q3PXT3NF16JAX08QTBMVF` of review `01M41PW2V1NMN8WKZDND4XDBEB`
jercik marked this conversation as resolved
@ -35,3 +35,3 @@
## Fix direction and missing features
Fix docs to match code by default. Change code only for a confirmed **code-drift** where the doc is the intended source of truth. A third case hides inside "incorrect": the doc describes a capability that _should_ exist but doesn't (an endpoint, a flag). Removing the claim and _implementing_ the feature are both valid — surface the fork to the user, and if the claim is removed, flag the missing feature as separate work rather than silently dropping it. ADRs are decision records, not behaviour docs. A `proposed` ADR is an open question: correct its statement of current behaviour if the code contradicts it, never delete it as stale. An `accepted` ADR is the source of truth for the decision it records: code contradicting it is `code-drift`, and changing the decision means superseding the ADR per `project-docs`, never editing it to match the code.
Fix docs to match code by default. Change code only for a confirmed **code-drift** where the doc is the intended source of truth. A third case hides inside "incorrect": the doc describes a capability that _should_ exist but doesn't (an endpoint, a flag). Removing the claim and _implementing_ the feature are both valid — surface the fork to the user, and if the claim is removed, flag the missing feature as separate work rather than silently dropping it. ADRs are decision records, not behaviour docs. A `proposed` ADR is an open question: correct its statement of current behaviour if the code contradicts it, never delete it as stale. An `accepted` or status-less ADR is the source of truth for the decision it records: code contradicting it is `code-drift`. Changing the decision is the user's call, after which the ADR is edited, replaced, or deleted per `project-docs`; never edit an ADR just to match the code.

high — Doc drift guidance duplicates the ADR status set

I read the entire verify-doc-drift skill and the status rules in skills/project-docs/SKILL.md. The defining rule there says: An ADR marked accepted, or without a status, is decided and binding; proposedmeans the decision is still open. Other statuses, includingsupersededanddeprecated, are outdated and do not bind. Its ADR format also lists Status frontmatter (proposed | accepted) for new records. The anchored paragraph copies the named status cases into this separate skill, including the newly added status-less case. It currently agrees for the cases it names but omits the outdated-status case, so this apparent status-handling summary can direct a doc-drift audit to its generic fix-to-code default instead of the canonical ADR maintenance rule; later status changes would also have to be synchronized here. This is a static comparison, not a run. No exception applies: the intended agent can open project-docs, this is current procedure rather than a dated record, and the copied status labels are not examples. Replace the status enumeration with a reference to project-docs status classification, then retain this skill’s own instructions in terms of open and binding decisions without listing the status values again. A refutation would need evidence that this skill, rather than project-docs, defines ADR status semantics; its explicit per project-docs reference and the canonical status rules point the other way.

lens restated-sets · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41Q4WAY5EARNZ1HW1KVX1QX of review 01M41PW2V1NMN8WKZDND4XDBEB

<!-- review:claim:01M41Q4WAY5EARNZ1HW1KVX1QX --> **high** — Doc drift guidance duplicates the ADR status set > I read the entire `verify-doc-drift` skill and the status rules in `skills/project-docs/SKILL.md`. The defining rule there says: `An ADR marked `accepted`, or without a status, is decided and binding; `proposed` means the decision is still open. Other statuses, including `superseded` and `deprecated`, are outdated and do not bind`. Its ADR format also lists `Status frontmatter (`proposed | accepted`)` for new records. The anchored paragraph copies the named status cases into this separate skill, including the newly added status-less case. It currently agrees for the cases it names but omits the outdated-status case, so this apparent status-handling summary can direct a doc-drift audit to its generic fix-to-code default instead of the canonical ADR maintenance rule; later status changes would also have to be synchronized here. This is a static comparison, not a run. No exception applies: the intended agent can open `project-docs`, this is current procedure rather than a dated record, and the copied status labels are not examples. Replace the status enumeration with a reference to `project-docs` status classification, then retain this skill’s own instructions in terms of open and binding decisions without listing the status values again. A refutation would need evidence that this skill, rather than `project-docs`, defines ADR status semantics; its explicit `per project-docs` reference and the canonical status rules point the other way. lens `restated-sets` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41Q4WAY5EARNZ1HW1KVX1QX` of review `01M41PW2V1NMN8WKZDND4XDBEB`
Author
Owner

At 8e30def, verify-doc-drift has no declared project-docs dependency and never invokes that skill. A prose per-project-docs reference does not establish delivery or loading. writing-for-agents explicitly permits short required material from undeclared skills that readers may not have, and axskills guarantees delivery only for declared dependencies. The local paragraph preserves the audit-specific open-versus-binding repair direction and agrees with the canonical meaning for each case it covers. No operative unknown/outdated ADR case or wrong audit outcome was reproduced. Deleting the local classification solely because status names recur would remove needed context from a standalone reader; the claimed no-exception premise is false.

<!-- gh-feedback:reply-to:110620 --> At 8e30def, verify-doc-drift has no declared project-docs dependency and never invokes that skill. A prose per-project-docs reference does not establish delivery or loading. writing-for-agents explicitly permits short required material from undeclared skills that readers may not have, and axskills guarantees delivery only for declared dependencies. The local paragraph preserves the audit-specific open-versus-binding repair direction and agrees with the canonical meaning for each case it covers. No operative unknown/outdated ADR case or wrong audit outcome was reproduced. Deleting the local classification solely because status names recur would remove needed context from a standalone reader; the claimed no-exception premise is false.

superseded by review 01M41SFZWKA8GGYFCW0ZY7PGEB for head bc0921e42bab9862a5108abda1377c10de421e99

<!-- review:superseded:01M41SFZWKA8GGYFCW0ZY7PGEB --> superseded by review `01M41SFZWKA8GGYFCW0ZY7PGEB` for head `bc0921e42bab9862a5108abda1377c10de421e99`
Author
Owner

Replying to review comment #110619

The repeated completion count is tracked in #104. That independently based follow-up refers to the verification checks defined above; it has not merged into this PR.

The local verification procedure also adds program-specific obligations absent from the shared category list: kept-surface snapshots and surviving tests, and rejection or unrepresentability of dropped situations. Those details remain useful implementation instructions. Matching category labels alone do not establish that deleting this procedure preserves its requirements, so the follow-up retains those details.

> Replying to review comment #110619 The repeated completion count is tracked in https://code.j4k.dev/j4k-oss/agent-skills/pulls/104. That independently based follow-up refers to the verification checks defined above; it has not merged into this PR. The local verification procedure also adds program-specific obligations absent from the shared category list: kept-surface snapshots and surviving tests, and rejection or unrepresentability of dropped situations. Those details remain useful implementation instructions. Matching category labels alone do not establish that deleting this procedure preserves its requirements, so the follow-up retains those details.
Author
Owner

Replying to review comment #110343

Tracked in #105. That follow-up replaces copied maintenance options with references to the declared, loaded project-docs rules while retaining the user-answer triggers. It is based on this PR's 8e30deffaa because those triggers differ from current main. Its unmerged source changes are not fixes in this PR.

> Replying to review comment #110343 Tracked in https://code.j4k.dev/j4k-oss/agent-skills/pulls/105. That follow-up replaces copied maintenance options with references to the declared, loaded `project-docs` rules while retaining the user-answer triggers. It is based on this PR's 8e30deffaacb6c045b09ace982a6db57d95bd269 because those triggers differ from current main. Its unmerged source changes are not fixes in this PR.
Author
Owner

Replying to review comment #101438

The loading/admission duplication claims 01M41NSPBCZ0RT5CJ631H1466R and 01M41NT9GH2SCY2K5Q8GPW3487 are tracked in #106. The skill already declares and invokes project-docs; the follow-up refers to its loading and admission procedures while retaining the interview trigger and timing. Its two-line change is independently based on main and is not yet merged into this PR.

> Replying to review comment #101438 The loading/admission duplication claims `01M41NSPBCZ0RT5CJ631H1466R` and `01M41NT9GH2SCY2K5Q8GPW3487` are tracked in https://code.j4k.dev/j4k-oss/agent-skills/pulls/106. The skill already declares and invokes `project-docs`; the follow-up refers to its loading and admission procedures while retaining the interview trigger and timing. Its two-line change is independently based on main and is not yet merged into this PR.
fix(skills): integrate current reengineering workflows
All checks were successful
commit-msg / commitlint (pull_request) Successful in 23s
Node tests / node:test (pull_request) Successful in 3m7s
Review / Review (pull_request_target) Successful in 8m54s
bc0921e42b
@ -58,3 +58,3 @@
- **Use the glossary's canonical terms** in everything you produce — plans, code, commit messages. When the user's wording conflicts with a defined term, flag the mismatch instead of silently adopting either side.
- **Don't re-litigate recorded ADRs.** A status-less ADR counts as accepted; only an ADR marked `proposed` is still open, and a `deprecated` or `superseded` one no longer binds — its successor, where one exists, governs instead. If the plan contradicts a binding ADR, name it ("this conflicts with ADR-0007") and either adjust the plan or supersede the ADR — never silently override a recorded decision.
- **Don't re-litigate recorded ADRs.** An ADR marked `accepted`, or without a status, is decided and binding; `proposed` means the decision is still open. Other statuses, including `superseded` and `deprecated`, are outdated and do not bind: flag them and use the maintenance rules when that cleanup is authorized. Read any current ADR an outdated record points to and apply these status rules. If the plan contradicts a binding ADR, name it ("this conflicts with ADR-0007"), then either adjust the plan or, on the user's decision, change the ADR per the maintenance rules — never silently override a recorded decision.

high — ADR loading rule repeats the complete status value set

I read the changed ADR loading rule and the ADR format section in skills/project-docs/SKILL.md. The format section defines the valid frontmatter values as ‘Status frontmatter (proposed | accepted)’. The loading rule separately names both values to explain their meanings, so a future edit to the format set can leave the loading procedure silently incomplete. The values and their semantics are in the same accessible skill, and this is not a dated record or an example subset. Put the open/binding meaning beside each value in the ADR format definition, then have the loading rule say to apply that definition; it can separately say that an ADR without status is binding and how to handle outdated values. Refutation would be a separate system-readable schema defining the status values independently; I found none in the subject.

lens restated-sets · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41SRZK1AA7QXAW0CH87CMZE of review 01M41SFZWKA8GGYFCW0ZY7PGEB

<!-- review:claim:01M41SRZK1AA7QXAW0CH87CMZE --> **high** — ADR loading rule repeats the complete status value set > I read the changed ADR loading rule and the ADR format section in `skills/project-docs/SKILL.md`. The format section defines the valid frontmatter values as ‘**Status** frontmatter (`proposed | accepted`)’. The loading rule separately names both values to explain their meanings, so a future edit to the format set can leave the loading procedure silently incomplete. The values and their semantics are in the same accessible skill, and this is not a dated record or an example subset. Put the open/binding meaning beside each value in the ADR format definition, then have the loading rule say to apply that definition; it can separately say that an ADR without status is binding and how to handle outdated values. Refutation would be a separate system-readable schema defining the status values independently; I found none in the subject. lens `restated-sets` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41SRZK1AA7QXAW0CH87CMZE` of review `01M41SFZWKA8GGYFCW0ZY7PGEB`

medium — Statusless reengineering backlog becomes binding without migration

I traced the changed status rule through project-docs, reengineer-program, and verify-doc-drift. The prior reengineer-program instruction explicitly treated status-less ADRs as proposed and left them unchanged until the user answered; the new text removes that exception while project-docs now says every statusless ADR is decided and binding. Thus an existing statusless ADR that was intentionally left in the reengineering decision backlog is now taken as an approved requirement: reengineer-program may use it to permit design or implementation, and verify-doc-drift will classify code that disagrees with it as code drift. This conclusion follows from the served before/after text; this snapshot contains no consumer project's ADRs, so the presence of affected backlog records in a particular project is unverified. Preserve the old interpretation for those records or require an explicit migration/confirmation before treating them as binding.

lens general-bug · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41SN326AB5VY88DAFHFGAQ8 of review 01M41SFZWKA8GGYFCW0ZY7PGEB

<!-- review:claim:01M41SN326AB5VY88DAFHFGAQ8 --> **medium** — Statusless reengineering backlog becomes binding without migration > I traced the changed status rule through `project-docs`, `reengineer-program`, and `verify-doc-drift`. The prior `reengineer-program` instruction explicitly treated status-less ADRs as proposed and left them unchanged until the user answered; the new text removes that exception while `project-docs` now says every statusless ADR is decided and binding. Thus an existing statusless ADR that was intentionally left in the reengineering decision backlog is now taken as an approved requirement: `reengineer-program` may use it to permit design or implementation, and `verify-doc-drift` will classify code that disagrees with it as code drift. This conclusion follows from the served before/after text; this snapshot contains no consumer project's ADRs, so the presence of affected backlog records in a particular project is unverified. Preserve the old interpretation for those records or require an explicit migration/confirmation before treating them as binding. lens `general-bug` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41SN326AB5VY88DAFHFGAQ8` of review `01M41SFZWKA8GGYFCW0ZY7PGEB`
Author
Owner

The proposed mandatory migration contradicts the approved statusless-binding contract. The old source explicitly required unrecorded roots to be created as proposed and answered roots to become accepted; no unanswered statusless record is supplied here. Our bounded owned-record check found no such case and does not claim a complete historical census. A concrete operative backlog record would warrant investigation, but the earlier conflicting fallback alone does not establish one. The independent narrow-contract calibration also confirmed authority applies before any optional metadata edit.

<!-- gh-feedback:reply-to:111108 --> The proposed mandatory migration contradicts the approved statusless-binding contract. The old source explicitly required unrecorded roots to be created as proposed and answered roots to become accepted; no unanswered statusless record is supplied here. Our bounded owned-record check found no such case and does not claim a complete historical census. A concrete operative backlog record would warrant investigation, but the earlier conflicting fallback alone does not establish one. The independent narrow-contract calibration also confirmed authority applies before any optional metadata edit.
@ -3,3 +3,3 @@
Apply the shared reengineering skill's required design-abstractions reading before preparing the briefs.
Have three fresh-context sub-agents independently design the scope. Give all three the loaded modeling guidance and `CONTEXT.md`, the boundary ADR, and accepted ADRs as the sole sources of requirements. For a narrower-than-program scope, include exact boundary signatures. Keep the repository outside their reading scope. Give each a different constraint:
Have three fresh-context sub-agents independently design the scope. Give all three the loaded modeling guidance and `CONTEXT.md`, the boundary ADR, and binding ADRs as the sole sources of requirements. For a narrower-than-program scope, include exact boundary signatures. Keep the repository outside their reading scope. Give each a different constraint:

high — Design brief count duplicates the constraint list

I read the changed opening instruction and the constraint list directly below it in skills/reengineer-program/references/compare-designs.md. The list defines the briefs: ‘Smallest model,’ ‘Smallest interface,’ and ‘Easiest common case’; the next instruction says to give each designer a different constraint. ‘Three’ copies the size of that list, so adding or removing a design constraint can silently leave the number of agents wrong. The list is accessible in the same file, is exhaustive for this procedure, and is neither a dated record nor a table of contents. Replace the count with ‘Have a fresh-context sub-agent independently design the scope for each constraint below.’ Refutation would require a separate requirement for exactly three agents regardless of how many constraints the list defines; the procedure supplies no such rationale.

lens restated-sets · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41SPMTDFYW4XN25RCP5C0F1 of review 01M41SFZWKA8GGYFCW0ZY7PGEB

<!-- review:claim:01M41SPMTDFYW4XN25RCP5C0F1 --> **high** — Design brief count duplicates the constraint list > I read the changed opening instruction and the constraint list directly below it in `skills/reengineer-program/references/compare-designs.md`. The list defines the briefs: ‘Smallest model,’ ‘Smallest interface,’ and ‘Easiest common case’; the next instruction says to give each designer a different constraint. ‘Three’ copies the size of that list, so adding or removing a design constraint can silently leave the number of agents wrong. The list is accessible in the same file, is exhaustive for this procedure, and is neither a dated record nor a table of contents. Replace the count with ‘Have a fresh-context sub-agent independently design the scope for each constraint below.’ Refutation would require a separate requirement for exactly three agents regardless of how many constraints the list defines; the procedure supplies no such rationale. lens `restated-sets` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41SPMTDFYW4XN25RCP5C0F1` of review `01M41SFZWKA8GGYFCW0ZY7PGEB`
Author
Owner

The opening sentence is the existing normative requirement for three independent designers, carried from main e7d02b520e, followed by the named constraints assigned to those designers. This integration was explicitly required to preserve that design requirement; replacing it with an automatically variable agent count would change policy. The incidental repeated “all three” audience wording and completion-check count are owned by #104. That follow-up does not remove the normative opening count, and I am not claiming the broader allegation fully implemented.

<!-- gh-feedback:reply-to:111104 --> The opening sentence is the existing normative requirement for three independent designers, carried from main e7d02b520e58f0f41803193e983012270fec38ec, followed by the named constraints assigned to those designers. This integration was explicitly required to preserve that design requirement; replacing it with an automatically variable agent count would change policy. The incidental repeated “all three” audience wording and completion-check count are owned by https://code.j4k.dev/j4k-oss/agent-skills/pulls/104. That follow-up does not remove the normative opening count, and I am not claiming the broader allegation fully implemented.
@ -34,2 +33,3 @@
Have a fresh-context sub-agent attack the result from both directions: find situations the old code handles that the rebuild omits without a binding ADR, and promises the rebuild makes that no binding ADR covers. Mechanism-only differences and replacements chosen by a binding ADR are not findings.
Put each uncovered situation to the user and record the answer in an added or superseding ADR before adding, removing, or keeping its promise. Completion requires the implementation to satisfy the confirmed design, all three checks to pass, and every review finding to be resolved.
Put each uncovered situation to the user and record the answer in a new or edited ADR before adding, removing, or keeping its promise. Completion requires the implementation to satisfy the confirmed design, all three checks to pass, and every review finding to be resolved.

high — Completion rule copies the number of verification checks

I read the changed completion sentence in skills/reengineer-program/references/implementation.md, its Verify section, and skills/reengineering/SKILL.md. The shared skill defines the set under ‘Use three checks:’ with entries ‘Retained promises,’ ‘Dropped promises,’ and ‘Replaced or added promises’; the implementation reference repeats that structure as ‘Unchanged promises,’ ‘Dropped promises,’ and ‘Replaced or added promises.’ The changed sentence’s ‘all three checks’ copies the set size. If the shared verification procedure changes, the count can silently misstate the completion gate. The source is accessible to this workflow, and this is a set size rather than a number used in an argument. Say ‘Completion requires the implementation to satisfy the confirmed design, the verification checks specified by the shared reengineering skill to pass, and every review finding to be resolved.’ A separate, fixed three-check contractual requirement would refute this, but I found none beyond the list itself.

lens restated-sets · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41SQ0VYDZ9TZG3GVQ92RVYJ of review 01M41SFZWKA8GGYFCW0ZY7PGEB

<!-- review:claim:01M41SQ0VYDZ9TZG3GVQ92RVYJ --> **high** — Completion rule copies the number of verification checks > I read the changed completion sentence in `skills/reengineer-program/references/implementation.md`, its Verify section, and `skills/reengineering/SKILL.md`. The shared skill defines the set under ‘Use three checks:’ with entries ‘Retained promises,’ ‘Dropped promises,’ and ‘Replaced or added promises’; the implementation reference repeats that structure as ‘Unchanged promises,’ ‘Dropped promises,’ and ‘Replaced or added promises.’ The changed sentence’s ‘all three checks’ copies the set size. If the shared verification procedure changes, the count can silently misstate the completion gate. The source is accessible to this workflow, and this is a set size rather than a number used in an argument. Say ‘Completion requires the implementation to satisfy the confirmed design, the verification checks specified by the shared `reengineering` skill to pass, and every review finding to be resolved.’ A separate, fixed three-check contractual requirement would refute this, but I found none beyond the list itself. lens `restated-sets` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41SQ0VYDZ9TZG3GVQ92RVYJ` of review `01M41SFZWKA8GGYFCW0ZY7PGEB`
jercik marked this conversation as resolved
@ -35,3 +35,3 @@
## Fix direction and missing features
Fix docs to match code by default. Change code only for a confirmed **code-drift** where the doc is the intended source of truth. A third case hides inside "incorrect": the doc describes a capability that _should_ exist but doesn't (an endpoint, a flag). Removing the claim and _implementing_ the feature are both valid — surface the fork to the user, and if the claim is removed, flag the missing feature as separate work rather than silently dropping it. ADRs are decision records, not behaviour docs. A `proposed` ADR is an open question: correct its statement of current behaviour if the code contradicts it, never delete it as stale. An `accepted` ADR is the source of truth for the decision it records: code contradicting it is `code-drift`, and changing the decision means superseding the ADR per `project-docs`, never editing it to match the code.
Fix docs to match code by default. Change code only for a confirmed **code-drift** where the doc is the intended source of truth. A third case hides inside "incorrect": the doc describes a capability that _should_ exist but doesn't (an endpoint, a flag). Removing the claim and _implementing_ the feature are both valid — surface the fork to the user, and if the claim is removed, flag the missing feature as separate work rather than silently dropping it. ADRs are decision records, not behaviour docs. A `proposed` ADR is an open question: correct its statement of current behaviour if the code contradicts it, never delete it as stale. An `accepted` or status-less ADR is the source of truth for the decision it records: code contradicting it is `code-drift`. Changing the decision is the user's call, after which the ADR is edited, replaced, or deleted per `project-docs`; never edit an ADR just to match the code.

high — verify-doc-drift copies the binding ADR status set

I read the changed fix-direction paragraph in skills/verify-doc-drift/SKILL.md and the ADR loading and maintenance rules in skills/project-docs/SKILL.md. The latter is the canonical source for this classification: ‘An ADR marked accepted, or without a status, is decided and binding; proposed means the decision is still open.’ The new verify-doc-drift sentence repeats the members of the binding-status set. It currently agrees, but a later change to project-docs can leave this audit skill silently applying code-drift rules to the wrong ADRs. The reader can load the referenced project-docs skill, so the inaccessible-source exception does not apply. Say ‘A binding ADR, as defined by project-docs, is the source of truth for the decision it records: code contradicting it is code-drift.’ The decisive refutation would be another authoritative source for this skill’s independent status classification; I found none in the snapshot.

lens restated-sets · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41SNRHJN7GVSFZ4JS21YZ9H of review 01M41SFZWKA8GGYFCW0ZY7PGEB

<!-- review:claim:01M41SNRHJN7GVSFZ4JS21YZ9H --> **high** — verify-doc-drift copies the binding ADR status set > I read the changed fix-direction paragraph in `skills/verify-doc-drift/SKILL.md` and the ADR loading and maintenance rules in `skills/project-docs/SKILL.md`. The latter is the canonical source for this classification: ‘An ADR marked `accepted`, or without a status, is decided and binding; `proposed` means the decision is still open.’ The new verify-doc-drift sentence repeats the members of the binding-status set. It currently agrees, but a later change to project-docs can leave this audit skill silently applying code-drift rules to the wrong ADRs. The reader can load the referenced project-docs skill, so the inaccessible-source exception does not apply. Say ‘A binding ADR, as defined by `project-docs`, is the source of truth for the decision it records: code contradicting it is `code-drift`.’ The decisive refutation would be another authoritative source for this skill’s independent status classification; I found none in the snapshot. lens `restated-sets` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41SNRHJN7GVSFZ4JS21YZ9H` of review `01M41SFZWKA8GGYFCW0ZY7PGEB`

high — ADR drift rule repeats project-docs maintenance actions

I read the changed fix-direction paragraph in skills/verify-doc-drift/SKILL.md and the ‘Keep only current decisions’ rule in skills/project-docs/SKILL.md. The latter defines the maintenance actions: ‘When a decision changes, edit its ADR to state the new decision. When a new decision replaces it outright, delete the old ADR and add a new one. Delete an ADR whose decision no longer applies.’ The verify-doc-drift sentence copies that action set as ‘edited, replaced, or deleted.’ It currently agrees at a high level, but a change to the canonical maintenance rule can leave the audit skill prescribing stale options. The project-docs skill is directly named and accessible, so the reader does not need the copied list. Say ‘Changing the decision is the user’s call; then maintain the ADR according to project-docs. Never edit an ADR just to match the code.’ A distinct maintenance policy for drift audits would refute this, but the sentence explicitly delegates to project-docs.

lens restated-sets · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41STGVHDAQ8A4Z39KBR3HR6 of review 01M41SFZWKA8GGYFCW0ZY7PGEB

<!-- review:claim:01M41STGVHDAQ8A4Z39KBR3HR6 --> **high** — ADR drift rule repeats project-docs maintenance actions > I read the changed fix-direction paragraph in `skills/verify-doc-drift/SKILL.md` and the ‘Keep only current decisions’ rule in `skills/project-docs/SKILL.md`. The latter defines the maintenance actions: ‘When a decision changes, edit its ADR to state the new decision. When a new decision replaces it outright, delete the old ADR and add a new one. Delete an ADR whose decision no longer applies.’ The verify-doc-drift sentence copies that action set as ‘edited, replaced, or deleted.’ It currently agrees at a high level, but a change to the canonical maintenance rule can leave the audit skill prescribing stale options. The project-docs skill is directly named and accessible, so the reader does not need the copied list. Say ‘Changing the decision is the user’s call; then maintain the ADR according to `project-docs`. Never edit an ADR just to match the code.’ A distinct maintenance policy for drift audits would refute this, but the sentence explicitly delegates to project-docs. lens `restated-sets` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41STGVHDAQ8A4Z39KBR3HR6` of review `01M41SFZWKA8GGYFCW0ZY7PGEB`

medium — ADR maintenance refers to a skill that this audit never loads

I read the changed ADR guidance in verify-doc-drift/SKILL.md, the maintenance rules in project-docs/SKILL.md, and the skill-packaging standard. The audit skill delegates the decision-change procedure to project-docs, but its frontmatter has no axskills.requires dependency and its body never calls the Skill tool to load it. That procedure contains consequential choices: edit an existing ADR versus delete and replace it, update links after deletion, and preserve the user's decision rather than rewrite it to match drift. An agent invoked only for this audit can reach the changed sentence without those rules and make an incomplete ADR change, such as deleting an old ADR while leaving links to it. The packaging standard says to declare hard dependencies and call the skill at the point of use. Declare project-docs as a dependency and load it before ADR maintenance, while keeping the user's decision gate. A delivery contract that always supplies and loads project-docs for this skill would refute the concern; neither guarantee appears in this file.

lens writing-quality · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41SR8B4BRF263JVRJ39D11C of review 01M41SFZWKA8GGYFCW0ZY7PGEB

<!-- review:claim:01M41SR8B4BRF263JVRJ39D11C --> **medium** — ADR maintenance refers to a skill that this audit never loads > I read the changed ADR guidance in `verify-doc-drift/SKILL.md`, the maintenance rules in `project-docs/SKILL.md`, and the skill-packaging standard. The audit skill delegates the decision-change procedure to `project-docs`, but its frontmatter has no `axskills.requires` dependency and its body never calls the Skill tool to load it. That procedure contains consequential choices: edit an existing ADR versus delete and replace it, update links after deletion, and preserve the user's decision rather than rewrite it to match drift. An agent invoked only for this audit can reach the changed sentence without those rules and make an incomplete ADR change, such as deleting an old ADR while leaving links to it. The packaging standard says to declare hard dependencies and call the skill at the point of use. Declare `project-docs` as a dependency and load it before ADR maintenance, while keeping the user's decision gate. A delivery contract that always supplies and loads `project-docs` for this skill would refute the concern; neither guarantee appears in this file. lens `writing-quality` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41SR8B4BRF263JVRJ39D11C` of review `01M41SFZWKA8GGYFCW0ZY7PGEB`
jercik marked this conversation as resolved
Author
Owner

Replying to review comment #111105

Tracked in #104. That follow-up removes the copied count while preserving the implementation-specific verification methods. It will be integrated against main after this PR merges; it is not a fix already present in this head.

> Replying to review comment #111105 Tracked in https://code.j4k.dev/j4k-oss/agent-skills/pulls/104. That follow-up removes the copied count while preserving the implementation-specific verification methods. It will be integrated against main after this PR merges; it is not a fix already present in this head.
Author
Owner

Replying to review comment #111103

The same-file status-definition consolidation is owned by #107. It is prepared from default and will preserve the binding statusless/accepted and open proposed meanings when integrated after this PR. This parent retains the approved semantics; the unmerged follow-up is not claimed as fixed here.

> Replying to review comment #111103 The same-file status-definition consolidation is owned by https://code.j4k.dev/j4k-oss/agent-skills/pulls/107. It is prepared from default and will preserve the binding statusless/accepted and open proposed meanings when integrated after this PR. This parent retains the approved semantics; the unmerged follow-up is not claimed as fixed here.
Author
Owner

Replying to review comment #111106

The canonical dependency and status reference are owned together by #108. Its explicit dependency/invocation supplies project-docs before relying on the shared binding classification; it preserves audit-specific drift mapping. This parent is unchanged and the follow-up remains unmerged.

> Replying to review comment #111106 The canonical dependency and status reference are owned together by https://code.j4k.dev/j4k-oss/agent-skills/pulls/108. Its explicit dependency/invocation supplies project-docs before relying on the shared binding classification; it preserves audit-specific drift mapping. This parent is unchanged and the follow-up remains unmerged.
Author
Owner

Replying to review comment #111107

The maintenance-action reference is owned by #108 together with the missing canonical dependency/invocation. The repair retains the user decision gate and the prohibition on changing intent merely to match code. It is a separate unmerged follow-up.

> Replying to review comment #111107 The maintenance-action reference is owned by https://code.j4k.dev/j4k-oss/agent-skills/pulls/108 together with the missing canonical dependency/invocation. The repair retains the user decision gate and the prohibition on changing intent merely to match code. It is a separate unmerged follow-up.
Author
Owner

Replying to review comment #111109

Verified on default e7d02b520e as well: verify-doc-drift names project-docs for ADR maintenance but declares no dependency and never invokes it. #108 owns the explicit dependency and point-of-use invocation plus coherent status/action references. This delivery gap is acknowledged as independently owned, not silently counted as fixed by this parent.

> Replying to review comment #111109 Verified on default e7d02b520e58f0f41803193e983012270fec38ec as well: verify-doc-drift names project-docs for ADR maintenance but declares no dependency and never invokes it. https://code.j4k.dev/j4k-oss/agent-skills/pulls/108 owns the explicit dependency and point-of-use invocation plus coherent status/action references. This delivery gap is acknowledged as independently owned, not silently counted as fixed by this parent.
jercik merged commit b93e79a33d into main 2026-10-03 22:06:15 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
j4k-oss/agent-skills!98
No description provided.