Skip to content

docs(source-control): correct the bare-name wrapper PATH and allow-rule rationale - #1399

Merged
kyle-sexton merged 9 commits into
mainfrom
docs/843-babysit-wrapper-bare-name-rationale
Jul 26, 2026
Merged

docs(source-control): correct the bare-name wrapper PATH and allow-rule rationale#1399
kyle-sexton merged 9 commits into
mainfrom
docs/843-babysit-wrapper-bare-name-rationale

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

No related issue: #843 stays open deliberately — see "Why #843 stays open" below.

Summary

safety.md asserted that the bundled babysit wrappers' bare names "are not on the Bash tool's
PATH", and the two bin/ wrapper headers presented their bare-command allow-rule rationale as
operative fact. Both are wrong, in opposite directions. This corrects the justification; the
operational guidance is unchanged.

What the evidence actually shows

Correction 1 — bare-name resolution is unreliable, not absent. Plugin bin/ reaches the Bash
tool's PATH only via the per-session shell snapshot's final export PATH= line. Sweeping the
local snapshot corpus (112 snapshots, 06-26 -> 07-25) found that line present-with-plugin-bins in
35, present-without in 5, and absent entirely in 72 — with four snapshots carrying this
plugin's own
bin/. So the documented feature is delivered on this Windows/Git-Bash machine,
then intermittently lost; when the line does not land, every enabled plugin's bin/ goes with it.
anthropics/claude-code#68066 supplies the
mechanism and root cause — note it is a macOS/zsh report, so it is the mechanism source, not the
Windows evidence, and its own log shows healthy and degraded sessions interleaving minutes apart.
The prior "never delivers it here" reading came from sampling only degraded sessions: #843 was filed
07-21, and every reproduction on it landed after the 07-18 healthy runs.

Correction 2 — the path form cannot match a bare-name allow rule (this one is categorical).
Before matching Bash rules, Claude Code strips only a fixed wrapper set: timeout, time, nice,
nohup, stdbuf, command, builtin, noglob, and bare xargs
(permissions, verified live this session). bash
is not among them.
So bash "…/bin/source-control-babysit-merge" … matches as a bash command
and can never satisfy a pre-approved Bash(source-control-babysit-merge:*) — not
"opportunistically", but for every call this skill issues. A per-call prompt on these invocations is
therefore expected behavior, not a misconfiguration, which is worth stating plainly since the
operator pre-approved those bare names specifically to avoid it. This is residual #1 on #843, now
with a verified mechanism.

Changes

  • skills/babysit-prs/reference/safety.md — "Guarded Mutation Wrappers" now carries both facts
    above, each with its source, replacing the flat "not on PATH" claim.
  • bin/source-control-babysit-merge, bin/source-control-babysit-resolve-thread — headers state
    the bare-name rationale as currently unrealized and point at safety.md rather than
    restating the analysis (pointer-not-copy). The upstream owner/repo#N reference deliberately
    lives only in markdown: the comment-hygiene CI policy bans tracker-refs in code comments —
    verified by running that action's own comment-hygiene-patterns.sh against both wrappers, with a
    positive control confirming the scanner does flag the pattern.
  • docs/PLUGIN-PHILOSOPHY.md — the bin/ stance row gains the caveat. This is the table a future
    plugin author reads before making a bare name load-bearing, so leaving it asserting
    unconditional PATH delivery would reproduce this exact defect. Row's Verified date left at
    2026-07-17: the caveat cites its own sources and is not a re-verification of that row's linked
    page.
  • Version 0.26.4 -> 0.26.5 + CHANGELOG entry.

No engine, CLI, flag, or guard behavior changes.

Test plan

Run against the branch worktree; all green:

  • Wrapper guards still fire (the edits touch only comments, but these are the authorization
    layer, so they were exercised rather than assumed):
    • source-control-babysit-merge … --allow-unpinned-head → refused, exit 2
    • source-control-babysit-merge without --allowed-owners → fail-closed JSON, exit 3
    • source-control-babysit-resolve-thread without --allowed-owners → fail-closed JSON, exit 3
  • shellcheck on both wrappers — clean.
  • markdownlint-cli2 on both changed markdown files — 0 errors.
  • typos --config _typos.toml on the changed files — clean.
  • editorconfig-checker on the diff — clean.
  • comment-hygiene-patterns.sh (fetched from the pinned ci-workflows action) on both wrappers —
    clean; positive control confirms it flags owner/repo#N.
  • PR body checked against the real pr-issue-linkage validator regexes (closing-keyword +
    ## Related extraction) before opening — this caught an accidental closing keyword in an earlier
    draft heading that would have auto-closed the tracking issue on merge.
  • Exec bits preserved (100755 on both wrappers); no CRLF introduced.
  • plugin.json parses; version bumped.

Fresh-docs check per CLAUDE.md: the plugins reference was re-fetched this session
(https://code.claude.com/docs/en/plugins-reference). The File locations row still reads
"Executables added to the Bash tool's PATH. Files here are invokable as bare commands in any Bash
tool call while the plugin is enabled", with no Windows/Git-Bash caveat — so upstream still
promises the behavior, which is why this PR corrects our justification rather than our guidance.

Related

Why #843 stays open

The dispatching lane's default is a closing keyword; this deviates deliberately, for operator
override. #843's own body designates it the tracker for the upstream dependency, and its two
residuals — the lost bare-name allow-rule match, and the wrappers' unrealized design intent —
cannot be closed downstream. Closing it on a docs correction would strand a live upstream
regression that now has an unexplained second signature (below).

Operator decisions parked here

  1. The upstream-report default from triage is withdrawn. Filing a "Windows/Git-Bash functional
    non-delivery" report would now be factually wrong, and #68066 already carries the mechanism.
    The live question is different: upstream declined #68066 as NOT_PLANNED — do we want to
    argue it?
    The corpus here (clean 07-18-works / 07-19-onward-fails break, 64% session failure
    rate over 112 snapshots) is materially stronger than what #68066 carried. This lane filed
    nothing upstream and is not authorized to.
  2. Unexplained residual, flagged rather than papered over. Five snapshots have a complete
    export PATH= line carrying zero plugin bins — three of them today. That is not truncation,
    so either #68066's mechanism does not fully account for this or there is a second failure mode.
    The prose says "consistent with #68066's signature", never "caused by".
  3. An independent fresh-context reviewer audited this diff (rationale withheld) and its two
    IMPORTANT findings were adopted, not argued: the "opportunistic" mischaracterization above, and
    an over-tidy "one degraded window" narrative in the CHANGELOG that the cited upstream issue does
    not support. Both are fixed in the second commit; precise session counts were dropped from the
    CHANGELOG because no future reader can reproduce them from this repo (the methodology lives on
    source-control:babysit-prs: plugin bin/ not on Bash-tool PATH — guarded wrapper bare names unresolvable (blocks narrow allow-rule match) #843).
  4. Two historical design docs left untouched
    docs/topics/babysit-prs-migration/design/phase-2-engine-modules.md:35 and PLAN.md:478 still
    describe bare names as the pinned invocation. They are completed-migration records that were
    already superseded by source-control:babysit-prs: 'source-control-babysit-resolve-thread' bare wrapper not on PATH (command not found), worker must fall back to raw GraphQL #484, not by this change; rewriting them would falsify what was decided
    at the time. Flagged, deliberately out of scope.

🤖 Generated with Claude Code

kyle-sexton and others added 3 commits July 25, 2026 16:30
`safety.md` asserted the bundled wrappers' bare names "are not on the
Bash tool's PATH", and the two bin/ wrapper headers presented their
bare-command allow-rule rationale as operative fact. Both were wrong, in
opposite directions.

A shell-snapshot corpus on the reporting machine shows plugin bin/
directories reaching the Bash tool's PATH in 35 of 112 sessions, 4 of
them carrying this plugin's own bin/. The feature is delivered, then
intermittently lost: a plugin's bin/ arrives only via the session shell
snapshot's final `export PATH=` line, and when that line does not land
every enabled plugin's bin/ goes with it. The "Windows/Git-Bash never
delivers it" reading was a sampling artifact — every reproduction on the
issue fell inside one degraded window.

Operational guidance is unchanged and was already correct: the
${CLAUDE_PLUGIN_ROOT}/bin/ path form stays canonical because it works in
both states. Only the justification changed, and it mattered — a reader
who tested on a healthy-snapshot session found the doc contradicting
their own shell, and the documented reason to keep the path form
evaporated exactly when it looked safe to drop.

The wrapper headers point at safety.md rather than restating the
analysis; the upstream reference lives in the markdown, which the
comment-hygiene tracker-ref policy does not scan.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…dy narrative

Independent review caught two overclaims in the first pass.

The wrapper headers called the bare-name allow-rule match "opportunistic",
implying it sometimes fires. It cannot fire at all for this skill's calls.
Claude Code strips only a fixed wrapper set before matching Bash rules
(timeout, time, nice, nohup, stdbuf, command, builtin, noglob, bare xargs
-- verified against the live permissions page); `bash` is not among them,
so `bash ".../bin/source-control-babysit-merge"` matches as a bash command
and never satisfies Bash(source-control-babysit-merge:*). The benefit is
not intermittent, it is unrealized -- which is exactly the residual the
tracking issue records, and it means a per-call prompt on these
invocations is expected rather than a misconfiguration. safety.md now
carries both halves (unreliable delivery, non-matching form) with their
sources; the headers point there instead of characterizing it.

The CHANGELOG claimed every reproduction "fell inside one degraded
window". The upstream issue's own log shows healthy and degraded sessions
interleaving minutes apart, and that issue is macOS/zsh only -- it
supplies the mechanism, not the Windows/Git-Bash evidence. The entry now
separates the two sources instead of merging them into one tidy story,
and drops precise session counts that no future reader can reproduce from
this repo.

Also caveats the bin/ row in PLUGIN-PHILOSOPHY.md, the table a future
plugin author reads before making a bare name load-bearing.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…per-bare-name-rationale

# Conflicts:
#	plugins/source-control/.claude-plugin/plugin.json
#	plugins/source-control/CHANGELOG.md

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 201ebcc032

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/source-control/CHANGELOG.md
@kyle-sexton

Copy link
Copy Markdown
Contributor Author

This was generated by AI during autonomous execution.

Conflict status: mechanical version-bump race, not a content conflict.

This branch was integrated with main once (per the dispatching lane's bound). It went
CONFLICTING again within minutes, and will keep doing so while the merge lane is draining
source-control PRs: every one of them bumps the same two files, so the conflict surface is
permanently contended.

The conflict is confined to exactly two files, and never to the substance of this PR:

  • plugins/source-control/.claude-plugin/plugin.json — the version string
  • plugins/source-control/CHANGELOG.md — the top version heading

The four files carrying this PR's actual change — both bin/ wrappers,
skills/babysit-prs/reference/safety.md, and docs/PLUGIN-PHILOSOPHY.md — have not conflicted
in any integration pass and were verified byte-identical to the reviewed state after each one.

Resolution recipe (mechanical, no judgement needed): take main's version of both files, then
re-apply this PR's CHANGELOG entry under the next patch version and set plugin.json to match.
This branch has already been through that twice — 0.26.50.26.60.26.8 — as main moved.

Verified green on the merged tree before the last push: shellcheck, markdownlint-cli2, typos,
editorconfig-checker, the comment-hygiene policy from the pinned ci-workflows action, and the
three wrapper guard paths (--allow-unpinned-head → exit 2; both wrappers fail-closed → exit 3).

Not merging is this lane's boundary, so the final integration belongs to whoever merges.

…per-bare-name-rationale

# Conflicts:
#	plugins/source-control/CHANGELOG.md
@claude

claude Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 52s —— View job


PR Review

  • Gather context and understand the request
  • Read changed files
  • Review for correctness, security, and CLAUDE.md alignment
  • Post final review

Summary

This is a documentation-only correction. No guard logic, operational behavior, or invocation forms change — the diff touches only comments, prose, and a version bump. The two factual corrections are sound and the evidence trail is solid.


Correctness

Correction 1 — PATH delivery is unreliable, not absent.
The empirical support (corpus of 112 shell snapshots, 35 with plugin bins, 72 without) is appropriate for a "unreliable, not absent" claim. The upstream issue reference (anthropics/claude-code#68066) is correctly characterized as the mechanism source, not the Windows evidence. The inference is well-bounded.

Correction 2 — path-form invocation cannot satisfy a bare-name allow rule.
This is categorical and the reasoning is right: bash is not in the stripped-wrapper set, so bash "…/bin/source-control-babysit-merge" … matches as a bash command, period. The claim is not hedged when it doesn't need to be.

Wrapper script headers.
Only comments changed; the argument-prefix guard, --allow-unpinned-head rejection, and babysit_python dispatch are byte-identical to before. The "pointer-not-copy" pattern — pointing to safety.md rather than duplicating the analysis in a 12-line header — is correct; single source of truth for the rationale.

safety.md restructure.
The old flat assertion ("bare names are not on the Bash tool's PATH") is replaced by two clearly separated, attributed bullets. The preserved conclusion (use ${CLAUDE_PLUGIN_ROOT}/bin/) is still correct — it's the only form that runs in both PATH states. The "Expect a per-call prompt on these invocations" is now stated plainly rather than leaving it to operator inference; that's good.

PLUGIN-PHILOSOPHY.md row.
The caveat is the right addition in the right place: the table is the first thing a new plugin author reads before committing to a design. Placing the warning there prevents the documented failure mode from being reproduced by a future plugin. The Verified date staying at 2026-07-17 is explained in the PR body — the caveat cites its own linked sources rather than re-verifying the row's linked page. Acceptable, and documented.

CHANGELOG.
Uses Keep a Changelog format. The [0.26.9] entry correctly names what changed (justification, not guidance) and explains why the distinction matters to a future reader. The #843 reference uses backtick formatting \#843`— consistent with the tracker-ref hygiene policy (which bans bareowner/repo#N` in code comments; markdown is not code).


Security

No security surface changes. The authorization layer (wrapper guards, --allow-unpinned-head rejection, fail-closed --allowed-owners requirement) is untouched. The documentation now more accurately describes why per-call prompts appear on these invocations — which is a security-adjacent clarification (operators expecting the allow rule to suppress prompts were wrong, and now they won't be).


CLAUDE.md Alignment

  • Fresh-docs mandate: safety.md's prose body is a prose edit (not a contract surface change), so the mandate is out of scope per CLAUDE.md's discriminator. The author fetched the plugins reference and permissions page anyway and cites both — no issue.
  • Repo-agnostic / no hardcoded paths: No new hardcoded paths introduced.
  • Versioned: Patch bump (0.26.80.26.9) is appropriate for a documentation-only correction.

Observations (non-blocking)

Codex review finding is spurious. The Codex comment references rest_hydrate_reviews, pr_queue_snapshot.py, babysit_feedback.py:112–115, and commit 521d78e — none of which appear in this PR's diff. That finding belongs to a different PR or a confused context window; it has no bearing on this change and should be dismissed.

safety.md table at line 164–167 uses bare names (source-control-babysit-merge, source-control-babysit-resolve-thread) as concept labels in a | Script | … comparison table, immediately above the new caveat about not invoking bare. These are component labels in an explanatory table, not invocation instructions, so there's no actual inconsistency — but a first-time reader who just absorbed "bare names don't reliably resolve" could briefly wonder. Worth a glance but not a change.


Verdict

Clean documentation correction. Claims are accurate, evidence is attributed, guidance is unchanged, guards are untouched. No blocking issues.

@claude

claude Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 25s —— View job


Security Review — PR #1399

  • Gather context and understand the request
  • Fetch PR diff
  • Read changed shell scripts (bin/source-control-babysit-merge, bin/source-control-babysit-resolve-thread)
  • Read changed documentation files
  • Analyze for security issues
  • Post findings

Findings

No security issues found.

Scope of changes

All six changed files are either documentation/prose updates or a version bump. The security-relevant executable code in both bin/ wrappers is entirely unchanged by this PR — the diff in those files is limited to comment lines in the script headers:

  • bin/source-control-babysit-merge: lines 3–12 are comment text only. The set -euo pipefail, PYTHONUTF8, here/scripts construction, --allow-unpinned-head prefix-guard loop, and babysit_python invocation are all byte-identical to origin/main.
  • bin/source-control-babysit-resolve-thread: same — lines 3–11 are comment text only; all executable lines are unchanged.

Analysis of each changed file

File Nature of change Security surface
bin/source-control-babysit-merge Header comments updated None — no executable lines changed
bin/source-control-babysit-resolve-thread Header comments updated None — no executable lines changed
skills/babysit-prs/reference/safety.md Prose correction to "Guarded Mutation Wrappers" None — documentation only
docs/PLUGIN-PHILOSOPHY.md One table row expanded with a caveat None — documentation only
plugins/source-control/CHANGELOG.md New entry added None
plugins/source-control/.claude-plugin/plugin.json Version 0.26.80.26.9 None

Security model impact

The safety.md change clarifies two facts:

  1. Bare-name PATH delivery is unreliable — so the prior "not on PATH" claim was sometimes false. The correction is accurate and does not weaken the security model; the actual guards are in the Python engine, not in PATH resolution.
  2. bash "…/bin/wrapper" … can never match a Bash(wrapper-name:*) allow rule, because bash is not in Claude Code's stripped-prefix set. This is a security-positive correction: callers who believed the allow rule was covering these invocations were wrong, and every such invocation actually triggers a human-approval prompt. The update removes a documentation-induced false sense of unattended coverage.

No changes to GitHub Actions workflows, permissions, secrets handling, MCP server configuration, or any other supply-chain surface.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a29376413f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/source-control/skills/babysit-prs/reference/safety.md Outdated
# Conflicts:
#	plugins/source-control/.claude-plugin/plugin.json
#	plugins/source-control/CHANGELOG.md
In auto mode an uncovered Bash call gets no permission prompt: the
classifier decides, and can deny silently. Claiming a per-call prompt
leaves an operator waiting for one that never arrives.
@claude

claude Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error after 2s —— View job


I'll analyze this and get back to you.

@claude

claude Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error after 2s —— View job


I'll analyze this and get back to you.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 95f513b433

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/source-control/skills/babysit-prs/reference/safety.md Outdated
Plan mode prompts for shell commands only when the auto-mode classifier is
not reviewing them; useAutoModeDuringPlan is on by default.
@claude

claude Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error after 2s —— View job


I'll analyze this and get back to you.

@claude

claude Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error after 2s —— View job


I'll analyze this and get back to you.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5864ca78c6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/source-control/skills/babysit-prs/reference/safety.md
The marketplace-wide convention still stated categorical non-delivery and a
classifier-on-every-invocation rule, so a plugin author following the declared
source of truth got the reading this PR corrects downstream.
@claude

claude Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error after 2s —— View job


I'll analyze this and get back to you.

@claude

claude Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error after 2s —— View job


I'll analyze this and get back to you.

# Conflicts:
#	plugins/source-control/CHANGELOG.md
@claude

claude Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 25s —— View job


Security Review — PR #1399

  • Gather PR diff
  • Read changed shell scripts
  • Read changed documentation files
  • Analyze for security issues
  • Post findings

No security issues found.

Scope of changes

All eight changed files are documentation/prose corrections, a version bump, or comment-only edits to shell scripts. Security-relevant executable code in both bin/ wrappers is byte-identical to origin/main.

Shell script analysis

bin/source-control-babysit-merge and bin/source-control-babysit-resolve-thread: diff is confined entirely to comment lines in each file's header block. Every executable statement — set -euo pipefail, the PYTHONUTF8=1 export, the here/scripts self-locating construction, the --allow-unpinned-head prefix-guard loop, the . "$scripts/babysit-python.sh" source, and the babysit_python dispatch — is unchanged.

The existing argument-guard loop uses [[ "$arg" == --a* && "--allow-unpinned-head" == "$arg"* ]] with properly quoted variables throughout — no injection surface introduced or modified.

Documentation analysis

File Nature of change Security surface
bin/source-control-babysit-merge Header comments only None — no executable lines changed
bin/source-control-babysit-resolve-thread Header comments only None — no executable lines changed
skills/babysit-prs/reference/safety.md Prose correction (PATH delivery, permission-mode behavior) None
docs/PLUGIN-PHILOSOPHY.md One table row gains a caveat None
docs/conventions/permission-rule-hygiene/README.md Corrects "absent" → "unreliable", updates classifier/prompt description None
plugins/source-control/CHANGELOG.md New release entry None
plugins/source-control/.claude-plugin/plugin.json Version 0.31.50.31.6 None
docs/conventions/permission-rule-hygiene/CHANGELOG.md Convention version entry None

Security model impact

The safety.md correction is security-positive: it accurately states that the pre-approved Bash(source-control-babysit-merge:*) allow rule does not cover the bash "…/bin/…" invocation form, and that in auto mode an uncovered call routes to the classifier silently rather than prompting. Operators who believed the allow rule suppressed prompts were operating under a false assumption — the correction removes it.

No GitHub Actions workflow changes. No permission-widening. No action-pin or dependency-pin changes. No secret or credential handling anywhere in the diff.

@claude

claude Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 38s —— View job


PR Review — #1399

  • Gather context and understand the request
  • Read changed files
  • Review for correctness, security, and CLAUDE.md alignment
  • Post final review

Summary

Documentation-only correction across 8 files — no guard logic, no invocation forms, no operational behavior changes. Both factual corrections are accurate and well-attributed. No blocking issues.


Correctness

Correction 1 — PATH delivery is unreliable, not absent.

All three prose destinations (safety.md, permission-rule-hygiene/README.md, PLUGIN-PHILOSOPHY.md) now consistently state the per-session mechanism. The wording is careful: "delivery rides the per-session shell snapshot's final export PATH= line; when that line does not land, every enabled plugin's bin/ goes with it." The upstream issue (anthropics/claude-code#68066) is correctly scoped — cited as mechanism source, not Windows evidence. The correction that the prior reading came from "sampling only degraded sessions" is preserved in all three files with consistent attribution.

Correction 2 — path-form invocation cannot satisfy a bare-name allow rule.

The categorical claim is correct: bash is not in the process-wrapper stripping set, so bash "…/bin/source-control-babysit-merge" … matches as a bash command unconditionally. The follow-on mode-dependent split is accurate:

  • Prompting modes (Manual, accept-edits, plan-without-classifier) issue a per-call prompt ✓
  • Auto mode routes uncovered calls to the classifier, which may approve or deny silently ✓
  • autoMode.classifyAllShell: true suspends narrow Bash rules entirely ✓
  • Only an explicit permissions.ask rule forces a prompt in auto mode ✓

The plan-mode qualifier at safety.md:264–269 is accurate against the official page — "when auto mode is available and useAutoModeDuringPlan is on, which it is by default, the classifier reviews them 'instead of prompting you'" — and the kyle-sexton thread response at 2026-07-26T01:32 correctly cited the source. The phrasing "no-classifier branch" is slightly terse but the following sentence explains it fully, so no ambiguity survives.

Wrapper script headers — pointer-not-copy.

Executable logic in both wrappers (from set -euo pipefail onward) is byte-identical to origin/main:

The "pointer-not-copy" pattern (pointing headers at safety.md rather than duplicating the analysis) is the right call — the analysis is nuanced enough that two copies would diverge.

CHANGELOG entries.

Both the source-control CHANGELOG (0.31.6) and permission-rule-hygiene/CHANGELOG.md (1.2) describe only what changed in this PR, in Keep-a-Changelog / prose-narrative format respectively. The #843 reference in source-control's CHANGELOG is backtick-formatted (\#843`) — consistent with the comment-hygienepolicy that bans bareowner/repo#N` in code comments but does not apply to markdown.

Convention drift — closed.

The Codex finding at 2026-07-26T01:35 about permission-rule-hygiene/README.md diverging from safety.md was correctly identified and fixed in 3b8c3b8. The convention and the downstream skill now carry consistent readings of both corrections.


Security

The authorization layer is untouched:

  • --allow-unpinned-head prefix-guard (exit 2) — unchanged.
  • Fail-closed --allowed-owners requirement (exit 3) on both wrappers — unchanged.
  • babysit_python dispatch — unchanged.

The documentation now more accurately describes why per-call prompts appear in prompting modes and why they don't appear in auto mode — a security-adjacent clarification, not a relaxation. Operators who expected the bare-name allow rule to suppress auto-mode prompts were wrong; the corrected docs prevent that false assumption from being acted on.


CLAUDE.md Alignment

  • Fresh-docs mandate: safety.md's prose body is prose, not a contract surface change, so the mandate is out of scope per the discriminator. The author fetched the permissions and permission-modes pages and cites both.
  • Repo-agnostic: No new hardcoded paths.
  • No PII / secrets: Clean.
  • Versioned: Patch bump (0.31.50.31.6) appropriate for docs-only correction.

Verdict

Clean documentation correction. Both factual claims are accurate and attributed, guidance is unchanged, all guards are untouched, and the Codex findings about classifier wording and convention drift were addressed in the subsequent commits (95f513b, 3b8c3b8). No blocking issues.

@kyle-sexton
kyle-sexton merged commit 6c1f9e8 into main Jul 26, 2026
27 checks passed
@kyle-sexton
kyle-sexton deleted the docs/843-babysit-wrapper-bare-name-rationale branch July 26, 2026 01:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant