fix(rate-limit-guard): align unwrap-before-compose with context-guard

[kyle-sexton]

Closes #3602

Summary

rate-limit-guard's setup still treated bare quoting as an sh -c wrap trigger, the bug context-guard already fixed with type -P / type -t. The twins had drifted on the second half of their compose logic.

Fix

• Move peel rules and the shell-syntax guard into reference/unwrap-before-compose.md, canonical in context-guard and synced byte-identical into rate-limit-guard.
• Guard triggers are unquoted top-level shell syntax, or a command word that is not an executable (type -P empty while type -t reports builtin/function/alias). Bare quoting is never a trigger.
• Add scripts/sync-unwrap-before-compose.sh plus its discriminator test, a CI unwrap-before-compose-sync job on ci-status.needs, and a registry row.
• Port the quoting / builtin / peel evals onto rate-limit-guard setup. The builtin-renderer expected sh -c argument uses the POSIX '\'' escape so printf round-trips ulimit '-n' (same correction on the context-guard twin eval).
• Bump rate-limit-guard 0.7.24 → 0.7.25 and context-guard 0.7.30 → 0.7.31.

Verification

• scripts/sync-unwrap-before-compose.sh --check — copies match.
• scripts/sync-unwrap-before-compose.test.sh — 7/7.
• scripts/check-lane-coverage.sh --check — 55 lanes reachable from ci-status.needs.
• scripts/check-cross-plugin-source-drift.sh --check — no unregistered or drifted clusters.
• scripts/check-changelog-parity.sh --check-bump origin/main — pass.
• scripts/check-changed-skills.sh origin/main — 2 skills, 0 failed.
• printf '%s\n' 'ulimit '\''-n'\''' prints ulimit '-n'.

Related

Follow-up from Phase 2c (legacy-statusline-detect extract). Refs #3601.

  

[claude]

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

---

Security review complete

• Gather context (PR diff, changed files)
• Skip-gate check
• Review changed files for security-relevant surface
• Report findings

No security issues found.

This PR is a documentation/prose refactor plus CI wiring, mirroring existing patterns already in the repo:

• .github/workflows/ci.yml: new unwrap-before-compose-sync job follows the exact shape of the sibling *-sync jobs (pinned actions/checkout SHA, persist-credentials: false, no secrets, no pull_request_target/workflow_run). Safe.
• scripts/sync-unwrap-before-compose.sh and scripts/sync-unwrap-before-compose.test.sh: thin wrappers around the existing shared scripts/lib/sync-cluster.sh engine (unchanged in this PR). Test fixtures build in mktemp -d, no injection surface. --check-bump's $base argument flows into properly-quoted git diff/git show invocations, same as the other sync gates.
• plugins/context-guard/skills/setup/reference/unwrap-before-compose.md (new) and the corresponding plugins/rate-limit-guard/skills/setup/reference/unwrap-before-compose.md copy: pure prose — the peel/wrap decision rules an agent follows when composing a statusLine JSON edit. No executable logic, no secrets, no injectable input handling. The actual behavioral fix (dropping "bare quoting" as a wrap trigger, using type -P/type -t for builtin detection) was already shipped and reviewed in context-guard; this PR only deduplicates it byte-identical into rate-limit-guard.
• plugins/*/skills/setup/SKILL.md, plugins/rate-limit-guard/skills/setup/evals/evals.json, CHANGELOG.md, plugin.json version bumps, scripts/cross-plugin-source-registry.txt: documentation, eval fixtures, and registry bookkeeping only.

Nothing here touches authN/authZ, secret handling, injection surfaces, or GitHub Actions trust boundaries beyond the already-established sync-gate pattern.

[github-actions]

Last security-reviewed head: cbcb1e1a88b6a85c27dbc547aa00d11997fd0c1c. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-02T13:40:04.212037Z	cbcb1e1	Draft marked ready

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[claude]

Claude finished @kyle-sexton's task in 23s —— View job

---

Security review — skipped (already covered)

• Gather context (PR diff, changed files, prior comments)
• Skip-gate check
• Report result

Skip gate hit: the head SHA for this run, cbcb1e1a88b6a85c27dbc547aa00d11997fd0c1c, is the same commit already covered by the prior security review on this PR ("No security issues found"), and the tracking comment confirms cbcb1e1 as the last security-reviewed head. Per /review:security-review's skip gate (rule 3: "this head already has a successful security review that still applies"), no new review is performed.

No further action needed unless new commits are pushed to this branch.

[claude]

Claude finished @kyle-sexton's task in 2m 11s —— View job

---

CI code review complete

• Gather context (PR diff, changed files)
• Skip-gate check
• Review changed files for correctness/maintainability
• Report findings

Scope: correctness/maintainability only (security lane covers this repo separately, and already reviewed this head).

Summary

The refactor does what it says: the peel rules and shell-syntax guard move from context-guard/skills/setup/reference/statusline-edit.md into a new shared reference/unwrap-before-compose.md, synced byte-identical into rate-limit-guard (verified directly — diff between the two copies is empty). The actual bug fix — rate-limit-guard's old inline prose treated a bare pipe/quoting as a wrap trigger; the new shared doc correctly scopes the shell-syntax guard to unquoted top-level syntax or a non-executable command word (type -P/type -t) — reads correctly in both SKILL.mds.

scripts/sync-unwrap-before-compose.sh and its test mirror the sibling sync-legacy-statusline-detect.sh pattern almost exactly (diffed the two — only the cluster-specific names differ), the CI job addition follows the same shape as the other *-sync jobs, the registry entry matches the established format, and scripts/affected-tests.sh's R5/R6 shared-lib fan-out is generic over scripts/sync-*.sh --print-manifest, so the new script needs no additional wiring there — confirmed by reading that logic, not just taking the PR description's word for it.

Finding

One inline comment on plugins/rate-limit-guard/skills/setup/evals/evals.json (new eval id 11): the expected escaped-renderer string doesn't round-trip through printf per the doc's own escaping rule, so it would misgrade a correct response. The identical string already exists, untouched, in plugins/context-guard/skills/setup/evals/evals.json at line 150 (out of scope for this PR's diff, but flagged since this PR is specifically about fixing this exact class of bug).

Not independently re-run

The PR's Verification section lists scripts/sync-unwrap-before-compose.sh --check, its test suite, and several other gate scripts as passing. Sandboxed bash execution wasn't available in this review environment, so those results are author-claimed and unverified by me directly — I instead verified the underlying properties (byte-identical copies, unchanged shared libs, correct CI/registry wiring) by reading the files.

[github-actions]

Claude has reviewed this PR 3 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[claude]

Claude finished @cursor[bot]'s task in 2m 43s —— View job

---

CI code review complete

• Gather context (PR diff, changed files, prior comments)
• Skip-gate check
• Review changed files for correctness/maintainability
• Report findings

Scope: correctness/maintainability only (security lane covers this repo separately, and already reviewed the prior head with no findings).

Summary

HEAD is now 1b90767, one commit past the last full review (cbcb1e1). That commit, "fix: make the builtin-renderer eval escape round-trip", touches only the two evals.json files and resolves the one finding from the prior review pass (the eval-11 / eval-15 ulimit expected-output string that didn't round-trip through printf).

Verified directly, not just diffed:

• Extracted the fixed expected_output strings for rate-limit-guard eval id 11 (L108) and context-guard eval id 12 (L140) and hand-traced the escaping: 'ulimit '\''-n'\''' decodes to ulimit '-n' — quotes preserved, matching the actual shim renderer and the doc's own escaping rule in reference/unwrap-before-compose.md. This is the exact fix requested in the earlier inline comment.
• Normalized both evals.json files before/after this commit with jq -S (which decodes all \uXXXX escapes uniformly) and diffed — confirmed the only semantic change in each file is that one expected_output string. The raw git diff looks noisy (lots of — → — churn) because this commit's JSON write path escapes non-ASCII, but that's not new inconsistency: every other evals.json in the repo (docs-hygiene, repo-fleet-hygiene, visualization, work-items) already stores em dashes as —, so this actually brings these two files in line with the existing convention rather than away from it.
• Re-confirmed plugin.json versions (context-guard 0.7.31, rate-limit-guard 0.7.25) match their CHANGELOG.md headers, and the two reference/unwrap-before-compose.md copies are still byte-identical (diff empty) — unaffected by this commit, as expected.

No new findings on this delta. The rest of the PR (sync script, CI wiring, reference doc, SKILL.md changes) was already reviewed in depth on cbcb1e1 with only the now-fixed eval-string issue flagged — nothing else surfaced on a fresh look at those files.

Not independently re-run

Sandboxed shell execution in this environment doesn't include python3, so the JSON diffs above were done with jq -S instead (equivalent for this purpose — it also decodes \uXXXX escapes). The PR's listed verification commands (sync-unwrap-before-compose.sh --check, its test suite, check-lane-coverage.sh, etc.) were not re-run here; treat those as author-claimed.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cbcb1e1a88

ℹ️ 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".

[claude]

Claude finished @cursor[bot]'s task in 1m 52s —— View job

---

CI code review complete

• Gather context (PR diff, changed files, prior comments)
• Invoke /review:code-review procedure
• Review changed files for correctness/maintainability
• Post findings
• Final summary

Scope: correctness/maintainability only (security lane covers this repo separately).

What changed since the last review

The only diff between the previously-reviewed head (cbcb1e1) and this one (ee307a8) is the fix for the escaping bug flagged in the prior inline review comment: eval id 11 in plugins/rate-limit-guard/skills/setup/evals/evals.json and the identical id 15 in plugins/context-guard/skills/setup/evals/evals.json.

I verified the fix directly rather than taking the diff at face value:

• The expected_output now reads sh -c 'ulimit '\''-n'\''' in both files.
• That round-trips correctly: printf '%s\n' 'ulimit '\''-n'\''' → ulimit '-n' (quotes preserved), matching the actual renderer.
• It matches the doc's own escaping rule in reference/unwrap-before-compose.md:85 ("replace every ' in it with '\''").
• Confirmed both evals.json files still parse as valid JSON (jq empty).

One intermediate commit (1b90767) had re-serialized the entire evals.json files as a side effect (e.g. turning literal em-dashes into — escapes across unrelated eval entries), but the very next commit (ee307a8, "keep eval JSON bytes and only correct the builtin escape") cleanly reverted that collateral churn — confirmed the net diff from cbcb1e1 touches only the one intended line in each file, nothing else.

Result

No new findings. The previously-flagged escaping bug is fixed correctly in both plugins, and the fix is minimal and byte-scoped as intended.