feat(session-flow): add usage-limit reset checker for keep-going

[kyle-sexton]

Fixes #1321

Adds check-usage-limit-reset.py so keep-going can compare a worker's stated reset time to the clock without model arithmetic.

Test plan

• python3 check-usage-limit-reset.test.py (3 cases)

Related

• Closes keep-going: the usage-limit reset check is prose guidance with no mechanism #1321

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[claude]

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

---

Security Review

• Gather context (PR diff, changed files)
• Review changed files for security issues
• Report findings

This PR adds a small, self-contained Python CLI (check-usage-limit-reset.py) plus its test, a SKILL.md doc update showing how to invoke it, a version bump, and a changelog entry. Reviewed all 5 changed files against origin/main.

Findings

IMPORTANT — documented invocation interpolates untrusted text into a shell command (moderate confidence)

SKILL.md#L136-L138 instructs the agent to run:

python3 "${CLAUDE_PLUGIN_ROOT}/skills/keep-going/scripts/check-usage-limit-reset.py" "<limit message text>"

<limit message text> is meant to be filled in with the worker/subagent's actual limit-message text and executed via a real shell (the Bash tool). If an agent follows this pattern literally — substituting the message text directly inside the double-quoted argument rather than passing it as a properly-escaped argument — any shell metacharacters in that text (`, $( ), ", ;, !) would be interpreted by the shell, not passed through as a literal string. Double quotes in bash do not neutralize command substitution or embedded quotes.

In the common case this text is Anthropic's own system-generated limit banner, so the practical exploit window is narrow. But the skill explicitly targets the orchestration case — inspecting "a worker or subagent" and acting on "its real output" — and keep-going's own doctrine elsewhere is "read the real output first." If that output is ever paraphrased/relayed through content the worker processed (e.g. logs, transcripts, or text influenced by external/adversarial input the subagent handled), a crafted string designed to look like a reset clause but containing shell metacharacters could achieve command injection when the orchestrating agent copies it into the shown command form.

The script already supports a safer alternative — reading the message from stdin when no positional argument is given (check-usage-limit-reset.py#L60-L62). Recommend the doc example pipe the text via stdin (or via a shell variable set through read/heredoc rather than literal inline interpolation) so no untrusted text is ever assembled into a shell command line:

printf '%s' "$LIMIT_MESSAGE" | python3 "${CLAUDE_PLUGIN_ROOT}/skills/keep-going/scripts/check-usage-limit-reset.py"

Fix this →

Considered, not a real issue: zone = ZoneInfo(tz_name) at check-usage-limit-reset.py#L43 uses a regex-captured, attacker-influenceable tz group as a timezone key. This looked like a potential path-traversal-into-tzdata read, but CPython's zoneinfo module validates keys and rejects .. components / absolute paths / NUL bytes before any file lookup, so this isn't exploitable — any malformed key just raises and is caught by the surrounding except Exception, yielding exit code 2.

No other issues found: the script does no file I/O, network calls, eval/exec, or shell execution of its own; it handles no secrets/tokens/credentials; plugin.json and CHANGELOG.md changes are metadata-only; the .test.py file only exercises the script via subprocess.run with an argument list (no shell=True), which is safe.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c01ae0cd24

ℹ️ 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 @kyle-sexton's task in 2m 1s —— View job

---

Security Review

• Gather context (PR diff, changed files)
• Review changed files for security issues
• Report findings

Reviewed all 5 files changed in this PR (git diff ef10475~1 ef10475) against origin/main: the new check-usage-limit-reset.py script, its .test.py, the SKILL.md doc update, and the metadata-only plugin.json/CHANGELOG.md changes.

Findings

IMPORTANT — documented invocation interpolates untrusted text into a shell command (moderate confidence)

SKILL.md#L136-L138 still documents:

python3 "${CLAUDE_PLUGIN_ROOT}/skills/keep-going/scripts/check-usage-limit-reset.py" "<limit message text>"

<limit message text> is meant to be substituted with a worker/subagent's real limit-message text and run via the Bash tool. If an agent follows the example literally — dropping the message text directly inside the double-quoted argument instead of passing it as a properly escaped/quoted value — any shell metacharacters in that text (`, $( ), embedded ", ;, !) are interpreted by the shell rather than passed through literally. Double quotes in bash do not neutralize command substitution or embedded quotes.

The realistic exploit path is narrow (the text is normally Anthropic's own system-generated banner), but the skill explicitly targets the orchestration case — inspecting a worker/subagent's "real output" and acting on it, per keep-going's own doctrine. If that output is ever relayed through content the worker processed (logs, transcripts, text influenced by external/adversarial input), a string crafted to look like a reset clause but carrying shell metacharacters becomes command injection the moment an orchestrating agent copies it into the shown command form.

The script already supports a safe alternative — reading from stdin when no positional argument is given (check-usage-limit-reset.py#L60-L62). The doc example should pipe the text via stdin instead:

printf '%s' "$LIMIT_MESSAGE" | python3 "${CLAUDE_PLUGIN_ROOT}/skills/keep-going/scripts/check-usage-limit-reset.py"

Fix this →

Considered, not a real issue: zone = ZoneInfo(tz_name) at check-usage-limit-reset.py#L43 feeds a regex-captured, attacker-influenceable group into a timezone-key lookup. This looks like a possible path-traversal-into-tzdata read, but CPython's zoneinfo module validates keys and rejects .. components, absolute paths, and NUL bytes before any file access — a malformed key just raises and is caught by the surrounding except Exception, exiting 2. I also checked the regex (#L18-L24) for ReDoS: no nested/overlapping quantifiers, so no catastrophic-backtracking risk even on adversarial input.

No other issues found: the script does no file I/O, network calls, eval/exec, or shell execution of its own; it handles no secrets/tokens/credentials; the .test.py invokes the script via subprocess.run with an argument list (no shell=True), which is safe; plugin.json/CHANGELOG.md changes are metadata-only.

Note: two P2 findings from the Codex review on this PR (midnight-rollover date handling, timezone-less input assumed as UTC) are logic/correctness bugs in the reset calculation, not security vulnerabilities (no authorization bypass, injection, or data exposure) — out of scope for this security-only pass.

[claude]

Claude finished @kyle-sexton's task in 4m 0s —— View job

---

Code Review

• Gather context (PR diff, changed files, CLAUDE.md/REVIEW.md)
• Review changed files for correctness, error handling, test coverage, maintainability
• Post findings as inline comments + summary

Reviewed all 5 changed files against origin/main (via gh pr diff 2462). Scope: correctness, error handling, tests, maintainability — per this repo's REVIEW.md, security findings (e.g. the shell-interpolation risk in the SKILL.md example already raised by the claude-security-review lane above) are that lane's exclusive territory and are intentionally omitted here.

Findings (posted as inline comments)

1. 🔴 Important — midnight rollover not handled: current.replace(...) always pins the parsed reset time onto today's date, so an overnight observation (e.g. 23:00, "resets 2:30am") resolves to a reset already in the past today and reports "lifted" ~3.5 hours early. Traced through the actual regex/parse logic by hand (could not execute the script in this sandbox — see note below).
2. 🔴 Important — timezone-less reset silently assumed UTC: when no (tz) group is present, the script defaults to UTC — but the doc's own example (SKILL.md#L138) is exactly the timezone-less form (resets 3:45pm), which is far more likely to be local/session time than UTC. This makes the ambiguous case the common case, not an edge case.
3. 🔴 Important — test not wired into CI: scripts/run-plugin-tests.sh only discovers *.test.sh files. This repo's established convention for a Python test (see scripts/check-manifest-duplicate-keys.test.sh, scripts/check-contract-clause-coverage.test.sh) is a co-located .test.sh wrapper that execs the .py test directly — this PR's test has no such wrapper, so it doesn't run in CI and findings chore: initialize marketplace scaffold #1/fix: correct plugin-system inaccuracies and harden references #2 shipped without a safety net catching them.

Findings 1–3 line up with (and were independently verified beyond) the chatgpt-codex-connector review comments on this PR — I traced the regex/replace logic and the CI test-discovery script (run-plugin-tests.sh) by hand to confirm each one against this repo's actual code, rather than taking the bot's word for it.

No other issues found. plugin.json version bump and CHANGELOG.md entry are consistent with the change; the test file's subprocess.run usage (no shell=True, argument list) is sound; the except Exception → exit 2 boundary is a reasonable CLI contract.

Note on verification: I was unable to execute check-usage-limit-reset.py or its test suite directly — python3 invocations required interactive approval that wasn't available in this run. Findings above are based on manual code tracing, not execution; the PR author's own Test plan (3 passing cases) is author-claimed and unverified by me. The two bugs found here (overnight rollover, timezone-less input) are not covered by the 3 existing test cases, which is consistent with them shipping despite the stated test plan passing.

[github-actions]

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

[claude]

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

---

Security Review

• Gather context (PR diff, changed files)
• Review changed files for security issues
• Report findings

Reviewed all 5 files changed in this PR (git diff origin/main...HEAD at 87a95e2): the new check-usage-limit-reset.py script, its .test.py, the SKILL.md doc update, and the metadata-only plugin.json/CHANGELOG.md changes.

Findings

IMPORTANT — documented invocation interpolates untrusted text into a shell command (confidence: moderate)

SKILL.md#L136-L138 still documents:

python3 "${CLAUDE_PLUGIN_ROOT}/skills/keep-going/scripts/check-usage-limit-reset.py" "<limit message text>"

<limit message text> is meant to be substituted with a worker/subagent's real limit-message text and run via the Bash tool. If an agent follows the example literally — dropping the message text directly inside the double-quoted argument instead of passing it as a properly escaped/quoted value — any shell metacharacters in that text (`, $( ), embedded ", ;, !) are interpreted by the shell rather than passed through literally. Double quotes in bash do not neutralize command substitution or embedded quotes.

The realistic exploit path is narrow (the text is normally Anthropic's own system-generated banner), but the skill explicitly targets the orchestration case — inspecting a worker/subagent's "real output" and acting on it, per keep-going's own doctrine of reading real output rather than trusting a summary. If that output is ever relayed through content the worker processed (logs, transcripts, text influenced by external/adversarial input the subagent handled), a string crafted to look like a reset clause but carrying shell metacharacters becomes command injection the moment an orchestrating agent copies it into the shown command form.

The script already supports a safe alternative — reading from stdin when no positional argument is given (check-usage-limit-reset.py#L60-L62). The doc example should pipe the text via stdin instead:

printf '%s' "$LIMIT_MESSAGE" | python3 "${CLAUDE_PLUGIN_ROOT}/skills/keep-going/scripts/check-usage-limit-reset.py"

Fix this →

Considered, not a real issue: zone = ZoneInfo(tz_name) at check-usage-limit-reset.py#L43 feeds a regex-captured, attacker-influenceable group into a timezone-key lookup. This looks like a possible path-traversal-into-tzdata read, but CPython's zoneinfo module validates keys (rejecting .. components, absolute paths, and NUL bytes) before any file access — a malformed key just raises and is caught by the surrounding except Exception, exiting 2. The capture regex (#L22-L28) also has no nested/overlapping quantifiers, so there's no ReDoS risk on adversarial input.

No other issues found: the script does no file I/O, network calls, eval/exec, or shell execution of its own; it handles no secrets/tokens/credentials; the .test.py invokes the script via subprocess.run with an argument list (no shell=True), which is safe; plugin.json/CHANGELOG.md changes are metadata-only (version bump + changelog entry, no permission or config changes).

Note: the two P2 findings from the Codex review on this PR (midnight-rollover date handling, timezone-less input assumed as UTC) are logic/correctness bugs in the reset calculation, not security vulnerabilities (no authorization bypass, injection, or data exposure) — out of scope for this security-only pass.
· branch cursor/keep-going-reset-check-63e1

[claude]

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

---

Security Review

• Gather context (PR diff, changed files)
• Review changed files for security issues
• Report findings

Reviewed all 6 files changed in this PR (git diff against the branch point, since origin/main...HEAD has no merge base under this shallow checkout) at HEAD 688be81: the correctness-fix commits (3b0f068, 688be81) landed since the last completed security pass at 87a95e2, so this is a fresh review rather than a repeat.

Findings

IMPORTANT — documented invocation interpolates untrusted text into a shell command (posted inline on SKILL.md#L136-L138)

The documented example still substitutes <limit message text> directly inside a double-quoted shell argument:

python3 "${CLAUDE_PLUGIN_ROOT}/skills/keep-going/scripts/check-usage-limit-reset.py" "<limit message text>"

Double quotes don't neutralize `, $( ), embedded ", ;, or !. The skill's own doctrine is to inspect a worker/subagent's real output during orchestration — if that text is ever relayed through content the worker processed and contains a string crafted to look like a reset clause but carrying shell metacharacters, an agent that copies it into this command form gets command injection. The script already supports stdin when no positional arg is given (check-usage-limit-reset.py#L72-L74); the inline comment includes a committable suggestion piping via stdin instead. This has been raised by three prior review passes on this PR and remains unfixed.

Considered, not a real issue:

• zone = ZoneInfo(tz_raw.strip()) (check-usage-limit-reset.py#L44) — a regex-captured, attacker-influenceable group used as a timezone key. CPython's zoneinfo validates keys (rejects .., absolute paths, NUL bytes) before any file access; a malformed key just raises and is caught by the surrounding except Exception, exiting 2. No path traversal.
• RESET_RE (#L22-L28) has no nested/overlapping quantifiers — no ReDoS risk.
• The new local-timezone fallback logic (#L45-L49) and midnight-rollover fix (#L52-L56) are correctness changes to parse_reset, not security-relevant — they don't introduce new untrusted-input handling.
• check-usage-limit-reset.test.sh execs the test module via a BASH_SOURCE-derived path with no untrusted interpolation — safe.
• plugin.json/CHANGELOG.md changes are metadata-only (version bump + changelog entry, no permission or config changes).

No other issues found: the script performs no file I/O, network calls, eval/exec, or shell execution of its own; it handles no secrets/tokens/credentials.
· branch cursor/keep-going-reset-check-63e1

[github-actions]

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

[claude]

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

---

Security Review

• Gather context (PR diff since last reviewed head)
• Check skip-gate conditions
• Review changed files for security issues
• Report findings

Skip gate applied — no security-relevant change since the last completed review.

The last completed security review on this PR ran at 688be81 and reported one unresolved finding (shell-interpolation risk in the SKILL.md example). Diffing that reviewed tree against the current head (0aebfd3) shows the only change under plugins/session-flow/ is a file-mode fix:

diff --git a/plugins/session-flow/skills/keep-going/scripts/check-usage-limit-reset.test.py
old mode 100644
new mode 100755
diff --git a/plugins/session-flow/skills/keep-going/scripts/check-usage-limit-reset.test.sh
old mode 100644
new mode 100755

That's setting the executable bit on the two test scripts — no content change, no new security-relevant surface. Per this lane's skip gate ("this head already has a successful security review that still applies"), I'm not re-running a full pass or re-posting duplicate findings.

For reference, the still-open finding from the 688be81 review remains applicable and unresolved as of this head:

• IMPORTANT — SKILL.md#L136-L138 documents interpolating <limit message text> directly into a double-quoted shell argument. The script already supports reading from stdin when no positional argument is given (check-usage-limit-reset.py#L72-L74); recommend piping via stdin instead so untrusted text is never assembled into a shell command line:

  printf '%s' "$LIMIT_MESSAGE" | python3 "${CLAUDE_PLUGIN_ROOT}/skills/keep-going/scripts/check-usage-limit-reset.py"

No new findings on this head.
· branch cursor/keep-going-reset-check-63e1

[claude]

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

---

Code Review

• Gather context (PR diff, changed files)
• Check skip gate

Skip gate triggered: this PR is already merged (PR State: MERGED). Per /review:code-review's skip gate ("PR is closed or not open"), no review is performed and nothing further is posted.