v0.6.0: the GitHub rail works out of the box on a fresh install — status probe, setup card, token secret, classified errors, no-raise tools - #27
Conversation
…y errors - resolve_gh(): shutil.which first, then /opt/homebrew/bin, /usr/local/bin, ~/.local/bin, /usr/bin (a desktop build launched without a shell PATH still finds a brew/apt gh); cached per process, re-resolved on a stale path. - set_token_getter(): the plugin's github.token secret is injected into every gh run as GH_TOKEN/GITHUB_TOKEN, winning over an ambient env token; read live so a token pasted in Settings works without a restart. - check_gh_error() now CLASSIFIES: gh exit 4 / 'gh auth login' stderr → not authenticated (with the fix); GraphQL could-not-resolve / HTTP 404 → repo not found; 403/429 + rate limit → rate limit hit (retry-after when gh says); binary missing → not installed (looked in …). Everything else keeps the 'Error (gh exit N): …' shape existing callers match on. - parse_json()/dicts(): the one place a gh JSON body is type-checked, so a dict-for-a-file / bare scalar can never AttributeError through a tool. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d default_repo error (#23) - resolve_repo() accepts a zero-arg getter for the default (evaluated only when no explicit repo was passed) so the tools / /issue read the LIVE config instead of a register-time snapshot — an onboard_project or a Settings edit mid-session is seen by the next call. - default_repo_error(): a malformed github.default_repo (not owner/name) is a NAMED error — in /issue, the routes and the views — never fed to gh. Closes #23. - projects.py: last-resort repo source — parse the origin remote of every registry project path (those without a github: binding first) and of project_board.repo (skipping the board's '.' sentinel) into owner/name; HTTPS/SSH/scp shapes, github.com-anchored, cached per path for a minute. Picker order: explicit → registry bindings → remotes, deduped. - The GITHUB_DEFAULT_REPO / GH_REPO env fallback stays (a test depends on it) but is logged at INFO whenever it decides the repo — never silent. - gh_issue.py docstring: '/issue is the user-only chat path; the github_create_issue agent tool exists behind github.write' (the old 'deliberately NOT an agent tool' was false since write_tools shipped). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ere, github_status, docstring truth - github_repo_contents: the contents API returns a dict for a FILE; iterating it raised AttributeError through the tool layer and killed the agent's turn. Now: "Error: '<path>' is a file, not a directory — use github_read_file". - Every gh JSON body goes through parse_json()/dicts() (get_pr/get_issue/ list_issues/ci_runs/review guards) — a list, scalar, null row or garbage is an Error string, not an exception. The sweep found get_pr and list_issues raising on the wrong JSON type too. - New read tool github_status (always on, no write gate): one paragraph — gh installed? authenticated? as whom? which default repo? — so the model self-diagnoses instead of guessing from stderr. - tests/test_no_raise_sweep.py: enumerates EVERY tool register() produces (read+write+review, 23) and invokes each against a run_gh stub returning dict / list / list-of-nulls / empty / garbage / number / string / error / auth / not-found / missing-binary / timeout, asserting a str comes back. Plus a real missing-binary pass through run_gh itself. The arg table must match the registered set, so a new tool can't dodge the sweep. - Docstring truth: github_read_file's description (what the model sees) no longer carries 'TODO(team): implement via …'; the 'STUBBED' module/section comments are gone; factories accept a getter for the default repo. - check_gh_error(..., repo=) everywhere so a 404 names the repo. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…robe, live config wiring
- status.py: compute_status() probes gh --version + gh auth status --json
hosts (text fallback for an older gh) → {gh_path, gh_version,
authenticated, login, host, token_source, error, default_repo, repos};
the ACTIVE account decides; bounded timeout; never raises.
summarize_status() is the github_status tool's paragraph; gaps_for() /
report_gaps() push 'gh' / 'auth' messages to the host's report_setup_gap
seam (guarded — no seam, no-op) and clear them when healthy;
probe_in_background() runs that off the register() hot path in a daemon
thread, only when the seam exists.
- GET /api/plugins/github/status (gated data router) — plus
default_repo_error. resolve_config() is the ONE picker/default computation
shared by /config, /status and POST /issue: a malformed default_repo is
named, kept out of the picker, and the default falls through (#23).
- view.py: a shared setup card (_SETUP_CSS/_SETUP_JS spliced into BOTH
pages) rendered from /status inside the kit boot only (post-handshake):
gh missing → install + sign-in copy; not signed in → gh auth login or
paste a token in Settings ▸ GitHub (github.token); malformed default repo
→ the named error; a Re-check button. The existing 'No repositories
configured' state stays (and the new-issue form now says it too).
- __init__.py: everything closes over LIVE getters (registry.live_config,
else the snapshot) — the tools' default repo, the picker, /issue, the data
router, and the github.token secret via set_token_getter; the setup probe
starts when the host has the seam.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… the code, release recipe - protoagent.plugin.yaml: version 0.6.0; secrets: [token] + the Settings field 'GitHub token (PAT) — optional; gh auth login also works'; the auth precedence and repo-resolution comments match gh_cli.py / projects.py. - pyproject.toml: 0.6.0 (test_version.py lockstep). - ruff==0.15.10 pinned in requirements-dev.txt and installed from there in ci.yml (the protoAgent core / projectBoard pin) — a floating ruff fails format --check on correctly-formatted code. - README: the real tool inventory — 11 read / 8 write / 3 review, all live (was 8/3 with five 'stubbed'); auth precedence; repo sources; the setup card. - PROTO.md: §3 file map (status.py, projects.py, review_tools.py), §4 '/issue is the user-only chat path; github_create_issue exists behind github.write' + the live-config rule, §5 inventory + error classification + first-run status + the no-raise sweep, §6 never-raise rule, NEW §7 release recipe (tag -a / push / gh release create; no CHANGELOG — the PR body is it). - CLAUDE.md / AGENTS.md pointers no longer mention 'the stubs'. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…be, gh-native token precedence, no-raise spawn, off-loop picker Adversarial review of #27, all items: BLOCKERS 1. resolve_gh() caches only a HIT; a miss is re-resolved every call, and compute_status() drops the cache first — so Re-check (and the next tool) sees gh installed after boot, and sees it removed. gh_available() now uses the same resolver (it disagreed with the tools before). Tests cover absent→installed and installed→removed. 2. compute_and_report(): /status AND the github_status tool now report to report_setup_gap (edge-triggered — failing key gets its message, passing key gets None) so the host banner clears on the very request that sees the recovery. build_data_router(cfg, registry=) and get_read_tools(..., registry=) carry the registry; register() passes it. MINORS 3. Token precedence matches gh: only a non-empty config token is injected (GH_TOKEN + GITHUB_TOKEN); otherwise the env passes through untouched so gh's own GH_TOKEN > GITHUB_TOKEN > keyring applies. resolve_token()/ token_source() check GH_TOKEN first. Manifest/README/PROTO wording. 4. run_gh(): trailing → (126, '', 'could not run gh at …') (ENOEXEC/EMFILE), contextlib.suppress(ProcessLookupError) around kill(). Sweep extended: every tool × {ENOEXEC, EMFILE, kill-after-exit}. 5. effective_default_repo() takes the picker as a GETTER and short-circuits when default_repo is set (no git fork on the configured path); the data routes and github_status resolve via asyncio.to_thread; the probe thread primes the remote cache; TTL 60s → 10min. Tests assert the picker is never computed with a default set, and that routes resolve off-loop. 6. auth_hint(token_source) — a rejected env/config token says 'fix/unset it' / 'replace it in Settings ▸ GitHub', not 'run gh auth login'. Used by the classified error, the summary, the gaps, and the views' card. The gh 401 body is trimmed to its status line. 7. Tool counts: 12 read / 8 write / 3 review = 23 everywhere. NITS - github_path_exists classifies FIRST (error_kind): auth/rate-limit/missing binary are UNVERIFIED errors, never a MISSING verdict; the 404 wording names the inaccessible-repo case. - README/PROTO state #23's named error is on the plugin's GET /config + the card (on read), not on Settings save. - gaps_for caps the why fragment at 80 chars so the host's ~300-char banner keeps the hint. - conftest: an autouse fixture clears GH_TOKEN/GITHUB_TOKEN, the token getter and the gh cache per test, so a dev box's ambient token can't skew hint assertions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…be, gh-native token precedence, no-raise spawn, off-loop picker Adversarial review of #27, all items: BLOCKERS 1. resolve_gh() caches only a HIT; a miss is re-resolved every call, and compute_status() drops the cache first — so Re-check (and the next tool) sees gh installed after boot, and sees it removed. gh_available() now uses the same resolver (it disagreed with the tools before). Tests cover absent→installed and installed→removed. 2. compute_and_report(): /status AND the github_status tool now report to report_setup_gap (edge-triggered — failing key gets its message, passing key gets None) so the host banner clears on the very request that sees the recovery. build_data_router(cfg, registry=) and get_read_tools(..., registry=) carry the registry; register() passes it. MINORS 3. Token precedence matches gh: only a non-empty config token is injected (GH_TOKEN + GITHUB_TOKEN); otherwise the env passes through untouched so gh's own GH_TOKEN > GITHUB_TOKEN > keyring applies. resolve_token()/ token_source() check GH_TOKEN first. Manifest/README/PROTO wording. 4. run_gh(): trailing 'except OSError' -> (126, '', 'could not run gh at ...') (ENOEXEC/EMFILE), contextlib.suppress(ProcessLookupError) around kill(). Sweep extended: every tool x {ENOEXEC, EMFILE, kill-after-exit}. 5. effective_default_repo() takes the picker as a GETTER and short-circuits when default_repo is set (no git fork on the configured path); the data routes and github_status resolve via asyncio.to_thread; the probe thread primes the remote cache; TTL 60s -> 10min. Tests assert the picker is never computed with a default set, and that routes resolve off-loop. 6. auth_hint(token_source) — a rejected env/config token says 'fix/unset it' / 'replace it in Settings > GitHub', not 'run gh auth login'. Used by the classified error, the summary, the gaps, and the views' card. The gh 401 body is trimmed to its status line. 7. Tool counts: 12 read / 8 write / 3 review = 23 everywhere. NITS - github_path_exists classifies FIRST (error_kind): auth/rate-limit/missing binary are UNVERIFIED errors, never a MISSING verdict; the 404 wording names the inaccessible-repo case. - README/PROTO state #23's named error is on the plugin's GET /config + the card (on read), not on Settings save. - gaps_for caps the why fragment at 80 chars so the host's ~300-char banner keeps the hint. - conftest: an autouse fixture clears GH_TOKEN/GITHUB_TOKEN, the token getter and the gh cache per test, so a dev box's ambient token can't skew hint assertions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
f843584 to
2e54593
Compare
There was a problem hiding this comment.
QA panel review — PASS
code-review-structural · head 2e54593bb704 · formal
No line-by-line review of PR #27 has actually happened yet, so there is no risk assessment to give and no findings to report — this round was consumed by a false premise, not a code review. The panel's finders all reported that the PR/head SHA could not be resolved and treated this as a full coverage gap; the verifier independently tested that claim and refuted it: github_pr_diff(repo="protoLabsAI/github-plugin", number=27) succeeds and returns a large, coherent diff (ci.yml, AGENTS.md, CLAUDE.md, PROTO.md, status.py/projects.py/review_tools.py, etc.). The only part of the finders' report that holds up is that github_read_file/github_read_pr_file 404 at the pinned head SHA — a narrower tool/environment quirk, not evidence the PR doesn't exist.
Fix-first item: re-run this review using github_pr_diff by PR number as the primary read path (falling back to the diff content itself, or retrying without a pinned ref, for any file-level lookups) so the actual diff — which touches CI config, docs, and several new tool modules — gets read at all.
What the panel disagreed on: nothing substantive, since no finder produced real findings; the entire round was a shared, apparently environment-caused failure to fetch the PR, which the verifier caught and reversed.
Gap: this remains a full coverage gap on the actual PR #27 changes — not because the PR is unreachable (it isn't) but because no finder has yet performed the read/analysis. Flagging explicitly per the instructions: this is a heavily-changed, entirely un-reviewed diff and should be re-run before merge.
No findings — the review came back clean.
findings JSON (machine-readable)
[]
Docs-only (plus one test docstring). The Project Manager archetype's first-run path —
docs/guides/build-with-a-coding-agent.mdand the pages around it — now describes what actually ships, including the behavior landing in #2977 (required-input gate, copied delegate, managed-project registration,report_setup_gap) and #2978 (PM preset rules). Those two PRs updateddocs/guides/bundles.md,docs/guides/plugins.md, ADR 0040, and the template bundle; this PR builds on them and doesn't duplicate them.Review note — #2977 / #2978 are OPEN. This PR cites their behavior as "core ≥ 0.146 (#2977)". Hold this one until they merge, or drop the version claims if they change.
What changed, by file
docs/guides/build-with-a-coding-agent.mdbr(cargo install beads_rust, NOT Homebrewbd; quotes the board's exact refusal text),git/gh(gh auth login; the GitHub rail's auth precedence, noting the Settings ▸ GitHub token field is github-plugin 0.6.0 — in flight). PATH reconciled withcoding-agents.md: the desktop build passes the login-shell PATH; an absolutecommandis the safe fallback for launchd/systemd/odd shells. Says what the console shows when each is missing (dropdown text, Board setup card / red callout, and the #2977report_setup_gapwarning banner once a board release adopts the seam — v0.41.4 doesn't yet).auto_mergetoggle, default on); the coder dropdown listsacpdelegates only and reads No coding (acp) delegates configured; the two required fields are a hard gate (create → 400 + cleanup; host install refuses to activate) — the old "create it anyway" paragraph is gone.writefollowsonboarding.write_default; a non-emptyprojects:IS the fs fence) with onboarding seeded only when noonboarding:exists and a remote was parsed, scoped to exactly the typed repo (root: <parent>,allow: [github.com/<owner>/<name>]). Mirrors the canonical wording indocs/guides/bundles.mdand links to it. Honest caveat: this ridesproject: trueon the bundle's repo input, which archetype v0.5.0 doesn't flag yet — until then add theprojects:entry yourself (replaces the hand-written instruction).--input project_board.repo=… --input project_board.coder=… --soul config/soul-presets/project-manager.md(#2977's new flags); required prompts must be answered or the create is refused, malformed--inputis a usage error, the delegate answer is copied from the host config without the host model, and without--soulthe workspace has no persona (and the CLI never records the capability contract). ThePOST /api/fleetbody and the resulting YAML follow for reference.propose_delegateworks from any running agent; the copied entry is verbatim, so fix the path on the host first.auto_merge: true, fifth field; the bare plugin defaults off); off = a human merges; ties to the #2978 rule thatauto_mergeis the authorization for the board's own reviewed PRs.workspace.yaml); a wizard-installed host PM never shows it — check Settings ▸ Capabilities ▸ Tools instead.list_agentsis the empty bench".docs/tutorials/first-agent.md— step 2 names the five fields, that repo + coder are required, and that the create is refused until anacpcoder is picked.docs/guides/delegates.md— new section Let the agent propose one (propose_delegate): validate → probe → park for approval; registers even on an empty roster; explicit approve only; autonomous turns fail closed; core ≥ 0.145 (#2953).docs/reference/starter-tools.md— thedelegatesrow documentslist_agentsandpropose_delegate(binds always).docs/reference/configuration.md—projectssection notes the Configure-step registration (#2977), thewrite_default/ fence consequence, and the allow-scoped onboarding seed rule.docs/guides/fleet.md— archetype list matches the catalog'sarchetypesarray (Basic, Cowork, Design System Engineer, Project Manager, Custom; Social Marketing is inheld:); quick-start shows the PM archetype with--input/--soul; the usage line gains[--input KEY=VALUE …] [--soul FILE]with the no-persona / no-host-model caveats.README.md— drops Social Marketing from the wizard's archetype list.0041,0042,0049,0055,0064,0072,0078,0083, plus the0083row indocs/adr/index.md.pm-stack→portfolio-manager-archetype(GitHub redirectspm-stackthere), notproject-manager-archetype; the single-repo PM ships fromproject-manager-archetype.portfolio-pluginis still the live plugin repo (not renamed) and is left as-is;leadEngineeris noted as the reference host fork, not a published archetype;cowork-stack→cowork-archetype.docs/dev/version-coherence.md,docs/plans/codified-delivery-loop.md— stale names annotated/updated in place (M7 now namesproject-manager-archetype; naming note at the top of the plan).tests/test_bundled_config_assets.py— docstring cites the config_inputs feature as #2934 (shipped by PR #2938) instead of #2938 alone. No behavior change.Gates (run locally in a worktree off
origin/main@ 423ec4ec)Anchors used by new cross-links were verified against the built HTML (
#let-the-agent-propose-one-propose-delegate,#_1-stand-up-the-pm,#_2-wire-a-coder,#configure-an-acp-delegate,#projects). Test-free by nature (docs); the one.pychange is a docstring.Follow-ups surfaced (not in this PR)
project-manager-archetypeneedsproject: trueon itsproject_board.repoinput for #2977's managed-project registration to fire (v0.5.0 predates the flag).projectBoard-plugindoesn't callregistry.report_setup_gapyet — the br / coder / gh banner is a seam without a caller until it does.🤖 Generated with Claude Code