sagemaker-ai was dropped from the marketplace in #1762 (validate-plugins
adoption) due to a manifest/YAML error. AWS has since fixed it; the plugin
validates clean at awslabs/agent-plugins@187edde (claude plugin validate passes).
Re-listed as a git-subdir source SHA-pinned to current monorepo HEAD,
matching its sibling AWS entries (deploy-on-aws, databases-on-aws).
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Both plugins in awslabs/agent-plugins had their subpaths edited in commit
187edde (after the morning bump cron pinned them to f16aaf2a), so they fell
behind again on merge. Manual catch-up bump to current monorepo HEAD.
- databases-on-aws: 4 files changed under plugins/databases-on-aws/ (v1.1.0)
- deploy-on-aws: 7 files changed under plugins/deploy-on-aws/ (v1.2.0)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PR #2112's telemetry visibility surfaced an immediate finding from
the first 3h of v2.0.1 data: **2,406 phase=2 / err=99 sessions** —
"venv stage / uncategorized" — dominating BUILD_FAILED. The original
err_kind detection patterns were all pip-flavored (pip_no_match,
dns_fail, ssl_verify, etc.) and didn't catch venv-creation failure
modes, so they all collapsed to the catch-all _uncategorized (99)
bucket.
This PR fills the gap on two axes.
## 1. Five new venv-specific err_kind categories (codes 11-15)
Each gated on `err_phase == "venv"` so the same substring doesn't
mis-fire in pip-phase failures:
- 11 `venv_ensurepip_fail` — Debian/Ubuntu without python3-venv
installed; stderr matches "ensurepip is not available" or
"ensurepip ... returned non-zero". Predicted to be the biggest
chunk based on Linux distro market share.
- 12 `venv_path_too_long` — Windows MAX_PATH (260) or POSIX
ENAMETOOLONG. Triggered when state_dir + venv layout exceeds
the path limit (deep Lib/site-packages/<pkg>/<...> paths).
- 13 `venv_no_module` — `python3 -m venv` itself missing
("No module named 'venv'"). Rare but distinctive.
- 14 `venv_already_exists` — Errno 17 / "file exists" — sentinel
race past O_EXCL or stale dir survived `--clear`.
- 15 `venv_setup_failed` — generic "virtual environment was not
created successfully" catch-all for venv setup failures that
don't match a more specific category.
All 5 occupy reserved slots in SDK_BOOTSTRAP_ERR_CODES per the
APPEND-ONLY contract from PR #2112.
## 2. `sdk_bootstrap_stderr_sig` integer hash
For "other:<tail>" err_kinds (which encode to _uncategorized = 99),
emit a bounded integer hash (0-999) of the first ~30 chars of the
stderr tail. This restores cardinality to the _uncategorized bucket
in BQ aggregation without unbounded keyspace — same stderr message
always maps to the same bucket, so a real failure mode replicating
across thousands of machines clusters cleanly. Bounded at 1000
buckets: well below any "high cardinality" alarm but wide enough to
distinguish ~30 distinct dominant patterns (birthday-paradox
collision probability ~50% at ~37 distinct inputs).
The field auto-omits (`if sig:` gate) when err_kind is categorized
— no key-budget cost on the common-case categorized failures.
## Version bump 2.0.1 → 2.0.2
PR #2114 confirmed the version-bump mechanism is the only way to
propagate code changes to the existing fleet — without a bump, CC's
plugin updater short-circuits on string-equality of installation
version vs marketplace version. Following the policy we established:
**bump patch on every functional PR**.
By 17:31:42Z on 2026-06-01 (1m22s after #2114 merged), v2.0.1 was
already appearing in BQ. v2.0.2 should follow the same propagation
curve — ~30% adoption within 3 hours, full convergence within a few
days.
## Verified locally
- py_compile clean.
- 15 new tests in test_venv_failure_deepdive.py (added to internal
test suite at sg-staging/tests/, not in this PR):
* 5 parametrized: each new err_kind maps to its expected code (11-15).
* 1 APPEND-ONLY regression: existing codes 1-10 + 99 unchanged.
* 6 stderr_sig: non-other inputs → 0; None/empty → 0; deterministic
same-input → same-output; bounded to 0-999; distinct inputs →
distinct hashes (5/5 with P(collision) ≈ 1%); leading-chars focus
(path-varying stderr with shared 30-char prefix collide as designed).
* 1 static-shape catcher: every new `err_kind = "venv_..."` branch
in main() is guarded by `err_phase == "venv"`. Catches the
regression where someone adds a venv pattern without the phase
gate and starts mis-categorizing pip-phase failures.
* 1 map-coverage: all err_kind strings assigned anywhere in
ensure_agent_sdk.main() are present in SDK_BOOTSTRAP_ERR_CODES
(catches new categories added in code but forgotten in the map).
* 1 emit-shape: the metric block uses `_encode_stderr_sig`, the
`sdk_bootstrap_stderr_sig` key is written conditionally on `if
sig:`. Catches the regression where someone removes the
helper or makes the emit unconditional (would pad every
categorized BUILD_FAILED row with a zero-valued field).
- Full suite: 452/452 pass + 2 skipped (live API tests, opt-in).
## What this unblocks in BQ
```sql
-- For the 2,406 sessions/3h that were phase=2/err=99 on v2.0.1,
-- v2.0.2+ will split them across the new categories. Query:
SELECT
CAST(JSON_VALUE(additional_metadata, "$.sdk_bootstrap_err") AS INT64) AS err,
CAST(JSON_VALUE(additional_metadata, "$.sdk_bootstrap_stderr_sig") AS INT64) AS sig,
COUNT(*) AS sessions
FROM `proj-product-data-nhme.raw_events.claude_code_internal_event`
WHERE _PARTITIONTIME >= ...
AND CAST(JSON_VALUE(additional_metadata, "$.sdk_bootstrap") AS INT64) = 3
AND CAST(JSON_VALUE(additional_metadata, "$.sdk_bootstrap_phase") AS INT64) = 2 -- venv
GROUP BY err, sig
ORDER BY sessions DESC
```
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The nvidia-skills entry was added in PR #2088 with:
"source": {
"source": "git-subdir",
"url": "https://github.com/NVIDIA/skills.git",
"path": "plugins/nvidia-skills",
"ref": "main"
}
It's missing the required `sha` field. The marketplace validator
enforces invariant I5 ("source.sha is missing or not a 40-char hex
SHA") on every git-subdir source — without it, the action fails:
##[error]invariant I5: nvidia-skills: source.sha is missing or not
a 40-char hex SHA
This has been silently failing the "Validate Plugins" CI on every
PR that touches marketplace.json since #2088 merged on 2026-05-03.
Confirmed by checking the last 5 completed validate runs on main —
all 5 ❌, including PR #2114 (security-guidance bump that you merged
earlier today). The validator failure was getting swallowed because
all the other PR-level checks (Check MCP URLs, Scan Plugins, Validate
Plugin Licenses) were passing, and humans were `gh pr merge --admin`-ing
through it.
Fix: add the sha field pinned to the current upstream HEAD of
github.com/NVIDIA/skills.git on the `main` branch.
Resolved via: git ls-remote https://github.com/NVIDIA/skills.git refs/heads/main
SHA: 62b685a20ac45285cafd1e22782abbed33172c17
This mirrors the shape of other git-subdir entries with both `ref`
and `sha` (e.g. 42crunch-api-security-testing pins ref="v1.5.5",
sha="b404d99a...", adobe-for-creativity pins ref="main", sha="8d74ee6b...").
Unblocks every in-flight PR that touches marketplace.json — including
PR #2154 (security-guidance venv-deepdive) which is currently
red-blocked on this.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bump: switch to per-entry PR mode (one PR per stale plugin)
Replaces the single batched bump PR with one PR per stale plugin so a
single failing plugin no longer blocks the rest. Pins to a feature
branch of the bump-plugin-shas action that adds 'pr-mode: per-entry';
re-pin to the merge commit on the action's main when that lands.
- pr-mode: per-entry → one PR per plugin on bump/<slug>
- max_bumps default lowered 130 → 30 (per-entry scans cost more)
- scan dispatch fanned out over pr-urls JSON (one per per-entry branch)
- header comments updated for per-entry semantics
* bump: re-pin to merged composite action SHA on -community main
The pr-mode: per-entry input now lives on main of the bump-plugin-shas
action (merged at e2019b2a). Update the pin and drop the now-stale
header comment that tracked the feature branch.
* bump: dispatch all three required checks per per-entry PR
Bump PRs are opened with GITHUB_TOKEN, which doesn't fire on:pull_request
(recursion guard). The per-entry cutover already dispatched scan-plugins.yml
per branch to satisfy the `scan` required check, but `check` (Check MCP URLs)
and `validate` (Validate Plugins) are also required on main and likewise
never fired — leaving every bump PR BLOCKED on missing checks (observed on
the batched #2079, which only cleared after a human-authored push re-fired
the pull_request workflows).
Fix: dispatch all three workflows per per-entry bump branch. Each runs its
job unconditionally on workflow_dispatch, so the check run lands on the
branch HEAD (= PR head) and satisfies the required check.
- validate-plugins.yml: add workflow_dispatch trigger (check-mcp-urls.yml
already had one). gh workflow run requires the trigger on the default
branch; this lands together with the per-entry bump so main stays
consistent.
- bump-plugin-shas.yml: loop the dispatch over
{scan-plugins,check-mcp-urls,validate-plugins}; tolerate a single
transient dispatch failure (warn, don't abort) so one hiccup can't
strand the rest of the batch.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* bump: fail the per-entry check-dispatch step when a dispatch fails
The dispatch step logged each failed gh workflow run as a warning and exited 0, so a transient API error or rate limit could leave a per-entry bump PR missing a required check while the bump run still showed green. The composite action skips slugs with an open PR, so the stranded PR was never retried.
Attempt every dispatch (one failure must not strand the other branches), record failures via a temp file (the while loop runs in a pipe subshell), then emit an error and exit non-zero if any dispatch failed, so the bump run goes red and the affected PR can be re-dispatched.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The 8 PRs we shipped since 2026-05-26 (#2076, #2077, #2078, #2086,
#2091, #2100, #2101, #2105) all changed plugin code without bumping
the version. CC's plugin updater uses string equality for the
freshness check (pluginOperations.ts:1835):
const isUpToDate =
installation.version === newVersion ||
installation.installPath === versionedPath ||
installation.installPath === zipPath
if (isUpToDate) return { alreadyUpToDate: true }
Users who installed v2.0.0 anywhere between 2026-05-26 and 2026-05-31
have `installation.version === "2.0.0"` in their installed_plugins.json.
The marketplace also advertises "2.0.0" (until this commit), so
isUpToDate returns true and the plugin cache directory is never
refreshed — they keep running whatever 2.0.0 code was current on the
day they installed. The marketplace git pull happens; the per-user
cache install does NOT.
Empirical evidence: in BQ today (5/31) on Windows v2.0.0 fires,
**73% emit sdk_bootstrap outcome 4 (SKIP_WIN32)** — a code path
retired in PR #2055's Windows-enable fix. Those users are running a
plugin tree that pre-dates the fix, even though their telemetry
shows pv=20000.
The fix is a one-line version bump. Once the marketplace advertises
2.0.1, every CC autoupdate cycle sees installation.version (2.0.0)
!= newVersion (2.0.1), installs the new version, and the user's next
session loads the fixed code.
This PR:
1. plugins/security-guidance/.claude-plugin/plugin.json: 2.0.0 → 2.0.1
2. .claude-plugin/marketplace.json security-guidance entry: 2.0.0 → 2.0.1
What 2.0.1 carries (versus 2.0.0 as published 5/26):
- #2076 — Graphite gt commit/push detection
- #2077 — hookSpecificOutput.additionalContext on async-rewake exit-2
- #2078 — CLAUDE_CONFIG_DIR support
- #2086 — core.quotePath=false on diff feeders (Arabic/Hebrew/CJK paths)
- #2091 — fix Bash(...|...) if-clause regression from #2076
- #2100 — drop text=True from subprocess.run, bake PYTHONUTF8=1 (Windows non-cp1252 path crash)
- #2101 — core.quotePath=false on GIT_CMD globally
- #2105 — output_format → output_config.format API migration (#2098)
Verified locally:
- plugin.json + marketplace.json both valid JSON.
- _read_plugin_version_int() returns 20001 (was 20000).
- Existing test suite passes — 408 tests, no regressions caused by
the version bump itself. (29 unrelated failures are from
test_telemetry_failure_signals.py which expects PR #2112's
not-yet-merged code.)
Going forward: bumping `patch` on every functional PR closes this
gap entirely. Without that policy, every fix only reaches NEW
installs, never the existing fleet.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fills two failure-visibility gaps in plugin telemetry.
## Gap 1: HTTP errors from _call_claude invisible
Before: a 4xx/5xx response from the LLM API caused `_call_claude` to
return None and produce ZERO fingerprint in tengu_hook_plugin_metrics.
A failed call looked identical to "no review needed". The recent
deprecation-400 outage (PR #2105, output_format → output_config.format,
#2098) was invisible in aggregate dashboards until a user manually
reported errors from their debug log. Cohort-specific or partial
outages would never show up in BQ.
Fix: add `http_err_last` (most recent status) and `http_err_count`
to the existing `_USAGE` accumulator in `_base.py`. `_usage_metrics()`
snapshots them whenever count > 0 (skip-path no-pollute contract
preserved when count == 0). All `_call_claude` error sites now call
the new `_record_http_error()` helper alongside the existing
`_last_call_claude_http_error` module-state assignment.
Now any future API failure category is queryable in BQ in real time:
SELECT
DATE(server_timestamp, "America/Los_Angeles") AS d,
CAST(JSON_VALUE(additional_metadata, "$.http_err_last") AS INT64) AS code,
COUNT(*) AS n
FROM ... WHERE event_name = "tengu_hook_plugin_metrics"
AND JSON_VALUE(additional_metadata, "$.pluginId") LIKE "%security-guidance%"
AND JSON_VALUE(additional_metadata, "$.http_err_count") IS NOT NULL
GROUP BY d, code ORDER BY d, n DESC
## Gap 2: sdk_bootstrap_phase / sdk_bootstrap_err always NULL in BQ
Before: ensure_agent_sdk.py emitted these as strings (e.g. "pip",
"dns_fail"). CC's plugin-metrics pipeline silently drops
plugin-emitted string values — only bool|finite-number plugin metrics
reach BigQuery. (CC-core fields like `subscription_type` are exempt
because they're injected downstream of plugin validation.)
Confirmed empirically: ~185K BUILD_FAILED rows in BQ over the past 2
days had `sdk_bootstrap_phase` = NULL and `sdk_bootstrap_err` = NULL
despite the Python code emitting them. ~28K BUILD_FAILED sessions/day
had no diagnostic split — flying blind on whether the failures are
pip-no-match vs dns-fail vs ssl-verify vs proxy-auth etc.
Fix: encode phase + err_kind as stable integers via
SDK_BOOTSTRAP_PHASE_CODES and SDK_BOOTSTRAP_ERR_CODES. Phase: 1=pre,
2=venv, 3=pip, 4=main. Err: 10 known categories (1-10), 11-98
reserved, 99 = uncategorized catch-all (covers "exc:<X>", "other:<X>",
and unmapped strings). APPEND-ONLY for telemetry stability.
Also corrects the misleading "CC accepts string metric values"
comment in ensure_agent_sdk.py that led to the bug originally.
Verified locally on macOS Python 3.13:
- py_compile clean.
- 32 new tests in test_telemetry_failure_signals.py (added to
internal test suite at sg-staging/tests/, not in this PR):
* 4 HTTP-error tracking unit tests: _record_http_error increments
count + tracks most-recent; handles None/invalid; -1 for
network/timeout.
* 4 _usage_metrics emission tests: empty when no activity;
successful call has no http_err fields; failure-only has http_err
and no api_calls; mixed has both.
* 1 contract test: every emitted value is bool|finite-number
(catches future regression of the string-dropping bug class).
* 13 sdk_bootstrap encoding tests (parametrized over the 10 known
err_kind categories + 5 catch-all shapes): each maps to the
right integer; unknown phase = 0; unknown err = 99.
* 1 static-shape regression catcher: every `err_kind = "..."`
string in ensure_agent_sdk.main() must be in
SDK_BOOTSTRAP_ERR_CODES (otherwise new err_kinds silently
collapse to 99).
* 2 emit-shape regression catchers: the assignments in main() go
through _encode_phase / _encode_err_kind helpers (no raw
strings); no literal string values for sdk_bootstrap_phase/err.
* 1 comment-accuracy: the misleading "CC accepts string metric
values" comment is gone.
- Full suite: 437/437 pass + 2 skipped (live API tests, opt-in).
NOT verified end-to-end against BQ — would require shipping +
observing in production for 24h to confirm the http_err and
sdk_bootstrap_phase/err fields actually appear in
tengu_hook_plugin_metrics rows. The unit tests pin the contract; if
the wire shape is broken, BQ will show NULL for the new fields and we
revisit (with the same diagnostic the BUILD_FAILED bug gave us).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fixes#2098. The Anthropic Messages API moved structured-output
schema specification from a top-level `output_format` field to a
nested `output_config.format` field, per
https://platform.claude.com/docs/en/build-with-claude/structured-outputs.
Per docs the old form "will continue working for a transition period"
— and indeed for api-key + non-streaming auth it still returns HTTP
200 (verified via live API). But OAuth Bearer users with CLI 2.1.158
hit `invalid_request_error: output_format: This field is deprecated.
Use 'output_config.format' instead.` consistently — reporter saw 462
errors in one day. The trigger appears to be auth mode + possibly
stream:true (their controlled curl bypass used Bearer + stream=true);
api-key + non-streaming was my initial repro attempt and didn't fire.
The bug only affected `_call_claude` (the legacy direct-urllib path).
The agentic `_agentic_review` path goes through claude_agent_sdk →
subprocesses to the `claude` CLI binary, which already uses the new
`output_config.format` shape correctly (per src/utils/sideQuery.ts:263
in claude-cli-internal). So this PR only needs to fix the plugin's
direct HTTP path.
This commit:
1. llm.py: rewrite the payload literal in `_call_claude` to use
`output_config: { format: { type: 'json_schema', schema: ... } }`
instead of top-level `output_format`.
2. llm.py: in the adaptive-thinking branch, MERGE `effort: "high"`
into the existing `output_config` dict instead of reassigning.
Reassignment would silently clobber the format schema set in (1).
The pre-existing code did `payload["output_config"] = {"effort":
"high"}` which was correct WHEN output_format was top-level (and
output_config wasn't otherwise used). With the migration the
existing dict carries the schema, so we extend it not replace it.
Verified locally on macOS Python 3.13:
- py_compile clean.
- Existing 401 tests still pass — 0 regression.
- 6 new tests in test_2098_output_config_format.py (added to
internal test suite at sg-staging/tests/, not in this PR):
* 2 static-shape: the `_call_claude` source no longer contains
top-level `"output_format":` AND uses `output_config`. The
adaptive-thinking branch does NOT reassign output_config (and
DOES set output_config['effort']). Catches the regression
class where a future refactor reintroduces either bug.
* 2 payload-shape unit (mocked urllib): both thinking_budget=0
and thinking_budget>0+adaptive code paths produce a payload
with the correct `output_config.format` shape AND no
`output_format` top-level. The adaptive path verifies both
`format` and `effort` coexist in output_config (i.e., the
merge fix works).
* 2 live-API gating (skip-on-no-key): the new shape returns
HTTP 200 against api.anthropic.com; the old shape's current
status is recorded for canary purposes (still 200 for
api-key today, but reporter shows it's 400 for OAuth).
- Full suite: 405/405 pass + 2 skipped (live API tests, opt-in).
- The reporter's exact deprecation 400 message reproduces if you
swap auth to OAuth Bearer + stream:true (could not test locally
without extracting the keychain OAuth token, which was out of
scope). The fix shape is API-contract-level so it doesn't depend
on which auth mode triggers the 400.
NOT verified end-to-end via OAuth-authenticated plugin invocation on
my machine (auto-mode classifier correctly declined to extract the
keychain token). Reporter's 462 production errors + the docs
migration notice + the live-API HTTP 200 on the new form are
sufficient evidence to ship.
Closes#2098.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Followup to PR #2086 (which added the flag to 4 specific git call
sites) and PR #2100 (text=True purge for #2099).
The Windows reporter for #2099 noticed more git invocations still
lacked the flag — rev-parse path queries (--show-toplevel, --git-dir,
--git-common-dir), reflog %gs subjects, and `git show <sha>:<path>`
all output paths but the per-site PR #2086 approach missed them. The
result: an Arabic-named directory shows up via _git_diff_range but
rev-parse-emitted paths get C-quoted, breaking downstream
os.path.isabs() checks.
Fix: add `-c core.quotePath=false` to GIT_CMD itself as the 4th
config-set. Every subprocess.run using the *GIT_CMD splat picks it
up automatically — diff feeders, rev-parse path queries, reflog log,
ls-files, status, git show. No more per-site flag duplication.
This commit:
1. gitutil.py: add -c core.quotePath=false to GIT_CMD.
2. Remove the now-redundant per-site flags at the 7 call sites that
previously had inline -c core.quotePath=false (cleanup, since the
global setting subsumes them):
gitutil.py: _git_diff_range, _git_name_only, _git_status_porcelain,
get_git_diff (4 sites)
diffstate.py: _list_untracked git ls-files (1 site)
security_reminder_hook.py: commit-review git diff + git show (2 sites)
Verified locally on latest main (post PR #2100 merge) with macOS
Python 3.13:
- py_compile clean on all 3 modified files.
- Bare main BEFORE my fix: 400/401 pass — 1 failure proves the gap
(test_git_cmd_contains_quotepath_false catches the missing flag).
- Main + my fix: 401/401 pass.
- 23 new tests in test_quotepath_global.py (added to internal test
suite at sg-staging/tests/, not in this PR):
* 1 GIT_CMD-level: GIT_CMD list contains core.quotePath=false as
a (-c, value) pair. Single source of truth — single place a
future PR will be caught if the flag gets dropped.
* 10 static-shape (one per hooks/*.py): every subprocess.run
uses the *GIT_CMD splat (no bare git invocation that would
bypass the global flag).
* 12 end-to-end (parametrized over Arabic, Hebrew, CJK directory
names): real git repo, _git_diff_range emits unquoted diff,
extract_file_paths_from_diff and parse_diff_into_files keep
the non-ASCII path in their output, _git_toplevel returns the
non-ASCII path intact.
- 1 staleness fix in test_diff_parser_non_ascii.py
(test_no_bare_git_diff_or_show_without_flag): updated to accept
EITHER inline core.quotePath=false OR *GIT_CMD splat (which
globally provides it).
NOT verified end-to-end on Windows with a non-ASCII repo root path.
The new global-flag test pins the contract permanently, and the
parametrized macOS tests confirm parser behavior on ASCII-control
paths in non-ASCII directories. The Windows-specific rev-parse
quoting behavior follows from the same git contract our macOS test
environment exercises (POSIX git always emits raw UTF-8 regardless
of quotePath; on Windows the flag is what makes output raw).
Closes the #2099 followup specifically about _git_diff_range /
rev-parse --show-toplevel / git log %gs paths slipping past.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
URGENT WINDOWS FIX. Sibling of #2056 / PR #2075 but covering 14 more
sites that PR #2075 missed.
The bug class: on Windows with cp1252 default encoding (typical
en-US locale), `subprocess.run(..., text=True)` decodes child stdout
AND stderr via `locale.getpreferredencoding()`. When git emits a
UTF-8 byte that's undefined in cp1252 (e.g. `0x81` from ف, present
in any path/filename/branch ref/commit message containing
Arabic/Hebrew/CJK), Python's internal `_readerthread` raises
UnicodeDecodeError. The thread crash is silent in Python 3.13+ (only
printed to stderr), but `subprocess.run` returns `stdout=None` and
the caller AttributeErrors on `.strip()`. The user sees a misleading
"WinError 267" or similar catch-all message instead of the real
decode failure.
PR #2075 fixed 6 specific helpers in `diffstate.py` / `gitutil.py`.
This commit covers the 14 survivors. Plus a defense-in-depth belt:
`PYTHONUTF8=1` exported by sg-python.sh.
This commit:
1. sg-python.sh: `export PYTHONUTF8=1` (PEP 540). No-op on
macOS/Linux (already UTF-8). On Windows, makes Python's
`locale.getpreferredencoding()` return UTF-8 instead of cp1252 —
so even if a future regression slips in text=True, the decode
succeeds. Must be set BEFORE Python starts; changing it from
inside the interpreter has no effect.
2. gitutil.py: convert 8 subprocess.run sites from
`capture_output=True, text=True` to `capture_output=True` +
manual `r.stdout.decode("utf-8", errors="replace")`:
- _git_rev_parse_head (stdout = SHA, stderr risk)
- _find_git_index (stdout = PATH, primary bug site)
- _temp_index git add (returncode only, stderr risk)
- _git_toplevel (stdout = PATH, primary bug site)
- _git_dir (stdout = PATH, primary bug site)
- _git_rev_list_range (stdout = SHAs, stderr risk)
- _detect_main_branch (stdout = ref, stderr risk)
- merge-base --is-ancestor (returncode only, stderr risk)
3. security_reminder_hook.py: convert 6 subprocess.run sites
(rev-parse @{u}/@{u}@{1}/local_ref, merge-base, HEAD lookup,
reflog SHA resolution) — same pattern.
4. security_reminder_hook.py: fix the misleading log line in
handle_user_prompt_submit. Was:
debug_log("Failed to capture git baseline (not a git repo?)")
Now includes the cwd in the message so the next reporter doesn't
waste an hour grepping for the real WinError, per reporter's
secondary finding.
Verified locally on macOS Python 3.13:
- py_compile clean on all modified files.
- bash -n sg-python.sh clean.
- sg-python.sh actually propagates PYTHONUTF8=1 to child Python
(verified via probe — sys.flags.utf8_mode=1).
- Existing 353 tests still pass — 0 regression.
- 25 new tests in test_2099_subprocess_text_true.py:
* 10 static-shape catchers (one per hooks/*.py file). Any
future PR that reintroduces text=True OR encoding= in
subprocess.run fails this check at PR time. Single source
of truth for the regression class.
* 2 sg-python.sh verifiers (literal export + actual
propagation to child Python).
* 5 macOS end-to-end against a real git repo containing
non-cp1252 content (`ف.py` filename): _git_toplevel,
_git_dir, _find_git_index, _git_rev_parse_head,
_git_rev_list_range all return clean values without
AttributeError / UnicodeDecodeError.
* 7 round-trip bytes-decode pattern verifiers (parametrized
over Arabic ف, Hebrew א, Japanese 案, raw 0x81, multiple
cp1252-undefined bytes, real-world git diff headers).
* 1 sanity check that cp1252 strict DOES raise on 0x81
(proves the test environment can catch the bug class).
- Full suite: 378/378 pass in 5.56s.
- End-to-end tmux smoke test driving real claude 2.1.145 CLI:
Made a git commit via Bash tool call. All 4 hooks fired through
the fixed plugin path:
11:28:16.730 Hook called with args: …/plugin/hooks/security_reminder_hook.py
11:28:16.734 Processing: hook_event=UserPromptSubmit
11:28:16.825 Captured git baseline: 445f7f213256
11:28:19.923 Hook called with args: …
11:28:19.923 Processing: hook_event=PostToolUse, tool=Bash
11:28:19.971 Commit review: detected git commit in command
11:28:20.020 Commit review: 1/1 sha(s) resolved, 1 files
11:28:26.415 Hook called with args: …
11:28:26.416 Processing: hook_event=Stop
11:28:26.550 Stop hook: empty review set
Confirms: PYTHONUTF8=1 export doesn't break anything; converted
helpers (_git_rev_parse_head, _git_toplevel, _git_dir,
_find_git_index) run end-to-end without issue on the happy path.
NOT verified end-to-end on Windows with actual non-cp1252 content
in path/filename/stderr. The static-shape catcher pins the
regression class permanently. Reporter's PYTHONUTF8=1 workaround
empirically proves the encoding-mode fix works for the affected
scenario; this commit just bakes it in.
Closes#2099.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
