Raise test token counts from 170K/180K to 800K/810K so compaction triggers at 78% of 1M instead of 78% of 200K, matching the actual GA context limit for 4.6 models.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Address Oracle review feedback: refactor 4 aliased mutations via argsObject
in plugin/tool-execute-before.ts and 1 via toolOutput in atlas/tool-execute-before.ts.
Strengthen audit test regex to catch Output.args mutations regardless of the
variable name prefix (toolOutput, argsObject aliases).
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Add replace-tool-args.audit.test.ts that scans src/**/*.ts for direct
output.args property assignments and Object.assign(output.args, ...) outside
the helper. Also fix the 9th mutation site discovered by the audit in
compaction-todo-preserver/hook.ts.
Add replace-tool-args.test.ts with 12 regression tests covering both mutable
and Object.freeze'd output.args scenarios for all hook patterns.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Replace output.args.command and output.args.prompt direct assignments
with replaceToolArgs() in non-interactive-env, prometheus-md-only,
and sisyphus-junior-notepad hooks.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Add 4 regression tests covering the pre-set effort path:
- pre-set effort=max + variant=max + github-copilot Opus -> clamped to high
- pre-set effort=max + variant=high + github-copilot Opus -> clamped to high (cubic violation case from PR #3608)
- pre-set effort=max + non-constrained Opus -> max preserved (no regression)
- pre-set effort=high + github-copilot Opus -> high preserved (don't overwrite valid pre-set)
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Move the pre-set effort handling BEFORE the message.variant !== "max" early-return so that output.options.effort="max" set via session params or model-requirements fallback chains is always clamped to "high" on constrained providers (github-copilot, Anthropic OAuth), even when message.variant is not "max".
This addresses the cubic violation from PR #3608 where the clamp block was gated behind the variant-based return.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Replace output.args.todos = parsed and Object.assign(output.args, result.modifiedInput)
with replaceToolArgs() calls that create a shallow clone instead of
mutating the potentially-frozen args object.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Addresses cubic-dev-ai P1 finding on #4113 (#4113 review).
The original chain `extractErrorName(error)?.toLowerCase().replace(...)`
is semantically safe — JavaScript optional chaining short-circuits the
ENTIRE access chain when the head returns null/undefined, so when
`extractErrorName` returns undefined the whole expression evaluates to
undefined without ever reaching `.replace()`. Verified empirically via
`const x = undefined; x?.toLowerCase().replace(/_/g, "")` returns
undefined with no crash.
Applying the suggested defensive `?.` before `.replace` anyway, since
it is semantically a no-op and explicit chaining at each hop is easier
for static analyzers to reason about.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes#3607.
## Root cause
On Windows, `detectCommandShellType()` fell through to `detectShellType()`
for two common environments and incorrectly returned `"unix"`:
1. **SHELL points at a Unix-shaped path** (e.g. Git Bash sets
`SHELL=/usr/bin/bash` on a fresh Windows install).
The `detectWindowsShellType(process.env.SHELL)` probe didn't recognize
`bash` as a Windows shell, so the function fell through and
`detectShellType()` returned `"unix"`.
2. **MSYSTEM is set but SHELL is not** (Git Bash leaves MSYSTEM permanently
set system-wide even when the active shell is PowerShell).
The fall-through path returned `"unix"` via the MSYSTEM check.
In both cases, the hook then prepended `export KEY=val;` to git commands,
which PowerShell rejects with:
`export : 无法将"export"项识别为 cmdlet...`
OpenCode on Windows runs the bash tool through a Windows shell
(PowerShell by default, cmd as the user-overridable fallback), regardless
of MSYSTEM or a Unix-shaped SHELL set by Git Bash — so the env prefix
must use Windows-compatible syntax.
## Fix
`detectCommandShellType()` now short-circuits on `process.platform === "win32"`:
- If `SHELL` points at a recognized Windows shell (`cmd.exe`, `powershell.exe`,
`pwsh.exe`), return that.
- If `SHELL` and `MSYSTEM` are both unset, fall back to `ComSpec` then to cmd.
- Otherwise, default to PowerShell — matching what OpenCode actually spawns.
`detectShellType()` is unchanged; other callers (including non-Windows
platforms) are unaffected.
## Test changes
Three pre-existing tests encoded the buggy behavior as expected behavior
and have been updated to assert the new PowerShell syntax with a
`(#3607)` marker and a comment explaining why a Unix-shaped SHELL on
win32 must still resolve to PowerShell. WSL is not affected because in
WSL `process.platform === "linux"`, not `"win32"`.
- `src/hooks/non-interactive-env/`: 24 tests pass / 0 fail
- `bunx tsc --noEmit`: clean
## Note on issue thread
The sisyphus-bot triage comment on #3607 framed this as a policy choice
between (A) forcing Windows env-prefix syntax and (B) resolving against
the OpenCode-configured shell. This PR implements option (A) as the
minimal surgical fix; option (B) remains a follow-up.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses cubic-dev-ai bot review on #4113 (P2): the RESOURCE_EXHAUSTED
and snake_case insufficient_quota fixtures contained quota-shaped
messages that already matched pre-existing message regexes, so the tests
passed even without the new errorName allow-list entry and the
underscore normalization respectively.
Replace both fixture messages with a generic "Request failed." so the
only path to a `quota_exceeded` classification is via the new code:
- RESOURCE_EXHAUSTED: only the new `errorName?.includes("resourceexhausted")`
match on the normalized name can fire.
- insufficient_quota (snake_case): only the new underscore-stripping
normalization can route the name to `insufficientquota` and match the
existing allow-list entry.
The third new test (Google ResourceExhausted message-only) is unchanged
because its message uniquely matches only the new
`/resource.?exhausted/i` pattern and not any existing quota regex.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes#3937.
Adds three small classification gaps to `classifyErrorType` so that
quota-exhaustion errors from a wider range of providers trigger
configured fallback chains instead of looping retry attempts:
- Normalize error names by stripping `_` and `-` so snake_case /
SCREAMING_SNAKE_CASE provider names (`insufficient_quota`,
`RESOURCE_EXHAUSTED`, `rate_limit_exceeded`) match the existing
alphanumeric `.includes()` checks.
- Add `resourceexhausted` to the quota error-name allow-list to cover
Google Generative AI's gRPC code 8 / `ResourceExhausted` surface.
- Add `/resource.?exhausted/i` to the quota message-pattern list so the
same error surface is caught when the provider only sets a generic
error name but puts the signal in the message.
Three new regression tests in
`quota-error-classifier.regression.test.ts` cover:
- Google `RESOURCE_EXHAUSTED` (gRPC error name + quota-shaped message)
- Google `ResourceExhausted` message form without HTTP status
- OpenAI snake_case `insufficient_quota` error name
No existing tests were touched; the underscore normalization preserves
all existing `.includes()` matches by rewriting the one underscore-bearing
literal (`ai_loadapikeyerror` → `ailoadapikeyerror`) so previously
matched names still resolve.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Guard command.execute.before against injecting when parts already
contain auto-slash-command tags, preventing duplication when both
chat.message and command.execute.before fire for the same slash command.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Unresolved git merge conflict markers (<<<<<<<, =======, >>>>>>>) in
TypeScript source files break parsing and can cause the plugin to fail
at runtime or tests to hang with cryptic errors. This guard scans all
.ts/.tsx/.json files under src/ and fails the test suite if any
conflict markers are found.
Closes #debugging-hang-investigation
- Replace the simple scheduledDelays array with an activeTimers Map so
that clearTimeout removes timers from the tracked set.
- This prevents false positives when internal withDispatchTimeout calls
setTimeout for safety timeouts that are immediately cancelled.
- Keeps the test intent unchanged: only genuinely scheduled retries are
counted as delayed duplicate retries.
- Wrap isSessionActive in withDispatchTimeout (capped at 5s) so a
stuck OpenCode SDK status() call cannot block internal prompts forever.
- Catch the timeout and treat session as inactive so the prompt can
proceed rather than hanging indefinitely.
- Add regression test: session.status that never resolves now times out
and allows dispatch instead of hanging the test (and production).
Refs: AGENTS.md internal-message-injection safety note
The SDK client disables fetch timeout (req.timeout = false). When the
opencode server is slow or unresponsive, client.session.status() hangs
forever, blocking the entire dispatchToHooks chain (sequential await)
and deadlocking the plugin.
Add a 5s timeout wrapper around the status call in isSessionActive().
On timeout, the catch block returns false (treat as inactive), letting
the caller proceed instead of hanging indefinitely.
The timeout parameter is exposed for testing to avoid 5s test delays.
Refs: .debugging/status-timeout-hang.md
The rule scan cache stored a path[] keyed by (projectRoot|startDir|
skipClaudeUserRules), so two issues stacked up on every tracked tool
call:
- Cache hits still ran safeRealpathSync(realpathSync) and re-derived
isGlobal / distance / isSingleFile for every cached path. That is a
per-candidate sync syscall plus repeated string-prefix walks.
- Sibling files in the same project landed under different startDir
keys, so the entire walk-and-recursive-scan chain repeated even
though every ancestor rule directory was identical.
Store the full RuleFileCandidate[] in the per-call cache so a cache hit
returns immediately with no realpath syscall. Add a separate per-
directory scan cache (getDirScan/setDirScan) keyed by absolute rule
directory path, so two sibling files reuse the same readdir + realpath
work for every shared ancestor.
Microbench (200 files / 20 modules / cached session):
- single sweep: 41.8ms -> 2.5ms (16x)
- 3-pass replay: 88.6ms -> 3.2ms (28x)
Pin the new invariants with two new tests:
- 'does not re-resolve symlinked rule path on cache hit' via a
retargeted directory symlink.
- 'reuses ancestor directory scan for sibling files in the same
project' by deleting the source rule file between the two calls.
findProjectRoot was keyed by exact startPath, so sibling files in the
same project repeated the entire upward marker walk. The walk does one
existsSync per marker per ancestor directory, which adds up on every
read/write/edit/multiedit tool call.
Track every directory visited during the walk and seed the cache with
the resolved root for each of them. Subsequent lookups for any
descendant short-circuit to the cached ancestor without re-running
marker probes. Cache invalidation still happens on session.deleted /
session.compacted, so production semantics are unchanged.
Pin the new contract via a sibling-startpath test, and make the
existing finder.test.ts beforeEach explicit about cache state so the
more aggressive cache does not leak between tests.
When the first prompt fails before any durable user message persists,
runtime fallback retry was rebuilding the request from parts alone and
losing the delegated agent system prompt and tool gates. Now it threads
bootstrap.system and bootstrap.tools into the retry body alongside the
captured retry parts, so the retried prompt keeps the same scope as the
initial delegate launch.
- script/run-ci-tests.ts: CI test sharding and isolation logic
- script/run-ci-tests.test.ts: tests for CI test target selection
- src/features/background-agent/session-route.ts: session prompt routing for background agents
- src/hooks/interactive-bash-session/parser.ts: interactive bash output parser
- src/hooks/ralph-loop/completion-promise-detector-test-input.ts: test fixture for completion promise detection
Adds a new `notepad-write-guard` hook that intercepts Write tool calls
whose target path matches `**/.sisyphus/notepads/**` and throws an
actionable error instead of allowing the write to proceed.
Without this guard, an agent that hits an Edit hash-mismatch failure
could silently fall back to Write, destroying the entire history of an
append-only notepad file (decisions.md, issues.md, etc.). The file
carries an explicit "NEVER overwrite" warning that the agent ignores
under context pressure.
The guard is path-based so it works regardless of plan name or nesting
depth. Non-notepad `.sisyphus/**` paths (e.g. plan files) are
unaffected. The hook is wired into `create-tool-guard-hooks` under the
hook name `notepad-write-guard` and follows the same safeCreateHook +
HookName schema pattern as every other tool-guard hook.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two P0 fixes for the assistant loop that repeats its final summary 3-5 times
after all todos are marked completed before stagnation detection finally halts it.
P0.1 — session-level stop flag: when handleSessionIdle detects incompleteCount===0
it now sets state.allTodosCompletedAt. Subsequent idle events for the same session
bail out immediately at the top of the function before any HTTP fetch or injection
logic runs, preventing the re-entry loop regardless of todo-fetch caching latency.
The flag is cleared by resetContinuationProgress so sessions that receive new todos
after completion resume enforcement normally.
P0.2 — snapshot comparison scope: getTodoSnapshot now only serialises the
{id → status} mapping (sorted by key). Content and priority changes are excluded
from the comparison. Previously those fields were included, causing hasTodoSnapshotChanged
to return true whenever the LLM re-wrote todo text with identical status — which
reported progressSource="todo" and reset stagnationCount to 0, preventing
MAX_STAGNATION_COUNT=3 from ever being reached.
P1 fixes (CONTINUATION_PROMPT adversarial wording, 10 s completion grace period)
are deferred to a follow-up PR as noted in the issue.
YeonGyu-Kim