mirror of
https://github.com/Gitlawb/openclaude.git
synced 2026-08-24 10:14:19 -05:00
main
16
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  NikandGitHub
|
2259c809f7 |
fix(shim): don't infer Z.AI tool_stream for non-catalog GLM gateways (#1908)
The name-based shim matcher inferred the full Z.AI GLM contract — including enableToolStreaming — for any glm-<n> model without a catalog entry. tool_stream is a Z.AI-proprietary streaming extension, so serving GLM through an arbitrary OpenAI-compatible gateway (e.g. NVIDIA NIM, integrate.api.nvidia.com) made every request fail immediately with 400 Unsupported parameter(s): tool_stream. Only a catalog entry may opt into tool_stream (Z.AI-contract gateways set it explicitly via transportOverrides.openaiShim). Inferred GLM routes keep the reasoning-shaping fields, which any GLM endpoint benefits from, but no longer send tool_stream; without it tool calls are simply not streamed. |
||
|
NikandGitHub
|
2f98208eaf |
fix(command-semantics): treat linter exit 1 as violations, not an error (#1846)
* fix(command-semantics): treat linter exit 1 as violations, not an error Linters and formatters use exit code 1 to mean "violations found", not a crash. commandSemantics fell back to DEFAULT_SEMANTIC for ruff/eslint, so a run that merely reported lint findings was flagged isError: true and the model retried the same command up to 3 times before giving up (observed on Windows with `uvx ruff check --fix`). Add a LINT_SEMANTIC (exit 1 = violations found, 2+ = real error, mirroring the existing grep/diff pattern) for ruff and eslint, in both the Bash and PowerShell tables. Wrapper runners (uvx, npx) inherit the wrapped tool's semantics only when it resolves to a recognized command — an unrecognized wrapped tool still falls back to the default, so `uvx <arbitrary>` is not blanket-treated as non-error. Adds coverage for ruff/eslint exit codes and the uvx/npx unwrap (including the unknown-wrapper fallback) in both Bash and PowerShell suites. * fix(command-semantics): normalize path-prefixed and quoted Bash linters extractBaseCommand returned the raw first token, so path-prefixed or quoted invocations (./node_modules/.bin/eslint, "ruff", /usr/bin/uvx ruff, npx ./node_modules/.bin/eslint) fell through to default exit-code semantics and a linter's exit 1 was mis-reported as an error. Normalize the base and wrapped command names (strip surrounding quotes and any path prefix) like the PowerShell implementation does, and match the wrapper by its normalized name. Adds regression coverage for path-prefixed and quoted linter/wrapper commands. * fix(command-semantics): normalize Windows .cmd/.bat/.ps1 shims on PowerShell path extractBaseCommand only stripped .exe, so npm-installed tools and wrappers invoked via their Windows .cmd shims (eslint.cmd, npx.cmd, .\node_modules\.bin\eslint.cmd) fell back to DEFAULT_SEMANTIC and reported exit 1 as an error, regressing the lint-exit-code fix on the PowerShell path. Broaden the suffix strip to the common PATHEXT executable/shim extensions (.exe/.cmd/.bat/.ps1) so direct and wrapped .cmd invocations resolve to the tool/wrapper name. Adds regression coverage for direct, path-prefixed, and wrapped .cmd forms. |
||
|
5105dff5f4 |
fix(config): recover from a healthy backup when the global config is corrupt (#1819)
* fix(config): recover from a healthy backup when the global config is corrupt A present-but-corrupt ~/.openclaude.json took getConfig down the ConfigParseError path, which reset to defaults (silently discarding the user's settings) or re-threw, even though healthy timestamped backups exist in ~/.claude/backups. getConfig only consulted a backup on ENOENT, and then only to print a manual 'cp' hint. Now a corrupt parse first tries to recover the most recent backup that still parses (merged over defaults) before doing anything destructive, so a one-off bad write no longer wipes config or crashes startup. Falls back to the existing defaults path when no backup is usable. Adds direct unit tests for the recovery helper. Closes #1807 * fix(config): iterate backups on recovery and skip rotating a corrupt file Address review on #1819: - recoverConfigFromBackup only tried the single newest backup, so a corrupt newest .backup left startup on the corrupted-config fallback even when an older healthy snapshot existed. Add listBackupsNewestFirst and iterate candidates newest-first, recovering from the first that parses. Added a newest-corrupt/older-healthy regression test. - saveGlobalConfig copied the live file into the backup rotation before writing. After an in-memory recovery the on-disk file is still corrupt, so that copy poisoned the rotation with the same bad content. Only rotate the current file into backups when it parses. * fix(config): never prune backups while the live config is corrupt Address review: the MAX_BACKUPS cleanup ran unconditionally, so when the live config is corrupt and only an older snapshot is healthy, startup's runMigrations() -> saveGlobalConfig() could unlink that last usable backup before the recovered config is durably written (#1807). Extract the prune decision into a pure, exported selectBackupsToPrune() that returns [] whenever the live config fails to parse, and gate the cleanup loop on it. Hoist the parse check so both the backup-copy and the prune paths share it. Add production-path coverage: getConfig recovery via _getConfigForTesting (corrupt live config -> healthy backup, and newest-corrupt -> older-healthy), plus direct selectBackupsToPrune tests (corrupt -> prunes nothing; healthy -> keeps newest maxBackups). * fix(config): skip valid-but-non-object backups during recovery recoverConfigFromBackup() returned the first backup that parsed, even when the parsed value was not a config object. A newest backup holding valid JSON like null, [], or a bare string spread into bare defaults or index/char keys and stopped, discarding the older healthy snapshots the #1807 recovery is meant to fall through to. Guard that parsedBackup is a non-null, non-array object before returning; otherwise continue to the next older backup. Adds a regression test: a valid-but-unusable newest backup (null) is skipped in favor of an older healthy one. * fix(config): recover global config from legacy backups and self-heal corrupt startup Two gaps in the #1807 recovery path surfaced in review: - listBackupsNewestFirst filtered only on the active basename, so once the global config is .openclaude.json it never tried the pre-rename .claude.json.backup.* snapshots that #1807 reports as the only surviving clean source. Recover the global config from the legacy basename too, and order backups by their .backup.<ts> timestamp so current and legacy snapshots interleave by recency instead of grouping by filename. - enableConfigs validated the global config with the throwing mode, so a corrupt config with no usable backup rethrew ConfigParseError through startup and locked users out on every launch. Drop the throwing mode so the corrupt-file/default fallback runs and startup self-heals. Adds regression coverage: legacy-basename recovery plus a scoping guard, and an enableConfigs no-crash test for the unrecoverable corrupt-config case. * test(config): re-register real env module after startup validation test The enableConfigs (#1807) regression test installs mock.module('./env.js', ...) so getGlobalClaudeFile() points at the virtual config path. Bun's mock.module() is process-global and is not undone by mock.restore(), so the teardown that only reset the fs implementation left later same-process tests importing ./env.js on the virtual path. Capture the real env module once in beforeAll and re-register it in afterEach alongside the fs reset, matching the established restore pattern in user.test.ts/effort.test.ts. --------- Co-authored-by: Pablosinyores <nikhilbajaj0182@gmail.com> |
||
|
NikandGitHub
|
f7d472e826 |
fix(provider): add Use Anthropic option to switch back from third-party profiles (#1429)
* fix(provider): add Use Anthropic option to switch back from third-party profiles The /provider menu offered no way back to built-in Anthropic once any third-party provider profile was active: getActiveProviderProfile falls back to profiles[0] when activeProviderProfileId is unset, so clearing the active id still re-selected a saved profile. Users had to hand-edit ~/.openclaude.json and restart. Add an explicit ANTHROPIC_DEFAULT_PROFILE_ID sentinel that getActive- ProviderProfile resolves to undefined (Anthropic) instead of profiles[0], and a clearActiveProviderProfile() that records the sentinel, clears the managed provider env in-session, and removes the startup profile mirror. Surface it as a 'Use Anthropic (built-in)' choice in /provider, shown whenever the current provider is not Anthropic. Saved profiles are kept for re-selection; the switch takes effect without a restart. Fixes #1426 * fix(provider): wire Use Anthropic into live ProviderManager and keep the sentinel Addresses review feedback on #1429: - The "Use Anthropic (built-in)" option now lives in the live ProviderManager "Set active provider" flow (the wizard path is test-only). It is offered only when a third-party profile or GitHub Models is currently active, and routes through clearActiveProviderProfile() + resets the session model to the built-in Anthropic default so the switch takes effect without a restart. - Teach the add/update/delete fallbacks that ANTHROPIC_DEFAULT_PROFILE_ID is a valid active state. Previously, adding a profile with makeActive:false, updating any profile, or deleting an inactive profile while on built-in Anthropic would silently reactivate profiles[0], switching the user back to a third-party provider. The delete path also no longer resolves the sentinel to profiles[0] when re-applying env. Added regression tests covering the add/update/delete sentinel-preservation paths. * fix(provider): clear startup provider overrides when switching back to Anthropic The /provider Anthropic activation branch cleared the managed session env and the startup profile file but left the startup provider override in user settings intact, so a restart would replay the third-party provider. Clear it the same way the saved-profile and GitHub paths do, surfacing any cleanup failure as a warning. Also assert the managed provider env is removed in the clearActiveProviderProfile session-env test. * fix(provider): clear startup overrides from /provider Anthropic branch; honor makeActive:false for implicit-active profiles Addresses two review findings on #1429: - The /provider 'Use Anthropic (built-in)' branch only called clearActiveProviderProfile(), so settings-backed startup overrides (CLAUDE_CODE_USE_OPENAI / OPENAI_BASE_URL / API key) survived and re-selected the third-party provider on restart. It now also calls clearStartupProviderOverrides() and surfaces a cleanup warning in the onDone message instead of reporting unconditional success, mirroring the ProviderManager Anthropic branch. - addProviderProfile(makeActive:false) still promoted the new profile when activeProviderProfileId was unset but saved profiles existed, because getActiveProviderProfile() implicitly resolves that state to the first profile while the old ternary treated !currentActive as 'no active'. Resolve the effective active state (sentinel, explicit id, or implicit first profile) before deciding, so makeActive:false never silently switches the active provider. Adds a regression test for the implicit-first-profile case (fails on the old ternary). * fix(provider): honor stale active id and clear hydrated GitHub token on Anthropic switch Addresses two findings on the switch-back-to-Anthropic path (#1426). P2 — stale active profile id (providerProfiles.ts): addProviderProfile's makeActive:false guard only preserved the implicit-first-profile case when activeProviderProfileId was unset. If the config carried an id for a deleted/missing profile, the guard treated it as "no active" and promoted the newly added profile, ignoring makeActive:false — even though getActiveProviderProfile() resolves a stale id to profiles[0]. Resolve an effectiveActiveId the same way getActiveProviderProfile does (sentinel -> built-in Anthropic, valid id -> that profile, stale/unset id with profiles -> implicit first, none -> nothing active) and keep it when makeActive:false. +regression test for the stale-id case (fails on the old guard). P2 — hydrated GitHub token leak (ProviderManager.tsx): Selecting "Use Anthropic (built-in)" while GitHub Models was active called only clearActiveProviderProfile(), which clears managed flags but leaves a GITHUB_TOKEN hydrated from secure storage (and its marker) in the session. Mirror the GitHub delete path: new clearHydratedGithubModelsTokenFromEnv() drops the hydrated token + marker while preserving a user-supplied token (one that does not match the stored credential). +unit tests for match / user-supplied / empty-storage / no-marker cases. * fix(provider): keep switch-back reachable when a non-Anthropic provider is active hasSelectableProviders gated the 'Set active provider' menu item, so when a non-Anthropic provider (GitHub Models or a saved profile) was active but no profile was saved and GitHub credentials were unavailable, the 'Use Anthropic (built-in)' recovery option was unreachable. Add a scoped canSwitchActiveProvider (true whenever GitHub is active or a profile is active) for the activate path only; edit/delete still require an actual profile. Add a ProviderManager UI test for the switch-back flow: select 'Use Anthropic (built-in)' and assert the onDone state (provider name, model reset) and that no managed CLAUDE_CODE_USE_* flags remain. * fix(provider): drop dead wizard switch-back path, dedup switch guard Address review on the legacy ProviderWizard 'anthropic' branch. ProviderWizard is test-only (live /provider renders ProviderManager), so its switch-back option duplicated the real path while diverging from it (no hydrated-GitHub-token cleanup, no model reset) and went untested. Remove the wizard's 'Use Anthropic' option, its handler branch, the now-unused ProviderChoice member, and the imports only it used, leaving the single tested switch-back in ProviderManager. Also reuse the component-scope canSwitchActiveProvider in renderMenu instead of recomputing it, so the two sites cannot drift. * test(provider): restore env + dispose mount in finally, assert token cleanup Address review on the switch-back manager-UI test: snapshot and restore the mutated process env and dispose the Ink mount in a finally block so a failed wait or assertion cannot leak provider flags or a live mount into later tests, and assert clearHydratedGithubModelsTokenFromEnv was called so dropping the hydrated GitHub token cleanup cannot pass unnoticed. * test(provider): assert switch-back forwards stored GitHub Models token The switch-back test seeded no stored token (readGithubModelsToken returned undefined) and only asserted clearHydratedGithubModelsTokenFromEnv was called, so it would still pass if the branch stopped forwarding the stored token into the helper. Seed a stored token and assert toHaveBeenCalledWith(storedToken) so the regression covers the exact GitHub Models switch-back path that preserves a user-supplied GITHUB_TOKEN while clearing only the hydrated secure-storage token. * fix(provider): keep built-in Anthropic active through the startup fallback applyActiveProviderProfileFromConfig() returned without marking provider env as handled for the Anthropic sentinel (getActiveProviderProfile resolves it to undefined). On a cold start after clearActiveProviderProfile() deleted the profile mirror, buildStartupEnvFromProfile() then treated the missing file as a fresh install and synthesized the default Gitlawb OpenGateway env, moving the user off built-in Anthropic. Clear managed provider env and set the applied flag for the sentinel so the fresh-install fallback is suppressed; an explicit startup provider selection still wins. Adds a cold-start regression test. * test(provider): assert startup-override cleanup and isolate cold-start env Address review findings: - ProviderManager switch-back test now anchors on the mocked clearStartupProviderOverrides symbol and asserts the Anthropic branch calls it, so the test fails if that call is dropped and a restart replays the third-party provider (proven fail-on-removal). - Cold-start sentinel test snapshots and clears every CLAUDE_CODE_USE_* flag (OpenAI/GitHub/Gemini/Mistral/Bedrock/Vertex/Foundry) plus the base-url/model and applied markers, restoring them in finally, so an inherited provider flag can no longer route it down the explicit-selection path or leak into later tests. * fix(provider): undo Copilot-key hydration on env cleanup hydrateGithubModelsTokenFromSecureStorage() has two hydration modes: a copilot_key blob populates GITHUB_COPILOT_KEY, while an OAuth blob populates GITHUB_TOKEN. clearHydratedGithubModelsTokenFromEnv() only cleared GITHUB_TOKEN, so undoing a copilot_key hydration removed the ownership marker while leaving the hydrated Copilot key in the session. Clear the GITHUB_COPILOT_KEY branch symmetrically (same stored-token match guard that preserves a user-supplied value). Adds helper coverage for both Copilot-key cases (matched key cleared; user-supplied differing key preserved). * fix(provider): revert hydrated Copilot key on GitHub provider delete The GitHub Models delete path hand-rolled its own env cleanup that only dropped GITHUB_TOKEN, so a hydrated copilot_key (stored in GITHUB_COPILOT_KEY under the same marker) was left behind once the marker was removed. Delegate to the shared clearHydratedGithubModelsTokenFromEnv helper so the delete flow reverts both hydration modes consistently with the switch-back path, and add a ProviderManager delete-flow regression test. * test(provider): assert switch-back refreshes session AppState model Capture AppState updates via onChangeAppState in the switch-back test and assert the Use Anthropic (built-in) path sets mainLoopModel to the Anthropic model from the result and clears mainLoopModelForSession to null. Without this the test would still pass if the setAppState block regressed, leaving a running session on the previous provider model. |
||
|
|
784d9a92ef |
fix(format): show sub-second durations with one decimal instead of "0s" (#1820)
* fix(format): show sub-second durations with one decimal instead of "0s" formatDuration's sub-second branch was guarded by `ms < 1` (1 millisecond) while its comment documents "For durations < 1s, show 1 decimal place". At that threshold the decimal branch could only fire for sub-millisecond values (always returning "0.0s"), so real sub-second durations fell through to Math.floor(ms / 1000) and rendered as "0s" — e.g. a 500ms agent run showed "0s". Corrected the threshold to `ms < 1000`. Adds formatDuration tests (the function was previously untested). * fix(format): round sub-second durations in integer milliseconds Rounding the raw fraction with (ms / 1000).toFixed(1) is unstable because values like 0.95 are not exactly representable in binary floating point, so 950ms rendered as 0.9s and 850ms as 0.8s. Round in integer milliseconds first via a shared oneDecimalSeconds helper, used by both formatSecondsShort and formatDuration's sub-second branch. Added halfway regression cases. --------- Co-authored-by: Pablosinyores <nikhilbajaj0182@gmail.com> |
||
|
NikandGitHub
|
4704cbc474 |
fix(core): join multi-block message text with a real newline (#1793)
extractTextFromContent joined text blocks with a literal "\\n" (backslash-n) instead of a newline. Assistant messages commonly arrive as multi-block content arrays, and the joined text feeds conversation-arc fact extraction whose regexes deliberately treat newlines as boundaries (e.g. env-var and URL values use [^\\s\\n"']+). With the literal separator a value at the end of one block absorbed the next block — e.g. API_KEY=secret123 across two blocks recorded the knowledge-graph value as "secret123\\nand..." instead of "secret123". Use a real newline so blocks stay separated. Adds a regression test asserting the extracted env-var value stops at the block boundary. |
||
|
NikandGitHub
|
de6b6bdd03 |
fix(context): treat Opus 4.7 as 1M-context capable in modelSupports1M (#1670)
modelSupports1M only matched claude-sonnet-4 and opus-4-6, but the firstParty default Opus is now claude-opus-4-7 and the default session model is claude-opus-4-7[1m] (getDefaultMainLoopModelSetting). With 4.7 unmatched, resolveSkillModelOverride drops the [1m] suffix when a skill specifies `model: opus` on an Opus 4.7 session, silently downgrading the effective window from 1M to 200K and tripping autocompact / "Context limit reached" at ~23% apparent usage. The same predicate gates the beta-based 1M window path in getContextWindowForModel. Add opus-4-7 to the predicate (the @[MODEL LAUNCH] checklist item missed at the 4.6 to 4.7 default bump) and cover modelSupports1M with regression tests, including the disable-switch path. |
||
|
NikandGitHub
|
8416faa75c |
fix(BashTool): include captured output in non-zero-exit error result (#1231) (#1249)
* fix(BashTool): include captured output in non-zero-exit error result (#1231) When a Bash tool command exits non-zero, the error result reaching the model and the UI was supposed to carry the merged stdout/stderr so the failure can actually be debugged. In practice users were seeing the result collapse to just "Error: Exit code N" with no diagnostic detail (see #1231 — succeed_with_output / fail_with_output reduced case). The failure path was sourcing the output from `result.stdout` directly while the success path used `stdoutAccumulator.toString()`. The accumulator is the canonical buffer — it captures the streamed output exactly as the success path returns it (with the trimEnd + EOL normalization at the top of the post-completion block), independent of whether the underlying ExecResult.stdout slot is populated. Whenever the shell runner streamed everything through the accumulator and left result.stdout unset (or only partially set), the failure path emitted an empty error body. Switch the ShellError throw to use the accumulator content as the primary source, with `result.stdout` as a fallback. Strip the trailing "Exit code N" marker so it isn't duplicated by getErrorParts(), which already prepends the code from ShellError.code. Behaviour is identical when both buffers agree. * fix(BashTool): recover failure output from progress.fullOutput when stdout slot is empty Addresses @jatmn's P1 finding on #1249. Previously the failure-body recovery picked between: 1. the truncating accumulator (after stripping the synthetic "Exit code N") 2. result.stdout Both sources can be empty in the failure mode reported in #1231: the shell runner streams every line through progress callbacks but the final ExecResult.stdout slot ends up empty (flush-after-result race, exit before EOF, output persisted to a file path, etc.). With both empty the patch collapsed back to the original "Error: Exit code N" body. Add the most recent progress.fullOutput value yielded by the streaming generator as a third fallback source, captured in the consumer loop. The selection logic is extracted into a pure selectFailureOutput() helper so it can be exercised directly by unit tests — including a reproducer for the exact failure mode jatmn called out (accumulator empty, result.stdout undefined, fullOutput non-empty). Local: 39 / 39 utils.test.ts pass, including 7 new selectFailureOutput cases. |
||
|
NikandGitHub
|
a7fc408779 |
fix(plugins): use mergeHooksSettings in marketplace supplement path (#1055) (#1167)
`finishLoadingPluginFromPath` supplemented `plugin.hooksConfig` with the
marketplace entry's hooks via object spread:
plugin.hooksConfig = {
...(plugin.hooksConfig || {}),
...(entry.hooks as HooksSettings),
}
`HooksSettings` values are matcher arrays keyed by event name. Object
spread replaced the entire per-event array from `plugin.json` with the
marketplace entry's array, silently dropping any matchers the manifest
already registered for the same event (e.g. both contributed
`PreToolUse` matchers — only the marketplace ones survived).
`mergeHooksSettings` already exists in this file and concatenates
per-event arrays correctly; it is the helper used in
`createPluginFromPath` for the analogous merge. Use it in the
marketplace supplement path too, and export it so the concat-not-replace
contract is locked in by a unit test.
|
||
|
NikandGitHub
|
890456b35e |
feat(nvidia-nim): dynamic model discovery via integrate.api.nvidia.com (#1099) (#1177)
* feat(nvidia-nim): dynamic model discovery against integrate.api.nvidia.com (#1099) Mirror the #1143 Groq hybrid-catalog pattern for the NVIDIA NIM gateway: replace the single-entry static catalog with discovery against https://integrate.api.nvidia.com/v1/models and a filter that excludes embedding, retriever, reranker, ASR (whisper, parakeet, canary, riva), TTS, image-gen (SDXL, flux, stable-diffusion, kosmos, florence), safety (llama-guard, nemoguard, content-safety), and reward models so the /model picker only surfaces chat/instruct ids. Settings match Groq's: - catalog.source: hybrid (keep the existing Nemotron 70B as the static fallback when discovery is unavailable) - discoveryCacheTtl: 1d - discoveryRefreshMode: background-if-stale - allowManualRefresh: true Adds a focused gateway test (`nvidia-nim.test.ts`) pinning the filter regex against representative real NVIDIA model ids — keeps, embedding drops, ASR drops, image-gen drops, safety drops, inactive drops, plus context_window forwarding — so the filter does not silently start admitting non-chat models as NVIDIA's catalog grows. The existing `src/utils/model/nvidiaNimModels.ts` env-var path (used when users set NVIDIA_NIM or detect via OPENAI_BASE_URL) is unchanged for now; its hand-rolled list keeps working. Wiring that path through the discovery service is a separate, larger change. * fix(nvidia-nim): allowlist chat models + accept discovered ids via /model Two reviewer-flagged blockers on the dynamic-discovery PR: 1. The existing exclusion regex silently admitted any non-chat model whose id did not contain one of its keyword tokens, so the live catalog at integrate.api.nvidia.com was pushing entries like baai/bge-m3, google/deplot, nvidia/gliner-pii, and nvidia/ising-calibration-* into the /model picker. Switch the filter to a positive allowlist keyed on instruct/chat/reasoning/code markers and known chat families. Retain a tight non-chat blacklist as a defense-in-depth pass for the rare instruct-tuned classifier (gliner-pii, deplot, etc.). 2. Inline `/model <id>` rejected ids that only existed in the discovery cache because validateModel checked the static catalog only. Add getDiscoveredNvidiaNimModelIds(), which reads the persisted discovery cache via getDiscoveryCacheKey/getCachedModels (includeStale: true), and consult it from validateModel before surfacing the not-found error. * fix(nvidia-nim): share discovery cache partition with descriptor picker Addresses @jatmn's [P2] re-review finding on #1177. The inline `/model <id>` fallback in `getDiscoveredNvidiaNimModelIds` built its cache key from a hard-coded `process.env.NVIDIA_API_KEY`, but the descriptor picker / startup discovery path (`getOpenAIDiscoveryRequestOptions` in `src/commands/model/model.tsx`) goes through `resolveRouteCredentialValue({ routeId: 'nvidia-nim' })` — whose credential list for the OpenAI-compatible `nvidia-nim` route includes both `NVIDIA_API_KEY` *and* `OPENAI_API_KEY`. Result: a valid NVIDIA setup authenticating via `OPENAI_API_KEY` would populate the discovery cache under the OpenAI-compatible partition, while `getDiscoveredNvidiaNimModelIds` looked in a different no-key partition and rejected the very models the picker had just learned about. `getDiscoveredNvidiaNimModelIds` now mirrors the picker's resolution shape: - `resolveProviderRequest({ model: OPENAI_MODEL, baseUrl: OPENAI_BASE_URL })` resolves the active route's effective base URL. - `resolveRouteCredentialValue({ routeId: 'nvidia-nim', baseUrl, processEnv })` walks the route's full credential list, so both `NVIDIA_API_KEY` and `OPENAI_API_KEY` setups land on the same cache partition the picker wrote. Behaviour-preserving: - `getDiscoveryCacheKey` still receives `undefined` for `apiKey` when no credential is present (matching the picker's pass-through), so the no-key case keeps the same `apiKeyHash: ''` partition as before — the failure-mode shape is unchanged for users who never set either env var. Local: `bun test src/integrations/gateways/nvidia-nim.test.ts` 10/10. * fix(nvidia-nim): include custom headers in discovery cache key (#1099) jatmn re-review on #1177 (2026-05-21): the inline `/model <id>` fallback in `getDiscoveredNvidiaNimModelIds()` was still rebuilding the discovery cache key with only `(baseUrl, apiKey)`. The descriptor picker side (`getOpenAIDiscoveryRequestOptions` in src/commands/model/model.tsx) passes `headers: parseCustomHeadersEnv(process.env.ANTHROPIC_CUSTOM_HEADERS)` into `getDiscoveryCacheKey`, so two users sharing a baseUrl + apiKey but differing in `ANTHROPIC_CUSTOM_HEADERS` ended up on different cache partitions and the inline validator missed the discovered ids the picker had just written. Pass the same parsed custom headers into the inline cache-key build so both code paths hash the same `(baseUrl, apiKey, headers)` shape. Add `nvidiaNimModels.test.ts` to pin the partition parity: - custom headers shift the partition off the no-headers default - inline validator key equals picker key for the same headers env - absent `ANTHROPIC_CUSTOM_HEADERS` keeps both keys identical |
||
|
NikandGitHub
|
ad3e208592 |
fix(promptinput): keep bash-mode ! out of the local mirror (#1179) (#1182)
* fix(promptinput): keep bash-mode `!` out of the local mirror (#1179) Typing `!` into empty input is meant to enter bash mode and leave the prompt buffer empty (the `!` shows in the mode prefix only). The useTextInput special case at the default keystroke handler was `cursor.insert(text).left()`, which placed `!` into the cursor text with the offset at 0, then called `onChange("!")`. PromptInput's `detectModeEntry` then stripped the controlled parent value back to "" with cursor 0 — values it numerically already held. Because the parent's controlled props ended up identical to what they were before the keystroke, React did not re-render PromptInput, the useLayoutEffect in useTextInput never re-ran, and the local mirror retained `!` at offset 0. Subsequent keystrokes inserted before the retained `!`, producing "!git status" with the cursor wedged before the `!` instead of a clean "git status" buffer. Fix is in useTextInput's default handler: when the keystroke is the input-mode character at the start of an empty buffer, emit `onChange` as a one-shot mode-entry notification but return `undefined` so `setValue` is not called. The local mirror stays at "" / offset 0, the parent strip remains a no-op on the controlled state, and the next character is inserted into a clean buffer. Test: - New regression in TextInput.test.tsx that mounts a controlled TextInput with a parent `onChange` mirroring PromptInput's strip (return early with `setValue('')` when the new value starts with `!`), types `!` then `git`, and asserts the rendered frame contains `git` and does NOT contain `!`, `!git`, or `git!`. - 4/4 in TextInput.test.tsx, 24/24 across PromptInput + hooks suites, build clean. * fix(promptinput): require empty buffer before suppressing bash-mode `!` mirror The previous condition keyed off cursor.isAtStart(), which is true at offset 0 even when the buffer is non-empty. If the prompt already contained 'git status' and the user moved the cursor to the start to prepend '!', the bang-mode handler emitted onChange('!') and skipped the cursor.insert, so the parent's strip handler wiped the buffer back to ''. Require cursor.text.length === 0 as well so the one-shot onChange path runs only on a truly empty buffer; otherwise fall through to cursor.insert(text) and let the parent see '!git status' for normal prepend behaviour. |
||
|
NikandGitHub
|
5247fb8977 |
fix(teammate-progress): keep cumulative token+tool counts across prompts (#475) (#1402)
The in-process teammate runner re-created the progress tracker on every prompt iteration, so task.progress.tokenCount and toolUseCount were reset between leader prompts to the same teammate. TeammateSpinnerLine, InProcessTeammateDetailDialog and the Spinner aggregate all read these counters directly, which is why agent-team pills appeared to lose tokens and tool uses partway through a session. The Claude API returns input_tokens as cumulative-per-request (each turn re-sends forkContextMessages history), so latestInputTokens already captures the running context cost. The fix moves createProgressTracker out of the while-loop so cumulativeOutputTokens and toolUseCount also keep their running totals across multiple prompts. Adds src/tasks/LocalAgentTask/progressTracker.test.ts pinning: - output tokens accumulate across multiple assistant messages - cumulative semantic survives a simulated multi-prompt teammate session - fresh-tracker-per-prompt regression repro (prior outputs + tool uses lost) - tool use count accumulates - cache_creation/read input tokens fold into latestInputTokens - recentActivities stays capped while toolUseCount keeps climbing bun test (full): 2998/2998 pass. bun run build clean. |
||
|
NikandGitHub
|
c53ef18716 |
fix(bashPermissions): apply MAX_SUBCOMMANDS cap in sandbox auto-allow path (#1057) (#1166)
* fix(bashPermissions): apply MAX_SUBCOMMANDS cap in sandbox auto-allow path (#1057) The cap from #21405 is enforced in `bashToolHasPermission`, but the `checkSandboxAutoAllow` shortcut path called `splitCommand` and iterated `matchingRulesForInput` once per subcommand before the main-path cap got a chance to run. With auto-allow-bash-if-sandboxed enabled, a crafted compound command whose legacy `splitCommand` output explodes could trigger N rule lookups in this path. Mirror the existing cap (MAX_SUBCOMMANDS_FOR_SECURITY_CHECK = 50) in `checkSandboxAutoAllow` right after the `splitCommand(command)` call: log + return `ask` with the same decision-reason shape as the main path. Regression test in bashPermissions.test.ts exercises sandbox auto-allow with 60 echo subcommands and asserts the `ask` short-circuit. * ci: re-trigger checks (likely-flaky failures on unrelated profile/SDK/KG tests) * fix(bashPermissions): gate sandbox cap on legacy splitter path (CC-643) Address reviewer feedback on #1057: the previous patch applied MAX_SUBCOMMANDS_FOR_SECURITY_CHECK in checkSandboxAutoAllow unconditionally on splitCommand output. The main bashToolHasPermission path only applies that cap when astSubcommands === null, because the fanout/ReDoS concern is specific to the legacy splitter — AST-parseable compound commands (e.g. long echo chains) are already bounded by structural parse and should not be downgraded to ask. Thread astSubcommands into checkSandboxAutoAllow and only fire the cap when AST is unavailable. Export checkSandboxAutoAllow so the symmetric behavior is directly testable without depending on tree-sitter WASM availability in the test runtime. Tests: - Legacy path (CLAUDE_CODE_DISABLE_COMMAND_INJECTION_CHECK=1 OR astSubcommands=null) over 50 subcommands -> ask, cap reason. - AST-validated path (astSubcommands provided) with 60 subcommands -> allow, sandbox auto-allow reason. |
||
|
NikandGitHub
|
b3b771476d |
fix(websearch): surface adapter failure when auto mode falls back to native (#994) (#1168)
* fix(websearch): surface adapter failure when auto mode falls back to native (#994) When `WEB_SEARCH_PROVIDER=auto` and the configured adapter chain fails on a recoverable error (DuckDuckGo "rate-limited from this network", adapter timeout, 5xx, etc.), the tool falls through to the native Anthropic / Codex web-search path silently. The only signal that the adapter failed is a `console.error` line — it never reaches the tool result the user sees. On rate-limit-prone networks (datacenter IPs, VPNs) this manifests as "no results found" with no actionable hint, exactly the symptom reported in #994. This change captures the adapter error in `adapterFallthroughNotice` inside the catch branch and prepends it to the eventual native / Codex output via a small pure helper, `withAdapterFallthroughNotice`. The hits-present and native-error paths are unchanged; the helper only mutates a shallow copy when a notice is set, and is a no-op otherwise. Result: users on a rate-limited adapter chain who get native results also see *why* the adapter failed, and users whose native search also returns nothing finally get the actionable diagnostic (configure TAVILY_API_KEY / FIRECRAWL_API_KEY / etc.) instead of a silent empty. Test coverage in WebSearchTool.test.ts asserts the pure-helper contract: no-op when notice is undefined, prepend-not-mutate when a notice is provided. * fix(websearch): narrow #994 fix to the reachable adapter-failure surface Address @techbrewboss feedback: the previous patch's `adapterFallthroughNotice` machinery and the `withAdapterFallthroughNotice` helper were unreachable under the current provider selection. `shouldUseAdapterProvider()` and `hasNativeSearchFallback()` are mutually exclusive in auto mode — when a native path exists (firstParty/vertex/foundry/Codex) the adapter is never tried, and when the adapter IS tried (openai-shim providers) there is no native fallback. So the assignment at `adapterFallthroughNotice = ...` and both `withAdapterFallthroughNotice(...)` call sites could never fire. Narrow the PR to the path that #994 actually hits today: an openai-shim provider (moonshot/minimax/nvidia-nim/github copilot) where the adapter fails transiently and there is no native fallback. The existing throw at that branch already surfaces the underlying adapter error verbatim; extract `buildAdapterUnavailableError(provider, errMsg)` so it is directly testable and cannot regress, and replace the dead notice helper + its tests with focused coverage of the reachable message. Drop the no-op shallow-copy `withAdapterFallthroughNotice` helper and its two tests; keep the descriptive error throw as the single, reachable surfacing path. |
||
|
NikandGitHub
|
0f6668f554 | feat(nvidia-nim): add latest chat models, remove duplicate Mixtral 8x22B entry. Verified against integrate.api.nvidia.com/v1/models on 2026-05-13. Tracks #1099. (#1145) | ||
|
NikandGitHub
|
0c88defbe0 |
fix(bashSecurity): tighten fc -e detection to avoid long-flag false positives (#1107)
Closes #1051 (BUG-01). The regex `/\s-\S*e/` in `validateZshDangerousCommands` only required `e` to appear *somewhere* after `-`, so any flag that happened to contain `e` — `-reset`, `-reverse`, `-message`, etc. — tripped the "dangerous fc" path and surfaced an interactive permission prompt to the user even though those flags do not invoke an editor and have nothing to do with the `fc -e <editor>` eval vector the check is trying to catch. Replace with `/\s-[a-zA-Z]{0,3}e(?:\s|$)/` so: * `e` must be the last letter in the flag bundle (followed by whitespace or end-of-string), not anywhere inside it * the bundle is capped at 4 chars total, matching the shape of real POSIX `fc` short-flag bundles (`-e`, `-le`, `-lne`) The bundle cap is what distinguishes us from the issue's initial suggested fix `/\s-[a-zA-Z]*e(?:\s|$)/`, which still false-positives on `-reverse` and `-message` because both end in `e` and the unbounded `*` swallows the entire word. New regression tests in `bashSecurity.test.ts` cover the real-flag positive cases (`-e`, `-le`, `-lne`) and the long-flag negative cases called out in the bug report (`-reset`, `-reverse`, `-message`), plus `-l` to confirm the safe list flag still passes through. |