docs: annotate ADR-0027 with post-v2.1.119 corrections#5203
Conversation
PR Summary by QodoAnnotate ADR-0027 with post-v2.1.119 behavior corrections
AI Description
High-Level Assessment
Files changed (1)
|
Site previewPreview: https://2bc13556-site.fullsend-ai.workers.dev Commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Code Review by Qodo
Context used✅ Tickets:
🎫 Migrate disallowedTools from agent frontmatter to .claude/settings.json permissions.deny✅ Compliance rules (platform):
54 rules✅ Skills:
writing-user-docs, writing-adrs 1.
|
ReviewFindingsMedium
Low
Previous runReviewFindingsMedium
Low
Previous run (2)ReviewFindingsMedium
Low
Previous run (3)ReviewFindingsMedium
Low
Previous run (4)ReviewFindingsMedium
Low
Previous run (5)ReviewFindingsMedium
Low
Previous run (6)ReviewFindingsMedium
Low
Labels: PR modifies documentation under Labels: PR modifies ADR documentation under docs/ADRs/ Previous run (7)ReviewFindingsMedium
Low
Labels: PR modifies documentation under docs/ADRs/ Previous run (8)ReviewVerdict: comment · 2 findings (2 medium) This PR adds three dated correction annotations to ADR-0027, noting that Claude Code v2.1.119 changed the enforcement behavior of The decision to annotate rather than rewrite is sound. However, two process concerns are worth noting. Findings1. ADR amendment scope (medium)File: The first annotation block in the Context section is 21 lines of substantive analytical content — listing three distinct bug classes, evaluating a workaround ( Suggested remediation: Consider splitting the first annotation into (a) a short note in ADR-0027 linking to the RCA discussion and noting the changed behavior (~5 lines), and (b) a companion document or new ADR that captures the detailed bug analysis, workaround evaluation, and tracking references. The second and third annotations (6 and 4 lines respectively) are well within acceptable scope. 2. Missing authorization (medium)File: The PR adds 29 lines to an Accepted ADR but has no linked issue. Per the project's intent-coherence rules, non-trivial changes (20+ lines) to governance artifacts typically require explicit authorization. The PR body references discussion #5182 for context, but this is not formally linked as authorizing work. Suggested remediation: Link an existing issue that tracks the need to document the v2.1.119 behavior changes, or create one. This is especially relevant for external contributors modifying Accepted ADRs. Additional notes
|
|
🤖 Finished Review · ✅ Success · Started 3:44 PM UTC · Completed 3:51 PM UTC |
319c8d9 to
5abd653
Compare
|
🤖 Review · |
5abd653 to
702959b
Compare
|
🤖 Finished Review · ✅ Success · Started 3:58 PM UTC · Completed 4:12 PM UTC |
702959b to
561afed
Compare
|
🤖 Review · |
561afed to
1f63101
Compare
|
🤖 Finished Review · ❌ Failure · Started 8:34 PM UTC · Completed 8:49 PM UTC |
|
/fs-review |
|
🤖 Review · ❌ Terminated · Started 9:35 AM UTC · Ended 9:50 AM UTC |
Addressing review findings (commit 1f63101)Medium — adr-immutability-violation (strikethrough, line 61): Keeping the strikethrough. The previous review round on commit 702959b explicitly recommended: "add strikethrough to the 'No effect' cells to make the correction visually obvious." The strikethrough preserves original text (visible through markup) while making the correction scannable at the table level. Removing it would reintroduce the internal-consistency finding from that same round, which flagged that "(corrected below)" was too easy to miss. This is a minimal inline cross-reference, not a rewrite. Medium — adr-immutability-violation ("invalidated", line 163): The language is factual — the claim was invalidated by v2.1.119, released the same day as the experiment it cites. The note is a 2-line neutral cross-reference using the established blockquote pattern. Low findings: adr-amendment-scope notes the Context annotation is now 7 lines with "modest" delta — no action needed. Edge-case-correctness (both |
|
🤖 Finished Review · ✅ Success · Started 10:22 AM UTC · Completed 10:35 AM UTC |
be083f8 to
fd2d529
Compare
|
🤖 Finished Review · ✅ Success · Started 10:19 AM UTC · Completed 10:33 AM UTC |
fd2d529 to
b69d7f6
Compare
|
🤖 Finished Review · ✅ Success · Started 10:34 AM UTC · Completed 10:48 AM UTC |
tools and disallowedTools are enforced in --agent sessions since Claude Code v2.1.119 (Apr 23), invalidating the ADR's claim that they are inert. However, multiple bugs make them unreliable: scoped disallowedTools patterns strip entire tools, scoped tools patterns silently grant full access, and disallowedTools is not inherited by subagents. Added dated correction notes at three locations without rewriting the original text, per ADR template guidance. Refs: fullsend-ai#303, fullsend-ai#5182 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Marta Anon <manon@redhat.com>
b69d7f6 to
7fe185a
Compare
|
🤖 Finished Review · ✅ Success · Started 10:59 AM UTC · Completed 11:12 AM UTC |
There was a problem hiding this comment.
See the review comment for full details.
Note: The following inline comments could not be posted on the diff (GitHub returned 422) and are included here instead:
docs/ADRs/0027-allowed-and-disallowed-tools-for-agents.md:61: [medium] adr-immutability-violation
Strikethrough markup applied to table cells (lines 61-62) and inline prose (lines 64-65) modifies the rendering of the original accepted ADR text. While the original words are still present, the visual modification constitutes inline editing rather than external annotation. No other ADR in the corpus uses strikethrough — all other correction notes append blockquote notes without altering original text. The appended blockquote notes in this PR are well-written and sufficient on their own to communicate the correction.
Suggested fix: Remove strikethrough from table cells (lines 61-62) and inline prose (lines 64-65). Restore original text and rely solely on the appended blockquote notes, which already follow the established annotation pattern used by other ADRs.
docs/ADRs/0027-allowed-and-disallowed-tools-for-agents.md(file-level): Line 57 · [low] internal-consistency
The section header Inert in --agent sessions — subagents only remains unannotated while the blockquote note states these keys ARE now enforced. This is not actionable because the ADR immutability rule requires preserving original text — the blockquote note serves as the amendment, which is the correct approach.
docs/ADRs/0027-allowed-and-disallowed-tools-for-agents.md:166: [low] redundant-notes
Three correction notes (Context blockquote, Decision blockquote at line 166, Consequences blockquote at line 183) all redirect to the Context section. The Decision and Consequences notes are terse forward references. Placing correction notes near each outdated claim is defensible for readers who jump directly to specific sections.
|
If you are having trouble finding the conversation that is not resolved you should know that sometimes the button does not work. I usually tell my agent and it does solve it using the graphql endpoint. I found this multiple times already. |
on it... but it's frustrating though -.- |
|
🤖 Finished Retro · ✅ Success · Started 1:55 PM UTC · Completed 2:10 PM UTC |
Retro: PR #5203 — ADR-0027 correction annotationsA 16-line docs-only PR that annotated three places in ADR-0027 where factual claims about Timeline
Key issuesSelf-contradictory recommendations (novel — see Proposal 1): The review agent suggested adding strikethrough in one round, then flagged the result as an immutability violation in the next three rounds. The root cause is that the prior-review pipeline only reads the "current section" of the sticky comment — remediation suggestions from earlier rounds are collapsed into Token cost on docs-only PRs (novel — see Proposal 2): All 8 successful review runs dispatched the full fleet of opus-tier sub-agents across all dimensions (correctness, security, intent-coherence, style-conventions, docs-currency, cross-repo-contracts). For a single-file ADR annotation, correctness, security, and cross-repo-contracts dimensions had nothing to analyze. Re-raising acknowledged findings (already tracked): The strikethrough finding was raised 3+ times despite the author posting explicit replies each time. This provides fresh evidence for agents#106 (author reply ingestion) and fullsend#3073 (re-raising after addressing). The severity anchoring rule ("preserve severity on unchanged code") created a perpetual loop with no escape valve — additional evidence for fullsend#2816 (persistent finding cache with dismissal lifecycle). Inline comment deduplication (already tracked): Multiple inline comments for the same finding across force pushes created unresolved conversation threads that blocked merge for days after human approval. This corroborates fullsend#5007 (inline comment dedup) and fullsend#1285 (unchanged inline comment regeneration). Proposals filed
|
Summary
ADR claims
toolsanddisallowedToolshave no effect in--agentsessions
but unreliable due to parsing bugs, scoping bugs, and subagent
inheritance issues
Context/Decision/Consequences)
Related to RCA discussion #5182
Authorization
Authorized by RCA discussion #5182,
which identified the stale claims and recommended corrective annotations.
Related: #303 (harness-level enforcement tracking).
Context
The ADR's claims were correct on v2.1.118 but updated hours later
by v2.1.119. The experiment it cites ran on the last unaffected version.
Full timeline and production evidence in discussion #5182.
The decision (Option A:
permissions.deny) remains correct — theannotations only correct factual claims about
tools/disallowedToolsbehavior.
Test plan
🤖 Generated with Claude Code