fix: audit improvements — grace window, injection escape, DRY errors, new options, tests, CI matrix#4
Merged
Merged
Conversation
… escape, new options
- noProgressTurnsBeforePause (default 2): no-progress counter now accumulates
across consecutive low-output idle cycles before stopping; counter resets when
a high-output turn is seen; managed entirely in the idle handler rather than
split across message.updated
- maxRecentMessages (default 12): messages query limit promoted from hardcoded
literal to a configurable normalizeOptions option
- applyPromptFailure: extracted shared helper used by both the response.error
branch and the catch block, eliminating duplicated failure-tracking logic
- escapeGoalText: broadened from escaping only </goal_objective> to escaping
all XML closing tags (replaceAll("</" → "<\/")) so user goal text cannot
break any structural tag in buildContinueMessage or the system transform
- testInternals: added applyPromptFailure and logPluginError exports
- tests: 14 new cases covering buildLimitWarning, outputTokensForMessage,
budgetWrapupNeeded, getSessionID, escapeGoalText (all structural tags),
stopReason, logPluginError console fallback, applyPromptFailure,
extractBlockedReason first-line edge case, normalizeOptions boundary inputs
(0/negative/NaN/null/undefined, budgetWrapupRatio at 0 and 1), grace window
integration (N-1 stalls allowed, counter reset on high-output turn)
- three existing no-progress tests updated with noProgressTurnsBeforePause: 1
to preserve their one-strike semantics under the new default
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ci: test matrix now covers Node 18, 20, and 22 to match the declared
engines >=18 range
docs: README safety limits table corrected (no-progress pause now shows
"2 consecutive turns" to match the grace window default); plugin-level
config example expanded from 7 to 12 options, adding the previously
undocumented noProgressTurnsBeforePause, maxRecentMessages,
warnTurnsRemaining, warnDurationMsRemaining, and warnTokensRemaining
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…and new helper tests main (PR #3) already implemented noProgressTurnsBeforePause, maxRecentMessages, the grace window, CI matrix, and extensive new tests. This merge keeps main's version for all those areas and contributes two remaining additions: - escapeGoalText: broadened to escape all XML closing tags (replaceAll "</" → "<\/") so user-supplied goal text cannot break any structural tag in buildContinueMessage; main's version still only escaped </goal_objective> - tests: 7 new unit tests for outputTokensForMessage, budgetWrapupNeeded, getSessionID, stopReason, normalizeOptions boundary inputs (zero/negative/NaN/null, budgetWrapupRatio at 0 and 1), and escapeGoalText covering all structural tags Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Code fixes (
src/goal-plugin.js)noProgressTurnsBeforePause(default 2): low-output idle cycles now accumulate a counter before stopping; the counter resets when a high-output turn is seen; removed the split reset that lived inmessage.updatedso the counter is managed in one place and the/goal statusvalue is meaningfulmaxRecentMessages(default 12): messages query limit promoted from a hardcoded literal to anormalizeOptionsoptionapplyPromptFailure: extracted shared helper used by both theresponse.errorbranch and thecatchblock — was duplicated logic with identical threshold/stop logic in two placesescapeGoalText: broadened from escaping only</goal_objective>to escaping all</sequences, so user-supplied goal text cannot break any structural XML tag inbuildContinueMessageor the system transform hookTests (
test/goal-plugin.test.js)buildLimitWarning,outputTokensForMessage,budgetWrapupNeeded,getSessionID,escapeGoalText(all structural tags),stopReason,logPluginErrorconsole fallback,applyPromptFailure,extractBlockedReasonfirst-line edge case,normalizeOptionsboundary inputs, grace window integration (N-1 stalls allowed, counter reset on high-output turn)noProgressTurnsBeforePause: 1to preserve their one-strike semantics under the new defaultCI (
.github/workflows/ci.yml)engines: >=18Docs (
README.md)noProgressTurnsBeforePause,maxRecentMessages,warnTurnsRemaining,warnDurationMsRemaining,warnTokensRemainingWhy it changed
Found during an independent audit of the codebase. The no-progress grace window is the most behaviour-visible change — previously one low-output turn stopped the goal; with the default of 2 the plugin tolerates a single brief turn before pausing.
Checks run
npm run check(syntax + 47 tests, all pass)npm run smokenpm run pack:check