fix(source-control): actionable owner/repo diagnostic on readiness-gate fetch failure#839
Conversation
…ailure babysit-readiness-gate.sh shells out to fetch-all-pr-comments.sh, which auto-derives owner/repo from the current directory via `gh repo view`. From a cwd that is not a checkout of the target repo (e.g. a targeted-recheck pass) that derivation returns empty and the fetch exits non-zero, which the gate surfaced only as a generic "fetch-all-pr-comments.sh failed" line plus exit 4 — the same undocumented exit code as a missing jq, leaving a worker to rediscover the FETCH_COMMENTS_OWNER/FETCH_COMMENTS_REPO override by trial and error. The gate's failure message now names the resolved cwd and the env-var override, fetch-all-pr-comments.sh's own "cannot resolve owner/repo" message names the cwd and override, and the gate's --help and exit-code docs cover it. The babysit-prs worker contract (SKILL.md, reference/loop.md) documents the override as a prerequisite for out-of-checkout passes. Regression tests cover the gate's exit-4 diagnostic, the fetch script's exit-2 message, and the env-var override path. No change to the readiness verdict or exit codes; diagnostics and docs only. Closes #475 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
|
Claude finished @kyle-sexton's task in 2m 51s —— View job Code Review
Verdict: Approve. This is a well-scoped, diagnostics-only fix with solid test coverage and no behavior change to the readiness verdict or exit codes. A few nits below, none blocking.
|
|
Claude finished @kyle-sexton's task in 1m 42s —— View job Security Review
No security issues found. This PR is diagnostics and documentation only — it adds two
Test stubs (both test files): each stub Other changes (plugin.json version bump, SKILL.md, reference/loop.md): documentation only, no executable surface. |
|
Post-green review: Approve/clean per both automated passes. No blocking findings — one non-blocking readability nit (SKILL.md run-on sentence in Step 0.1) left alone per the reviewer's own "not blocking" call. No inline review threads. Handing off to babysit for ongoing watch. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Claude finished @kyle-sexton's task in 2m 52s —— View job Code Review
Verdict: Approve. Diagnostics-only fix with correct shell idioms, thorough test coverage, and no behavior change to the readiness verdict or exit codes. A few observations below — none blocking.
|
|
Claude finished @kyle-sexton's task in 1m 44s —— View job Security Review
No security issues found. This PR adds two diagnostic
# babysit-readiness-gate.sh L152 — safe pattern
printf '...current directory (%s) via gh repo view...\n' "$PWD" >&2
# fetch-all-pr-comments.sh L101 — safe pattern
printf '...current directory (%s)...\n' "$PWD" >&2
Test stubs (
Documentation changes (
|
…de-ops, work-items Re-checked open PRs touching these plugins' plugin.json right before the version-bump collision protocol requires it: source-control carries #839 (0.15.8) and #840 (0.15.9), claude-ops carries #844 (0.17.2), work-items carries #826 (0.18.2) — all still open. Bumps this PR's claims one past each plugin's current highest open-PR claim (source-control 0.15.10, claude-ops 0.17.3, work-items 0.18.3) so this PR does not collide at merge time regardless of which sibling lands first. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
|
Tower merge-sit: merge-drive lane dark ~5.5h (rate-limit hold since ~10:36Z); tower sitting merges per stall protocol. Gate-verified live: CLEAN, 0 unresolved threads. This was generated by AI (control tower, session 6). |
Resolves conflicts on source-control and work-items plugin.json/CHANGELOG.md: main advanced source-control to 0.15.8 (#839, merged) and work-items to 0.18.2 (#826, merged) since this branch was last rebased. Also re-checked live open PRs at merge time and found work-items now carries a new open claim, #857 at 0.19.0 (Jira adapter) — re-bumped this branch's work-items claim from 0.18.3 to 0.19.1 to stay ahead of it. source-control's 0.15.10 and claude-ops's 0.17.3 remain valid (still one past #840/#860's 0.15.9 and #844/#860's 0.17.2 open claims, respectively). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
Integrate #839 (source-control 0.15.8, babysit-readiness-gate diagnostic). Resolved plugin.json to 0.15.9 (one past main) and stacked the 0.15.9 CHANGELOG entry above the merged-in 0.15.8 entry. SKILL.md auto-merged (distinct regions); babysit-prs skill-quality gate green at 499/500 lines.
…gs for babysit snapshot (#882) ## Summary Fixes #511. The babysit PR-queue snapshot derived `self_logins` — the posting identities whose comments self-classification suppresses — from the `--author` discovery filter, conflating two distinct concerns (*which authors' PRs to discover* vs *whose comments to suppress*). That broke in both directions: - **Under-inclusion:** configured extra self identities (`babysit_self_logins`) rode on `--author`, so when autopilot widening drops `--author` entirely, a bot poster's own comments re-fired `new_human_blocking_feedback` every cycle. - **Over-inclusion:** a `--queue --author alice` run authenticated as a different login made `alice` a self-login, suppressing Alice's genuine feedback from the worker-dispatch arm. ## The fix `pr_queue_snapshot.py` now resolves self-identity from dedicated **`--self`** (full override — exactly the given logins, `@me` not added) / **`--extra-self`** (added on top of the authenticated `@me`) flags, mirroring the existing `babysit-readiness-gate.sh` semantics, resolved **independently of `--author`** and before the `--pr`/`--queue` scope split (so single-PR scope is covered too). This supersedes the `@me`-only union added in #494, and the author-derived self fallback in `build_config` is removed so **no discovery author can leak into the self set** in any path. The skill's step-4 invocation and the `babysit_self_logins` userConfig table row now route the configured extras through `--extra-self` instead of overloading `--author @me,<self-logins>`; because self-identity no longer rides on `--author`, it survives autopilot widening. ## Tests (red-green, with transparency on the rewritten spec) - Added `SelfIdentityDecouplingTests` asserting both directions at the `build_snapshot` level (extra-self survives a dropped `--author`; a discovery author is not treated as self). **Verified these fail against the pre-fix author-union code and pass after the fix.** - The `resolve_self_logins` unit tests pinned the *old* author-union contract; they are rewritten to the new flag-based contract, and `test_discovery_authors_are_unioned_with_the_self_login` (which asserted the over-inclusion behavior now fixed) is deleted. `test_raw_author_fallback_drops_me_when_unresolved` becomes `test_missing_resolved_self_logins_yields_empty_not_author`. These are a ratified spec change (the issue's converged decision), not test weakening. ## Verification (local gates) - `python -m unittest discover -s tests -p 'test_*.py'` (the CI entry via `engine.test.sh`) — **341 tests OK**. - `check-changelog-parity.sh --check-bump origin/main` — PASS (0.16.0 entry present). - `check-skill-portability.sh origin/main` — PASS. - `check-changed-skills.sh` — SKILL.md 499/500 lines (under the hard cap); the one reported "engine.test.sh failed" is the pre-existing repo-wide ruff E402 quirk that does **not** reproduce in CI (main is green; merged babysit PRs #860/#839 pass skill-quality-gate), and this change adds zero new ruff errors (32 before == 32 after). - `markdownlint-cli2` on the changed docs — clean; `plugin.json` valid. - Independent fresh-context code review (producer ≠ reviewer). ## Version Per-plugin bump **0.15.9 → 0.16.0** (minor — new `--self`/`--extra-self` CLI surface alongside the behavior fix). `marketplace.json` pins no version for `source-control`, so only `plugin.json` changes. Closes #511 ## Related - #497 — single-`--pr` scope leaves `self_logins` empty (sibling facet of the same seam, triaged blocked-by #511). This change resolves self-identity for `--pr` scope too, but its acceptance is left for the #497 lane to verify — not closed here. - #494 — added the `@me`-only `--queue` union this supersedes. - #473 — engine-level self-filter (related self-dispatch symptom). - #881 — follow-up (filed): the snapshot raises on an unresolvable `@me` where the readiness gate degrades to the `--extra-self` set; tracks whether fail-loud is the intended contract. Surfaced in this PR's review. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…ot bare name (#484) (#840) ## Summary The `source-control:babysit-prs` worker/autopilot contract told workers to run the merge gate and resolve review threads by invoking the guarded mutation wrappers as **bare command names** (`source-control-babysit-merge …`, `source-control-babysit-resolve-thread …`). Those bare names are **not on the Bash tool's `PATH`**, so every such invocation failed `command not found` (exit 127) — reproduced first-hand in this environment. Workers fell back to raw `gh api graphql resolveReviewThread` calls, losing the wrapper's `--allowed-owners` guardrail and the JSON `action` receipt the contract tells them to parse, and re-implementing the safety logic ad hoc per worker. ## Fix Invoke each wrapper **by its bundled path** — `bash "${CLAUDE_PLUGIN_ROOT}/bin/<wrapper>" …` — exactly the form the read-only sibling scripts under `${CLAUDE_PLUGIN_ROOT}/scripts/` (`fetch-all-pr-comments.sh`, `babysit-readiness-gate.sh`) already use. Updated surfaces: - **`skills/babysit-prs/reference/safety.md`** — the single home for the exact commands. The prior *Guarded Mutation Wrappers* posture said the wrappers are invoked "only by their pinned bare wrapper names … never through an interpreter-prefixed path," which the bare-name PATH gap made unfollowable. It now prescribes the `bin/`-path form and narrows the prohibition to what actually protects the guards: **raw-Python invocation** (`python … babysit_merge.py`, which bypasses the wrapper's own guards such as the merge wrapper's `--allow-unpinned-head` rejection) and **piping a wrapper into an interpreter**. All fenced command blocks and the Pinned-Command Degradation operator handoffs use the path form. - **`skills/babysit-prs/SKILL.md`** — corrected the "invoked ONLY by their bare wrapper names" statement; points at `safety.md` as the single home and states that every `source-control-babysit-<x> …` command in this skill and its references is launched by the `bin/`-path form. - **`skills/babysit-prs/reference/orchestration.md`** — the worker prompt template (the literal contract handed to a dispatched worker) and the file's two other runnable references now use the path form, so the file is self-consistent for a standalone worker. - **`reference/review-discipline.md`** — a one-line pointer in D7.5 noting the babysit tiers resolve through the guarded wrapper (which adds `--allowed-owners`, bot-vs-human classification, and a JSON receipt). D7.5's general raw-GraphQL policy is **unchanged** — it stays the mechanism for `/pull-request`, which is not a babysit-tier consumer; repointing it at the babysit-tier wrapper would wrongly couple `/pull-request` to a babysit tool. Why the path form and not shipping the bare name onto PATH: the wrappers **are** designed to be bare commands on PATH, and Claude Code's plugin reference documents a plugin's `bin/` as added to the Bash tool's PATH "while the plugin is enabled." Empirically that is not happening in this environment — an upstream/harness matter this repo cannot fix. The `${CLAUDE_PLUGIN_ROOT}/bin/`-path form is the deterministic, repo-level invocation that works today and matches the established sibling-script convention. ## Verification - **Reproduced the gap** in this worktree's Bash tool (plugin enabled, Windows/Git-Bash): `which source-control-babysit-resolve-thread` → not found (exit 1); `$CLAUDE_PLUGIN_ROOT` is empty in the shell; no plugin `bin/` on `PATH`. - **Confirmed the path form keeps every guard** (so the posture refinement is safe): invoking a wrapper by path still runs the wrapper — - `bash "…/bin/source-control-babysit-merge" owner/repo#1 --allow-unpinned-head` → still rejected, **exit 2** (`--allow-unpinned-head is not permitted through the wrapper`). - `bash "…/bin/source-control-babysit-resolve-thread" owner/repo#1 --resolve` (no `--allowed-owners`) → still **fail-closed, exit 3**, with the JSON refusal receipt. - **Lint**: `markdownlint-cli2` clean (0 errors) on all four edited docs. - Doc-only change (`.md` + `plugin.json` version); no scripts or executables modified. Closes #484 ## Related - **#484** — this issue. - **#475 / #839** — same bare-wrapper-invocation class (the `babysit-readiness-gate.sh` owner/repo-derivation diagnostic), landing concurrently in `plugins/source-control/`. Per the coordinate-and-serialize protocol this PR is opened **DRAFT + `do-not-merge`**, with its version set **one increment past #839** (`0.15.9` vs #839's `0.15.8`); once #839 lands, drop `do-not-merge` and confirm the version still leads `main`. - **#512** — a sibling open `source-control:babysit-prs` issue (resolve-thread `humanThreadsActed` mislabel), noted as concurrent source-control work. - **Follow-up residual — allow-rule match (not fixed here):** the `${CLAUDE_PLUGIN_ROOT}/bin/…` form does not match a pre-approved bare-name `Bash(source-control-babysit-resolve-thread:*)` allow rule, so an operator's narrow allowlist entries no longer auto-approve these calls (per-call prompts return). The wrappers' own header comments and this repo's `bin/` design still assume the bare-name form; restoring it is deferred to the root-cause fix below. - **Follow-up residual — root cause, tracked as #843:** Claude Code documents a plugin's `bin/` as on the Bash tool's PATH while enabled, but it is empirically absent in this environment. Only closing that upstream/harness gap makes the bare names — and the allow-rule match above — work as intended. (Not fixed here.) --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Summary
babysit-readiness-gate.sh <N>(safe-tier babysit) could exit4with only ageneric
fetch-all-pr-comments.sh failed for PR <N>line — the same exit codeas a missing
jq— leaving a worker following the documented invocation torediscover, by trial and error, that setting
FETCH_COMMENTS_OWNER/FETCH_COMMENTS_REPOfixes it. This makes that failure mode self-explanatoryand documents the override in the worker contract.
Fix
Root cause: the gate shells out to
fetch-all-pr-comments.sh, whichauto-derives owner/repo from the current directory via
gh repo view. Theauto-derive already works from a checkout of the target repo; it returns empty
only when the cwd is not such a checkout (e.g. a targeted-recheck pass that
never runs
gh pr checkout). That is inherent to cwd-based resolution — theFETCH_COMMENTS_OWNER/FETCH_COMMENTS_REPOoverride is the cwd-independentescape hatch, and it already worked; it was just undocumented and the failure
was undifferentiated. So this change is diagnostics + docs, with no behavior
change to the readiness verdict or exit codes:
and names the
FETCH_COMMENTS_OWNER/FETCH_COMMENTS_REPOoverride (and the"run from the target repo checkout" fix), in addition to
fetch-all-pr-comments.sh'sown stderr. Its
--helpgains a "Repo resolution" section and the exit-4doc now covers the fetch-failure case, not just missing
jq.fetch-all-pr-comments.sh'scannot resolve owner/repomessage now names the cwd
gh repo viewwas resolved from and the override.SKILL.md(Step 0.1) andreference/loop.md(§5.0.3fetch step, §5.1.3 gate step E) document the override as a prerequisite for
passes whose cwd is not the target-repo checkout.
Considered and rejected: a new
--repo owner/repoflag. The env-var overridealready provides cwd-independent resolution, so a flag would duplicate a working
mechanism for no gain; keeping the surface small.
Verification
4from a non-repo cwd, then confirmed the newdiagnostic names the cwd and both env vars, still exiting
4.babysit-readiness-gate.test.sh— added a gate-level case: with agh repo view→empty stub onPATHand no--comments-json, the gate exits4and its stderr namesFETCH_COMMENTS_OWNER; plus a--help-documents-the-overrideassertion. 63/63 pass.
fetch-all-pr-comments.test.sh— added: unresolvable owner/repo exits2with the actionable message, and the env-var override resolves owner/repo even
when
gh repo viewis empty. 17/17 pass.shellcheckclean on all four scripts;skill-quality check babysit-prsPASS (
SKILL.md499/500, markdownlint clean).Related
gate counting bug; context only, not fixed here).
babysit_merge.pyread-only invocation blocked by the host auto-modepermission classifier (sibling
babysit-prsissue; context only, not fixed here).Closes #475
🤖 Generated with Claude Code
https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o