Skip to content

Documentation upgrade to "VERIFIED" based on source-reading; MEMORY.md wanted a live-run confirmation before repointing v3 #151

Description

@twistedmelonman

Warning

Unverified claim. This finding asserts that something is
incorrect, broken, or nonexistent, but carries no VERIFIED:
field recording a command that was actually run to check it.
Confirm the assertion against reality before acting on it — several
such findings have turned out to be false.

Non-Blocking Review Concern: Documentation upgrade to "VERIFIED" based on source-reading; MEMORY.md wanted a live-run confirmation before repointing v3

Source: pre-push whole-codebase review
Location: .github/workflows/claude-assistant.yml:57-73, README.md:476-496
Date: 2026-08-18

What was flagged

MEMORY.md (open thread "Reviewer judgment gates") notes disallowed_tools precedence was "documented as UNVERIFIED — needs one live @claude run before repointing v3". This diff upgrades the claim to VERIFIED using source-code analysis alone (no live run). The analysis is methodologically sound — the same source-reading approach correctly characterized allowed_tools as additive — and the diff is honest about what it does not establish (deny-beats-allow when a tool appears in both lists). The concern is not that the claim is wrong, but that the MEMORY.md gating condition (live run) was not satisfied, and the v3 tag may be repointed on the basis of this diff. If you intend to repoint v3 as a follow-on, confirm whether source-reading alone meets the bar you set, or run a live @claude invocation with disallowed_tools set to a known tool and verify the tool is actually blocked.

Context

This issue was automatically created from a non-blocking concern identified
during pre-push whole-codebase review. It was flagged for tracking.


Created by lib-review-issues.sh

Activity

  1. twistedmelonman commented on Aug 19, 2026

    @twistedmelonman
    MemberAuthor

    Reviewed while triaging the low-effort backlog (#153). The gating question this raises has already been answered — deliberately, in favor of source-reading.

    To restate the concern fairly: MEMORY.md said disallowed_tools precedence needed "one live @claude run before repointing v3", and #152 upgraded the claim to VERIFIED using source analysis alone. The worry is not that the claim is wrong, but that the bar was moved without saying so.

    Where it landed

    #152 answered this explicitly rather than silently. Its commit message states the method up front — resolved "by reading claude-code-action at 9d7150bc (v1.0.193), the SHA this workflow pins — the same method that settled allowed_tools, rather than by burning a live @claude run." So the substitution was a stated decision, not an oversight.

    The decisive fact that makes source-reading sufficient here: tag mode emits no denylist at all. src/modes/tag/index.ts:186 emits only --allowedTools; grepping the action for disallowedTools finds 0 occurrences in tag mode and 0 in agent mode. There is no competing value, so there is no precedence contest to lose — which is exactly why this input does not repeat the allowed_tools surprise. That input could only ever ADD because tag mode emits its own allowlist and the parser unions; the denylist has no counterpart to union against.

    A live run would have confirmed a behavior that has no mechanism to vary. That is a defensible reason to spend the source-reading rather than the CI cycle.

    What is still not claimed

    The deny-beats-allow rule for a tool named in both lists remains explicitly unestablished, and the workflow comment says so in place. That is Claude Code CLI behavior, outside this action, and unreachable through this input alone — it takes a caller contradicting its own two inputs to get there.

    On the v3 question specifically

    The repoint has already happened: v3 and v1 both point at e51e6b1 (the #152 merge), and v3.2.0 is cut. So the outcome this issue was gating is in the field. MEMORY.md now records the precedence as VERIFIED against v1.0.193.

    Note that today's #153 merge (6614b5f) is not in any tag — it is documentation and a maintenance-script fix with no consumer-facing behavior, so it needs no release.

    Leaving open for your call on whether the source-reading standard is the one you want recorded going forward, since that is a standing methodology question rather than a defect in this diff.

  2. claude commented on Aug 19, 2026

    @claude

    Claude finished @smartwatermelon's task in 2m 7s —— View job


    Tasks

    What I checked

    I independently re-read the two locations cited:

    • .github/workflows/claude-assistant.yml:43-73 — the header comment states the disallowed_tools VERIFIED claim against claude-code-action v1.0.193, cites src/modes/tag/index.ts:186 (only --allowedTools emitted) and base-action/src/parse-sdk-options.ts:245-266,321-322, and explicitly flags the deny-beats-allow-when-both-lists-name-a-tool case as not established.
    • README.md:470-509 — same content, same caveat, same SHA.

    Both match the trigger comment's description exactly. One thing I could not verify myself: the claim that v3/v1 both point at e51e6b1 and v3.2.0 is cut — this sandbox has no network access (git ls-remote, gh api both require approval I can't grant), so I can't confirm tag state independently. I have no reason to doubt it, but flagging that it's your report, not mine.

    On the methodology question

    I'd endorse source-reading as sufficient here, and as the standing rule going forward — but on a narrower basis than "source-reading matches the method that worked for allowed_tools last time":

    The two claims are different in kind, and that's what makes the substitution safe. allowed_tools was a claim about interaction — what happens when the caller's list and the action's own baseline list are both in play. That can only really be confirmed by watching the union happen (or by reading the merge logic and trusting it, which is what was actually done — also source-reading, for what it's worth). disallowed_tools's tag-mode claim is a claim about absence — that no code path in tag mode or agent mode emits a competing denylist. A live run only samples one execution path; it can't prove absence any better than an exhaustive grep already does. So for this specific claim, source-reading isn't a weaker substitute for a live run — it's actually the more rigorous check.

    The line I'd draw: source-reading against the exact pinned SHA is sufficient for structural claims (a path exists / doesn't exist, an input is or isn't wired through, grep finds zero occurrences project-wide). A live run stays required for behavioral claims — anything where two inputs interact at runtime and reading isolated line ranges doesn't tell you the outcome of that interaction. The still-open deny-beats-allow case is exactly that second kind, and it's correctly left unverified in both docs rather than papered over.

    So: I'd record this as the standing methodology rather than a one-off exception, conditioned on what this diff already does — every VERIFIED note states the method and the exact pinned SHA it was checked against, and explicitly separates what's structurally proven from what remains behaviorally open. That transparency is what keeps "VERIFIED" meaningful even when the verification wasn't a live run.

    Outcome

    No code changes needed — the documentation already accurately reflects this. Since the repoint you describe has already happened and the substance of the gate has been satisfied, this looks safe to close on your side; I don't have issue-closing in my toolset here so I'll leave that call to you.

  3. twistedmelonman commented on Aug 19, 2026

    @twistedmelonman
    MemberAuthor

    Closing as designed.

    The gating condition was satisfied deliberately rather than skipped: #152 substituted source-reading for the live @claude run and said so explicitly in its commit message, on the grounds that tag mode emits no denylist at all — so there is no precedence contest for a live run to observe. The v3 repoint this issue was guarding has since happened (v3/v1 at e51e6b1, v3.2.0 cut), and MEMORY.md records the precedence as VERIFIED against claude-code-action v1.0.193.

    The one thing still not claimed — deny-beats-allow when a tool appears in both lists — remains documented as unestablished in the workflow comment, which is the correct state for a fact that is CLI behavior outside this action.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    tech-debtTechnical debt to addressunverifiedFinding asserts a fact that was never checked

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions