mirror of
https://github.com/Gitlawb/openclaude.git
synced 2026-08-24 02:34:15 -05:00
main
1189
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  0xfandomandGitHub
|
eeed68f4fd |
feat(provider): add Cloudflare Workers AI integration (#1100) (#1178)
* feat(provider): add Cloudflare Workers AI integration
Adds Cloudflare Workers AI as a first-class OpenAI-compatible provider
preset, modeled on the Venice / Xiaomi MiMo descriptors.
- New `src/integrations/vendors/cloudflare.ts` descriptor:
- `classification: 'openai-compatible'`
- Default base URL with literal `<ACCOUNT_ID>` placeholder — users
substitute via `/provider` baseUrl edit, same shape as the Azure
OpenAI example already in `docs/advanced-setup.md`
- `CLOUDFLARE_API_TOKEN` env, with `OPENAI_API_KEY` as fallback
- `removeBodyFields: ['store']` since Workers AI rejects unknown
OpenAI body fields (mirrors Mistral / Gemini / Cerebras strip)
- Static catalog with current Workers AI chat models
(`@cf/meta/llama-3.3-70b-instruct-fp8-fast`,
`@cf/meta/llama-3.1-8b-instruct`,
`@cf/deepseek-ai/deepseek-r1-distill-qwen-32b`,
`@cf/qwen/qwen2.5-coder-32b-instruct`)
- Validation routing on `api.cloudflare.com` /
`gateway.ai.cloudflare.com` hosts so an env-pasted URL maps back
to the preset
- Env mirror sites in `src/utils/providerProfiles.ts`: mirror api key
into `CLOUDFLARE_API_TOKEN` when baseUrl contains a Cloudflare host
(3 sites: same-env check, openAIProfileEnv build, applyEnv).
- `CLOUDFLARE_API_TOKEN` added to `PROFILE_ENV_KEYS` / `SECRET_ENV_KEYS` /
`ProfileEnv` / `SecretValueSource` in `src/utils/providerProfile.ts`
so the profile-clean and secret-redact paths know about it.
- `src/utils/providerFlag.ts` `--provider <name>` startup flag now
detects a Cloudflare profile from `OPENAI_API_KEY ===
CLOUDFLARE_API_TOKEN` (mirrors how the other host-key mirrors are
reverse-mapped to their preset id).
- `bun run scripts/generate-integrations-artifacts.ts` regenerated
`integrationArtifacts.generated.ts` to include the cloudflare preset
+ route + vendor.
- Tests: `compatibility.test.ts` PRESETS list, new
`buildProfileSaveMessage` Cloudflare case in `provider.test.tsx`,
new `applyProviderProfileToProcessEnv` Cloudflare case in
`providerProfiles.test.ts`.
- Docs: README providers table row + `docs/advanced-setup.md` section
matching the MiMo / Mistral entries.
- Dedicated AI Gateway integration with `gateway_id` URL templating.
Today users can still paste a full Gateway URL into `OPENAI_BASE_URL`
and the preset's `matchBaseUrlHosts` picks `gateway.ai.cloudflare.com`
up.
- Dynamic `/models` discovery on the Groq #1143 / `mapModel` pattern —
Cloudflare's `/v1/models` returns the runnable model list and the
hybrid catalog path drops in cleanly. Left as a separate PR so this
one stays a focused preset add.
Closes #1100
* fix(cloudflare): narrow route matching to api.cloudflare.com host
`gateway.ai.cloudflare.com` is the shared host for *all* Cloudflare AI
Gateway routes (Workers AI, Anthropic, OpenAI, etc.), so matching it to
the Workers AI preset applied Workers-AI runtime metadata and
credential precedence (CLOUDFLARE_API_TOKEN before OPENAI_API_KEY, body
'store' strip, max_tokens field) to other providers' Gateway URLs.
Drop the shared host from the match list; a dedicated AI Gateway
integration with path-aware routing is the right follow-up.
Refs #1100.
* fix(provider-manager): keep Codex OAuth after DeepSeek when cloudflare added
The picker hardcoded `options.splice(7, 0, …)` to drop the Codex OAuth
entry right after DeepSeek. Adding cloudflare to ORDERED_PROVIDER_PRESETS
bumped DeepSeek to index 7, so the splice now lands Codex OAuth *before*
DeepSeek and breaks the test fixture that drives navigateToPreset by
keypress count.
Switch to a dynamic `findIndex('deepseek') + 1` lookup so any future
preset inserted between Bankr and DeepSeek keeps the established
ordering. Fixture updated to mirror the new picker order.
Caught by CI on 12b3ff… smoke-and-tests: 8 ProviderManager tests
timing out because navigateToPreset overshot/undershot the target.
* fix(cloudflare): exclude the shared AI Gateway host from Cloudflare routing
The profile env/alignment/startup paths mirrored CLOUDFLARE_API_TOKEN whenever
the profile URL merely contained 'gateway.ai.cloudflare.com'. That host is the
shared AI Gateway for all Cloudflare AI routes (Workers AI, OpenAI, Anthropic,
...), so a profile retargeted to /openai or /anthropic Gateway URLs was wrongly
tied to the Cloudflare route and credential precedence.
Add isCloudflareBaseUrl (hostname === api.cloudflare.com, matching the Workers
AI host and the descriptor's matchBaseUrlHosts) and route all three sites
through it, consistent with isXaiBaseUrl/isFireworksBaseUrl. Also restore
CLOUDFLARE_API_TOKEN in the provider profile test cleanup keys.
* fix(cloudflare): don't seed the placeholder base URL from the CLI shortcut
`openclaude --provider cloudflare` fell through the generic OpenAI-compatible
branch and applied the descriptor default base URL verbatim — including the
unresolved `<ACCOUNT_ID>` placeholder — leaving the shortcut 'configured' with
an endpoint that cannot serve a request. Skip seeding any base URL that still
contains a `<...>` placeholder, so the user must supply a real account-scoped
URL (OPENAI_BASE_URL / `/provider` edit) first, matching how the wizard treats
placeholder endpoints.
* test(cloudflare): assert exact null fallback for AI Gateway routes
The shared AI Gateway URL assertions used `.not.toBe('cloudflare')`, which
would also pass for any other non-cloudflare return value. The intended
fallback is null, so assert `.toBe(null)` to lock the regression boundary.
* fix(cloudflare): gate profile token mirroring on base URL host only
applyProviderProfileToProcessEnv mirrored CLOUDFLARE_API_TOKEN whenever
route.routeId === 'cloudflare'. route comes from the saved profile.provider,
so that disjunct is always true for a cloudflare profile, including one
retargeted to the shared gateway.ai.cloudflare.com AI Gateway host. The
sibling sites (isProcessEnvAlignedWithProfile, buildOpenAICompatibleStartupEnv)
already key on isCloudflareBaseUrl only; align this site with them so a
shared-gateway profile no longer leaks the token or stays pinned to the
cloudflare route. Add a regression test for the gateway.ai.cloudflare.com case.
* chore(integrations): regenerate artifacts for the Cloudflare vendor
The rebase took main's generated artifacts at the conflict; regenerate so the
Cloudflare vendor descriptor is registered in VENDOR_DESCRIPTORS and the
manifest alongside the providers main added.
* fix(cloudflare): mirror CLOUDFLARE_API_TOKEN into the OpenAI-compatible auth path
The --provider cloudflare shortcut fell through to the generic
OpenAI-compatible default branch and never copied CLOUDFLARE_API_TOKEN
into OPENAI_API_KEY, so a user who only set the token sent an
unauthenticated request. Add a dedicated cloudflare case that mirrors the
token (and clears a stale generic key when absent), keeping the
placeholder-URL skip.
buildOpenAICompatibleStartupEnv also returned from its strict-env branch
before the fallback CLOUDFLARE_API_TOKEN mirror, so a keyed Cloudflare
profile persisted a startup env that omitted the token and re-detected
inconsistently after relaunch. Mirror it in the strict branch alongside
nearai/fireworks. Add regression coverage for both paths.
* fix(cloudflare): gate token mirroring on a real Cloudflare endpoint
The cloudflare shortcut copied CLOUDFLARE_API_TOKEN into the generic
OPENAI_API_KEY unconditionally. The descriptor default carries an
unresolved `<ACCOUNT_ID>` placeholder and is never seeded, so with
OPENAI_BASE_URL unset (or still pointing at a previous OpenAI-compatible
provider) the token would be attached to the wrong host. Gate the mirror
on isCloudflareBaseUrl(getConfiguredOpenAIBaseUrl()) — only seed
OPENAI_API_KEY once the configured base URL resolves to
api.cloudflare.com, otherwise fail fast and leave it unset. Add
regression coverage for the unconfigured, stale-host, and AI-Gateway-host
cases.
* fix(cloudflare): reject placeholder URL and keep the OPENAI_API_KEY fallback
The token mirror keyed on the api.cloudflare.com host only, so the literal
<ACCOUNT_ID> placeholder URL (same host) passed the gate and copied the
token onto a non-working endpoint. It also deleted any generic
OPENAI_API_KEY when no token was set, breaking the documented
compatibility fallback for users authenticating a real Workers AI URL with
OPENAI_API_KEY. Mirror only on a real (non-placeholder) Cloudflare
endpoint, and preserve an existing generic key there when no dedicated
token is present.
Refs #1100
* refactor(cloudflare): model Workers AI as a gateway, not a vendor
Cloudflare Workers AI is a hosted OpenAI-compatible inference endpoint
reached over the shared openai transport, so it belongs with the gateway
providers (atlas-cloud, groq, together, ...) rather than the transport
vendors. Move it to gateways/cloudflare.ts via defineGateway (category
hosted, vendorId openai), regenerate the integration artifacts, and
allowlist its provider-specific @cf/* catalog ids in the gateway
descriptor check (no shared cross-provider descriptor exists, same as
azure-deployment).
Refs #1100
* fix(cloudflare): key Workers AI detection on the account path, not the host
api.cloudflare.com also serves the general Cloudflare REST API, so matching the
whole host treated unrelated URLs (e.g. /client/v4/user/tokens/verify) as the
Workers AI route and mirrored CLOUDFLARE_API_TOKEN into OPENAI_API_KEY for them.
isCloudflareBaseUrl now requires the Workers AI path
/client/v4/accounts/<account_id>/ai/v1 with a real (non-placeholder) account id,
and resolveRouteIdFromBaseUrl guards its cloudflare hostname match through the
same predicate. Both route detection and token/profile mirroring key on the
actual Workers AI endpoint.
Adds same-host negative regressions (general REST path is not routed and does
not mirror the token; unresolved <ACCOUNT_ID> placeholder is excluded) and
asserts the Cloudflare Workers AI preset appears in the first-run picker.
* fix(cloudflare): honor the Workers AI path boundary in the profile-provider fallback
resolveActiveRouteIdFromEnv returned the saved active-profile provider's route
id before consulting its base URL. For a `cloudflare` profile that had been
retargeted to a non-Workers URL — the shared AI Gateway host, or a general
api.cloudflare.com REST path — this still resolved as `cloudflare`, so the
Workers AI shim config (removeBodyFields: ['store'], Cloudflare model metadata)
and CLOUDFLARE_API_TOKEN mirroring were applied to a generic endpoint, even
though resolveRouteIdFromBaseUrl already excludes those URLs.
Gate the profile-provider shortcut through profileRouteHonorsBaseUrlBoundary,
which requires the path-aware isCloudflareBaseUrl for the cloudflare route (all
other routes are host-scoped by resolveProfileRoute and unaffected). A retargeted
profile now falls through to the generic openai/custom resolution; a genuine
Workers AI profile base URL still resolves as cloudflare.
Adds regressions for both retarget cases (gateway host + REST path) and the
positive Workers AI profile case.
* fix(cloudflare): require HTTPS and honor the Workers AI path in validation
isCloudflareBaseUrl accepted any scheme, so http://api.cloudflare.com/
client/v4/accounts/<id>/ai/v1 resolved as the cloudflare route and mirrored
CLOUDFLARE_API_TOKEN into OPENAI_API_KEY over cleartext. Require url.protocol
=== 'https:'.
Startup validation selected the Cloudflare target on host match alone, so a
non-Workers path like /client/v4/user/tokens/verify demanded Workers AI auth
instead of falling back to generic OpenAI validation. Gate the cloudflare
target on isCloudflareBaseUrl(request.baseUrl), mirroring the runtime route
resolver's path boundary.
* test(cloudflare): lock non-Workers path token boundary; fix stale host-only comments
The apply/persist paths already gate CLOUDFLARE_API_TOKEN mirroring on the
isCloudflareBaseUrl path predicate, but had no coverage for a same-host
non-Workers path (api.cloudflare.com/client/v4/user/tokens/verify) and the
comments beside the mirroring sites still described a host-only boundary.
Add negative apply and persist regressions asserting the token is not mirrored
or persisted for that non-Workers URL, and update the comments to describe the
real Workers AI path predicate instead of host-only matching.
* fix(cloudflare): fall back to a generic route for retargeted profiles
resolveProfileCapabilityRouteId returned the cloudflare capability route id for
any saved cloudflare profile whose base URL no longer resolves — including one
retargeted to gateway.ai.cloudflare.com or another OpenAI-compatible host. That
stripped generic capabilities (apiFormat, custom auth/request headers) from
profile sanitize/apply even though the runtime resolver runs such a profile as
a generic OpenAI-compatible route. Mirror the same isCloudflareBaseUrl boundary:
keep the cloudflare route only for the real Workers AI URL (or the unset
descriptor default) and fall back to 'custom' otherwise. Regression asserts a
retargeted cloudflare profile preserves OPENAI_API_FORMAT.
* test(cloudflare): assert retargeted profile resolves to the custom route
Pin both resolveActiveRouteIdFromEnv assertions for a retargeted cloudflare
profile to .toBe('custom') instead of .not.toBe('cloudflare'), so the test
locks the intended generic OpenAI-compatible fallback rather than merely
excluding the cloudflare route.
|
||
|
0xfandomandGitHub
|
3f85b255dd |
fix(commands): escape named-argument names before building the regex (#1914)
substituteArguments builds a dynamic RegExp from each frontmatter-defined
argument name without escaping regex metacharacters:
new RegExp(`\$${name}(?![\[\w])`, 'g'). parseArgumentNames only rejects
empty and numeric-only names, so an author-defined name that contains a regex
special char reaches the constructor. A name with an unbalanced '(' / '[' (e.g.
'pattern)') throws a SyntaxError on every invocation of that skill/command, and
a name like 'a.' silently over-matches ('.' turns $ab into the arg value).
Escape the name with the existing escapeRegExp helper so it is matched
literally.
Consumers: src/skills/loadSkillsDir.ts and src/utils/plugins/loadPluginCommands.ts
feed frontmatter argument names into substituteArguments.
|
||
 JATMNandGitHub
|
6602076b53 |
refactor(messages): extract system factories (6 of 8) (#1903)
* refactor(messages): extract system factories * test(messages): cover system factory extraction |
||
|
JATMNandGitHub
|
a154f711da |
refactor(messages): extract normalization helpers (3 of 8) (#1900)
* refactor(messages): extract normalization helpers * fix(messages): avoid discarded UUID allocation * style(messages): align normalize re-export quotes |
||
|
JATMNandGitHub
|
fc568b9a34 |
refactor(messages): extract streaming helpers (2 of 8) (#1899)
* refactor(messages): extract streaming helpers * fix(messages): remove streaming EOF whitespace * chore(messages): clean streaming extraction imports |
||
|
JATMNandGitHub
|
e42aeb34f7 |
refactor(messages): extract tool pairing helpers (1 of 8) (#1898)
* refactor(messages): extract tool pairing helpers * style(messages): align tool pairing re-export quotes |
||
|
JATMNandGitHub
|
e086e8c35a |
fix(safety): relax over-restrictive safety checks for benign coding tasks (#1897)
* fix(safety): relax over-restrictive safety checks for benign coding tasks (Fixes #1616) Issue #1616 reports refusals for routine, benign coding tasks. Two layers caused this: 1. Model-level over-refusal: CYBER_RISK_INSTRUCTION and the 'ask before acting' guidance biased the model toward refusing normal work. Reworded to explicitly permit ordinary engineering tasks and dual-use/security-adjacent work in authorized contexts, and to ask a clarifying question rather than refuse when intent is ambiguous. 2. Application-level heuristics that become hard blocks in auto/YOLO/headless mode: the bash command-injection check, the broad DANGEROUS_FILES/DANGEROUS_DIRECTORIES auto-edit guard, and the auto-mode stripping of ordinary interpreter allow-rules (Bash(python:*), npm run:*, etc.). Added an OPENCLAUDE_SAFETY_LEVEL knob (strict|balanced|permissive, default balanced). In 'permissive' the application-level heuristics above are relaxed while genuine Windows-path/symlink guards remain active. Default behavior is unchanged. Validation: - bun run typecheck: clean - bun run build: succeeds - bun test safetyLevel.test.ts, bashSecurity.safety.test.ts: pass - bashSecurity.test.ts, filesystem.test.ts, permissionSetup.test.ts, security-hardening.test.ts: pass (no regressions) * fix(safety): narrow permissive safety relaxations * fix(safety): address review follow-ups * fix(safety): address additional review findings * refactor(permissions): share rule normalization |
||
|
JATMNandGitHub
|
de9729500b |
fix(nvidia-nim): enable reasoning template kwargs (#1893)
* fix(nvidia-nim): enable reasoning template kwargs * fix(nim): preserve explicit provider selections * fix(nim): limit env-only startup precedence |
||
|
0xfandomandGitHub
|
cde6e090d3 |
fix(read): report zero lines for an empty file (#1881)
* fix(read): report zero lines for an empty file readFileInRangeFast ran the final-fragment block unconditionally, so a 0-byte file pushed one phantom empty line and returned totalLines: 1. FileReadTool picks its empty-file warning on totalLines === 0, so an empty file instead hit the else branch and emitted the wrong message: "the file exists but is shorter than the provided offset (1). The file has 1 lines." — leaving the dedicated "the contents are empty" message unreachable. Short-circuit empty input right after the BOM strip to return totalLines: 0. Trailing-newline counting (split semantics) is unchanged. Regression covers the empty file plus one-line and two-line no-trailing-newline controls. * test(read): clean up temp dirs created by readFileInRange tests Track each mkdtempSync directory and remove it in afterEach so the new tests don't leak openclaude-readrange-* dirs under the OS temp path across runs. |
||
 
|
2047fb250f |
fix(query-guard): exclude human-interaction wait from session timeout (#1879)
* fix(query-guard): exclude human-interaction wait from session timeout The session watchdog (idle 5min / hard-max 30min) counted time spent blocked on a human decision — permission prompts, AskUserQuestion, plan-mode selection — as stuck work and force-ended the query when a user took too long to choose. The watchdog is a fork-local addition (upstream Claude Code has no session-level query timeout) that never excluded human think-time. Add a reference-counted QueryGuard.beginUserInteraction() that freezes the watchdog while blocked on the user and, on resume, shifts the hard-max and lease deadlines forward by the paused duration and restarts the idle window. Wire it through queryActivity and wrap the permission resolution in toolExecution so every interactive 'ask' is covered. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(query-guard): scope watchdog suspension to interactive permission wait Address review feedback on #1879: - Move the suspend/resume out of checkPermissionsAndCallTool's wrapper around the whole permission resolution and into interactiveHandler, so it covers only the window that truly blocks on user input. Non-human async work (e.g. the classifier in hasPermissionsToUseTool) stays watched and a genuinely stuck check can still fire the watchdog. - Reset _suspendedAt alongside _suspendCount in end()/forceEnd() to keep the 'not suspended => suspendedAt=0' invariant. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(query-guard): cover watchdog suspend/resume wiring in interactiveHandler Address CodeRabbit feedback on #1879: add a focused test for the interactive permission path asserting beginUserInteraction runs once and the resume fn fires exactly once per terminal resolution (allow/reject/abort), and only once when two paths race, so a future resolution path bypassing resolveOnce fails the test instead of silently reintroducing the timeout bug. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(query-guard): resume watchdog on claim, not on resolve Address maintainer review on #1879 (P2a/P2b): resume the query watchdog the moment a permission decision is claimed, not when resolveOnce() completes. Every terminal path claims first, so resuming there means (a) post-decision async work (handleUserAllow -> persistPermissions) runs watched again once the human has decided, and (b) an exception in that work can no longer strand the watchdog suspended for the rest of the turn -- the likely cause of the CI smoke-and-tests full-suite hang. Resume stays idempotent (a resolveOnce safety net remains). Adds tests: resume-before-await and resume-on-throw. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(test): stop QueryGuard suspension tests leaking fake timers The beginUserInteraction describe block was accidentally nested inside the QueryLifecycleOperationTracker describe, which has no afterEach — so its vi.useFakeTimers() was never restored. The leaked fake clock froze setTimeout in whatever test file ran next, hanging the full single-concurrency suite: CI smoke-and-tests ran ~29m and was cancelled, stalling at src/utils/cwd.test.ts. Moved the block into the QueryGuard describe whose afterEach restores real timers. Verified locally: QueryGuard.test.ts + cwd.test.ts, and the full CI-order utils batch that previously hung >115s, now pass in <1s. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(query-guard): call watchdog resume at most once (exactly-once contract) Address Copilot review on #1879: beginUserInteraction()'s resume fn is documented as call-exactly-once, but it was invoked from both claim() and the resolveOnce safety net. Wrap it in a local idempotent helper so the underlying QueryActivity resume runs at most once, rather than relying on QueryGuard's own idempotence (other implementations may not have it). The test mock is now a plain spy, so a double-call would fail the exactly-once assertions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(query-guard): resume watchdog on external aborts An abort that bypasses the permission dialog callbacks — a bridge interrupt, or the REPL's now-priority/backgrounding paths calling abortController.abort() while a prompt is open — never runs claim()/resolveOnce(), so the captured resume was never called and QueryGuard stayed suspended. The turn could then hang on the unresolved permission promise with the watchdog disabled, unable to recover. Resume on the abort signal (idempotent, once) so the watchdog always recovers. Adds tests: abort-after-open, already-aborted, and no-double-resume. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(query-guard): resume watchdog if dialog setup throws synchronously Address review [P3]: beginUserInteraction() suspends QueryGuard before the handler pushes the dialog and before any claim(). A synchronous throw during setup (e.g. pushToQueue or bridge wiring) would exit the handler before any claim()/resolveOnce() runs, leaving the watchdog suspended for the rest of the turn. Wrap the setup path in try/catch that resumes then rethrows, so the error still propagates. Complements the abort-signal path (async cancel) — the two cover distinct exit modes and resume stays idempotent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(query-guard): resolve externally-aborted prompts immediately Address review [P2]: on an abort that bypasses the dialog callbacks (bridge interrupt, REPL now-priority/backgrounding), the abort handler only resumed the watchdog. That left the permission promise unresolved while resetting the idle deadline, so an already-open prompt kept queryGuard.isActive true for a full idle timeout before cleanup, blocking the queued interrupt/background work. The abort path now claims and resolves/cancels the pending permission (claim() also resumes the watchdog), so the awaiter unblocks immediately. Bridge/channel blocks still clean up their own subscriptions on abort. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(query-guard): stop setup on immediate abort, detach abort listener Address CodeRabbit review: - When abortSignal is already aborted before the dialog is shown, cancel and return so setup never enqueues a prompt (pushToQueue / bridge / channel / hook) that is immediately stale. - Detach the external abort listener on any normal terminal resolution so a resolved prompt doesn't retain a closure on the query-scoped abort signal. - Test: assert pushToQueue is not called for a pre-aborted prompt. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(query-guard): dequeue prompt on external abort; indent guarded setup Address review: - [P2] External abort (now-priority/bridge interrupt, backgrounding) resolved the permission but never removed the queued ToolUseConfirm. Since the REPL derives focusedInputDialog from toolUseConfirmQueue[0] and only UI actions call onDone, the stale dialog could stay focused after the aborted turn and interfere with foreground work. onExternalAbort now calls ctx.removeFromQueue() (no-op on the immediate-abort path, before the dialog is pushed). - [P3] Indent the setup body inside the try added for synchronous-throw safety, matching the surrounding two-space block style so the catch scope is visible. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(query-guard): clean up partial setup in the catch path Address CodeRabbit review: if setup throws after the abort listener is registered or after pushToQueue() succeeds, the catch now detaches the listener and dequeues the prompt before rethrowing, so failed setup can't leave stale UI/listeners behind. Moved removeExternalAbortListener out of the try so catch can reach it. Trim over-explanatory comments. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(query-guard): cancel bridge/channel prompts on external abort Address review: onExternalAbort resolved/dequeued the local prompt but skipped the bridgeCallbacks.cancelRequest(bridgeRequestId) and channelUnsubscribe() cleanup the normal allow/reject/abort paths do. With the bridge response handler unsubscribed by the abort listener, the remote UI could keep showing a stale prompt whose reply is ignored. Moved onExternalAbort below the bridgeRequestId/channelUnsubscribe declarations (so the immediate-abort branch can reach them without a TDZ) and mirror the local cleanup there. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(query-guard): keep lease deadlines active during human waits --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
0xfandomandGitHub
|
06e0ae6e0b |
feat(settings): per-model context_window and max_output_tokens overrides (#1234)
* feat(settings): per-model context_window and max_output_tokens overrides
Adds a `modelLimits` settings.json map so users can declare context window
and max output tokens for OpenAI-compatible models that are not in the
built-in catalog. Resolution order is env var → settings → catalog, so the
existing CLAUDE_CODE_OPENAI_CONTEXT_WINDOWS / _MAX_OUTPUT_TOKENS env vars
keep priority. Keys are matched exactly, then by prefix, with optional
"<host>:<model>" host-qualified forms.
Example:
"modelLimits": {
"qwen3.6-plus": { "contextWindow": 1048576, "maxOutputTokens": 32768 }
}
Closes #478
* fix(model-limits): apply settings overrides in resolveModelRuntimeLimits
The settings.json `modelLimits` fallback was only wired into the scalar
getOpenAIContextWindow / getOpenAIMaxOutputTokens helpers. The runtime
resolution path (resolveModelRuntimeLimits) instead consumes the *Matches
variants, which only returned env-var matches — so a configured modelLimits
override never reached the model whose limits are actually resolved at
runtime, contrary to the documented env → settings → catalog order.
Fold the settings lookup into getOpenAIContextWindowMatches /
getOpenAIMaxOutputTokenMatches as a dedicated `settings` field, and insert it
between the exact env override and the built-in catalog in
resolveModelRuntimeLimits. Declare `modelLimits` on GlobalConfig so the lookup
is typed. Add an integration test that drives resolveModelRuntimeLimits and
asserts settings resolve for exact, host-qualified and prefix keys, and that an
env override still wins.
* fix(model-limits): resolve modelLimits from settings.json, not global config
readSettingsLimits read getGlobalConfig().modelLimits, which is ~/.openclaude.json
— a different file from the settings.json the feature is documented and schema'd
for (SettingsSchema in settings/types.ts). A user adding modelLimits to
settings.json saw no effect. Read it from getInitialSettings() (the merged
settings snapshot, session-cached) instead, and drop the now-unused modelLimits
field from GlobalConfig so the override lives in one place. Rewire both test
suites to a gated-passthrough getInitialSettings mock.
* test(model-limits): capture real settings module at load, not in beforeEach
The settings modelLimits suites re-imported the real settings module with
a cold dynamic import inside beforeEach. That import sat right at Bun's
default 5s hook timeout and intermittently failed the first test. Capture
the genuine module once via a top-level query-string-busted static import
so the cost moves to module load (no hook timeout) and the gated
passthrough still bypasses other suites' settings.js mocks. Also correct
the integration-test comment to name the helpers the runtime path
actually calls (getOpenAIContextWindowMatches / getOpenAIMaxOutputTokenMatches).
* fix(model-limits): keep env-prefix override above settings in runtime resolver
resolveModelRuntimeLimits ordered the settings `modelLimits` value above
the env-prefix match, so a broad env-prefix override (e.g.
`{"my-custom":N}`) was silently overtaken by a more specific settings
entry (`my-custom-deployment`). The scalar getOpenAIContextWindow treats
env (exact ?? prefix) as strictly higher priority than settings; mirror
that in the runtime resolver: env.exact, then env.prefix, then settings,
then catalog/cache/descriptor. Regression covers both contextWindow and
maxOutputTokens.
* docs(model-limits): document the settings.json modelLimits override
Add a user-facing section near the env-var overrides covering the
modelLimits map, JSON shape, exact/prefix/host-qualified key matching, and
the env > settings > catalog > descriptor precedence.
* fix(model-limits): keep catalog above env-prefix in runtime precedence
The previous reorder put the env-prefix match above the catalog value,
which broke the existing invariant that a `:cloud` catalog variant takes
its known catalog limit rather than inheriting a broad base-model env
prefix (deepseek-v4-pro:cloud regressed to 262144). Correct order:
exact env -> catalog/cache -> env prefix -> settings -> descriptor. This
still keeps settings strictly below env-prefix (the original drift fix)
while preserving catalog precedence over prefix. Docs precedence updated
to match.
* docs(advanced-setup): document CLAUDE_CODE_OPENAI_MAX_OUTPUT_TOKENS env var
The modelLimits section referenced the max-output env var but the
Environment Variables table only listed the context-window one, leaving
output-only configuration undocumented. Add the matching table row.
Refs #478
* docs(model): align openaiContextWindows precedence comments with the resolver
The module header and the OpenALimitOverrideMatches.settings comment claimed a
resolution order of "env → settings → catalog", which contradicts
resolveModelRuntimeLimits (exact env → catalog/discovery cache → prefix env →
settings modelLimits → descriptor default). Since this module only produces the
override candidates and does not own the precedence, narrow the comments to say
so and point at runtimeMetadata.ts as the authoritative chain, so this
precedence-sensitive code isn't misread when touched again.
* fix(model): rank host-qualified modelLimits keys above bare model keys
lookupByModel grouped all exact matches (host-qualified and bare) ahead of all
prefix matches, so a bare exact key like `qwen3.6-plus` beat a host-qualified
prefix like `openrouter.ai:qwen3`. That defeated the advertised per-endpoint
disambiguation for versioned model families. A host-qualified key is strictly
more specific than a bare one, so rank both host-qualified forms (exact and
prefix) in the high-priority tier ahead of the bare exact match, leaving only
the bare prefix in the low-priority tier.
* fix(model): keep an exact modelLimits key ahead of any host-qualified prefix
The previous commit ranked host-qualified PREFIX matches above bare exact
matches, which broke the deliberate precedence in context.test.ts: an exact
`gpt-4o` limit was overridden by an unrelated `api.foo.com:gpt-4` prefix that
only matches a shorter, different model name.
Restore the tiering so an exact match (host-qualified or bare) always beats a
prefix, and a host-qualified key beats a bare key WITHIN the same match kind.
The supported way to set a different limit for the same model per endpoint is a
host-qualified EXACT key. Narrow the regression + comment to that behavior.
* docs(model): align modelLimits matching wording with exact-over-prefix rule
Narrow the advanced-setup wording so a host-qualified key only wins over a bare
key within the same match kind (a bare exact key still beats a host-qualified
prefix), matching lookupByModel's exact ?? prefix behavior; per-endpoint limits
for the same model need host-qualified exact keys. Also note modelLimits as part
of the documented user-override layer in the integration add-model and
common-pitfalls guides.
* docs(model): clarify modelLimits host-port key and catalog/cache precedence
Spell out that the host-qualified key uses new URL(baseUrl).host — including the
port when present (localhost:4000:my-model, not localhost:my-model) — and split
the precedence line so the built-in catalog is shown as checked before the
discovery-cache value, matching resolveModelRuntimeLimits.
|
||
|
0xfandomandGitHub
|
de751f369c |
fix(editor): guard editor-override lookup against prototype keys (#1915)
editFileInEditor resolved the editor command with EDITOR_OVERRIDES[editor] ?? editor, where editor comes from $VISUAL / $EDITOR (arbitrary strings). For a name that collides with an Object.prototype member — constructor, __proto__, hasOwnProperty, toString — the bare lookup returns the inherited member (a function / Object.prototype), which is non-nullish, so the '?? editor' fallback is defeated and editorCommand becomes a stringified function rather than the literal editor name; execSync then runs a corrupted command instead of the editor the user named. Extract resolveEditorCommand and gate the lookup on Object.hasOwn so unknown/proto names fall through to the literal name. |
||
 BogdanandGitHub
|
ae9a765fb5 | fix(env): align WebSearch and Ollama env docs (#1904) | ||
|
3b41cf3adb |
fix(command-semantics): cover remaining linter runner exits (#1700)
* fix(command-semantics): cover remaining linter runner exits * fix(command-semantics): handle env prefixes and PowerShell chains * fix(command-semantics): preserve runner and pipeline failures * fix(command-semantics): narrow setup and runner failure guards * fix(command-semantics): catch real setup failure stderr * fix(command-semantics): preserve setup failures with output * fix(command-semantics): parse inline env split strings * fix(command-semantics): inspect Bash merged failure output * fix(command-semantics): align PowerShell failure parsing * test(web-search): stabilize Brave timeout assertion * fix(command-semantics): cover package scripts and wrapper failures * fix(command-semantics): handle script run forms and PS call operator * fix(command-semantics): cover package prefixes and npm errors * fix(command-semantics): stabilize brave timeout and ps flags * test(websearch): assert brave timeout aborts fetch signal * fix(command-semantics): handle tsc diagnostic exit 1 * fix(command-semantics): preserve script diagnostics * fix(command-semantics): guard silent skipped diagnostics * fix(command-semantics): tighten setup failure guards --------- Co-authored-by: jatmn <the@jat.mn> |
||
 
|
9a53290588 |
feat(aimlapi): add guided top-up and key provisioning (#1886)
* fix(aimlapi): send valid rebate partner id (part_62yQ…) instead of literal 'Gitlawb' * feat(aimlapi): add guided top-up and key provisioning * fix: restore accidentally removed OpenGateway badge * fix(aimlapi): restore preset order and harden topup polling/logging * fix(aimlapi): validate --method choices instead of silently defaulting to card * feat(aimlapi): guided top-up and API key provisioning * fix(aimlapi): point non-interactive credential error at existing flags * docs(aimlapi): document guided top-up alongside the existing-key path --------- Co-authored-by: Lookoff123 <bataryshkinairina@gmail.com> |
||
|
BogdanandGitHub
|
e204d5ad36 |
feat(doctor): add WebSearch backend diagnostics (#1884)
* feat(doctor): add WebSearch backend diagnostics * fix(doctor): tighten Firecrawl cloud URL diagnostics * fix(firecrawl): align cloud URL detection * test(websearch): stabilize Brave timeout assertion * fix(firecrawl): handle bare cloud host casing * fix(doctor): align WebSearch auto diagnostics with fallback * fix(doctor): align custom preset diagnostics |
||
 
|
780f703747 |
fix(installer): gate native-binary install behind NATIVE_PACKAGE_URL (#1838)
* fix(installer): gate native-binary install behind NATIVE_PACKAGE_URL openclaude install inherited the upstream native installer, which downloads the first-party Claude Code binary from the GCS bucket, symlinks ~/.local/bin/openclaude to it, and uninstalls the npm package the user is running. Gate every native-installer surface behind hasNativeDistribution() so npm-only builds never touch the native path; setting NATIVE_PACKAGE_URL at build time re-enables it unchanged. Also gate background cleanupOldVersions(): the versions/staging/locks directories under ~/.local/share/claude (etc.) are shared with a coexisting first-party native Claude Code install, and the protection logic only recognizes our own launcher symlink — an npm-only build kept only the newest VERSION_RETENTION_COUNT binaries and could delete the version a user's pinned `claude` launcher still points to. Co-Authored-By: OpenClaude <openclaude@gitlawb.com> * fix(installer): keep npm-only guidance on npm paths * test(installer): share macro mock helper * fix(installer): clean stale native launcher in npm fallback * fix(update): clean stale native launcher in slash update * test(update): reuse shared macro helper --------- Co-authored-by: OpenClaude <openclaude@gitlawb.com> Co-authored-by: jatmn <the@jat.mn> |
||
|
|
fb1137275a |
feat: add ultrathink keyword detection and ultracode effort level (#1551) (#1630)
* feat: add ultrathink keyword detection and ultracode effort level Rebased onto current main so the diff contains only these changes — #1780's model-level effort routing now comes from main rather than being duplicated. - ultrathink: a `\bultrathink\b` keyword in a prompt injects a high-effort reminder, gated behind the isUltrathinkEnabled() rollout flag. - ultracode: a new session-only EffortLevel that maps to xhigh (or high) on the wire and grants a standing multi-agent orchestration permission. First-party only, suppressed under a per-agent providerOverride, and gated to xhigh-capable models. Honors CLAUDE_CODE_EFFORT_LEVEL precedence across the API path, the permission attachment, and the display surfaces; rejected from every agent-definition input (markdown/skill/plugin frontmatter, SDK, and JSON). Closes #1551 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(effort): clamp ultracode display availability * fix(effort): report effective effort overrides * test(model): avoid catalog-dependent effort label * fix(model): resolve current effort against session model * fix(spinner): resolve effort suffix against session model --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: jatmn <the@jat.mn> |
||
|
0xfandomandGitHub
|
aac2d8cdc8 |
fix(openai-shim): guard tool-arg field lookup against prototype keys (#1880)
STRING_ARGUMENT_TOOL_FIELDS is a plain-object lookup table keyed by a
provider-supplied tool-call name. hasToolFieldMapping used `name in table` and
getPlainStringToolArgumentField used a bare `table[name]` lookup, both of which
resolve inherited Object.prototype members. A tool call whose function.name is
'constructor', 'toString', '__proto__', etc. therefore reported a field mapping
(the Object constructor function), which is truthy, so the `?? null` fallback
never fired and normalizeToolArguments wrapped a JSON-encoded string argument
into a garbage-keyed object ({ 'function Object() { [native code] }': value })
instead of passing it through. Gate both lookups on Object.hasOwn, matching the
codebase convention for provider-keyed maps.
Consumers: src/services/api/openaiShim.ts (normalizeToolArguments /
hasToolFieldMapping over decoded tool_calls[i].function.name).
|
||
 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. |
||
|
0xfandomandGitHub
|
a46046ee90 |
fix(proxy): bypass subdomains for a bare NO_PROXY domain entry (#1848)
* fix(proxy): bypass subdomains for a bare NO_PROXY domain entry shouldBypassProxy matched a bare NO_PROXY entry (`example.com`) against the request host exactly, so a subdomain like `api.example.com` was routed THROUGH the proxy. But this module also drives undici's EnvHttpProxyAgent via getProxyAgent, and that path bypasses subdomains for a bare entry (matching the curl/Go/deno NO_PROXY convention). The result was that the same `NO_PROXY=example.com` produced different decisions for the same host depending on transport: bypassed on the fetch/undici path, proxied on the axios/WebSocket path. Match a bare domain against the host and any subdomain, aligning both paths. A lookalike such as `notexample.com` still does not match (the leading dot is required), and an IP-address entry can never gain a spurious subdomain match. * fix(proxy): align port-qualified and wildcard NO_PROXY entries with undici Two more spots where shouldBypassProxy diverged from the undici EnvHttpProxyAgent path this module also drives via getProxyAgent: - Port-qualified entries compared the whole `host:port` string, so `NO_PROXY=example.com:8080` bypassed `example.com:8080` but proxied `api.example.com:8080`. Parse the port off first (mirroring undici's `/^(.+):(\d+)$/`), require it to match, then apply the same exact-or-subdomain host predicate. - Leading-wildcard entries like `*.example.com` were treated as literal hostnames, so they never matched a subdomain. undici strips the leading `*.` (upstream proxy lists use `*.githubusercontent.com`); normalize it to the leading-dot suffix form before matching. Extends the regression coverage with the `api.example.com:8080` and `*.example.com` cases. * test(proxy): document IP-literal NO_PROXY exact-match boundary An all-numeric final label (e.g. 10.1.2.3.4) is not a valid WHATWG URL, so such a host never reaches the bare-domain subdomain-suffix arm; add coverage that the would-be dotted-IP subdomain of a bare IP entry does not bypass, plus a bracketed IPv6 exact-match case. |
||
 
|
67bebbdaca |
fix(bg): match background-session command args on token boundaries (#1834)
* fix(bg): match background-session command args on token boundaries commandLineContainsArgs matched each stored launch arg as a raw substring, so a selector like "1642" was satisfied by an unrelated live token "16420" in the same position. A reused PID whose command line merely contained the stored digits could therefore keep a dead session classified as running, and the kill path could target the wrong process. Match each stored arg against a whole whitespace-delimited token, in order, instead of a substring. Add regression coverage through isBackgroundSessionProcessAlive for the collision, the exact-token match, the session-id match, and the dead-process case. Refs #1770 * fix(bg): match the session id on token boundaries and span multi-word args Address review on #1770: - commandLineMatchesBackgroundSession matched session.sessionId with a raw includes(), so a short id like 'sess-1' matched an unrelated live command containing 'sess-100' and kept a reused-PID session classified as running. Route it through the same whole-token matcher. - commandLineContainsArgs required each stored arg to equal a single token, so a prompt stored as one argv entry (e.g. 'refactor auth') never matched the words ps renders it as, breaking --from-pr/resume launches that rely on the stored command. Expand each arg into its own tokens and match the flattened sequence as an ordered subsequence. Add regression tests: session-id substring collision stays dead, exact id token is alive, and a multi-word prompt arg matches across command tokens (all proven fail-on-old). * fix(bg): require background-session args to match as a contiguous token run The token-boundary matcher for background-session process identity treated the stored argv tokens as an ordered subsequence of the live command line, so unrelated tokens between matches were skipped. A reused PID whose command interleaves the stored tokens (e.g. stored [node, openclaude, 1642] against "node attacker openclaude extra 1642 --serve") was still classified alive, keeping the wrong-process kill risk open for token-insertion collisions. Match the flattened stored argv as one contiguous whole-token run instead. Leading interpreter path and trailing flags still match; interspersed tokens no longer do. Add a regression test asserting an interspersed-token command line is rejected (fails on the prior subsequence matcher, passes now). Refs #1770 * fix(bg): trim quotes so Windows quoted command lines match stored argv Get-CimInstance CommandLine returns the raw, quoted Windows command line, so a whitespace split fuses quotes onto the edge tokens ("C:\Program, node.exe", "refactor, auth") while session.command stores those values unquoted. The contiguous whole-token run therefore never matched, and a live non-forked --from-pr resume (whose only identity path is the stored command) was marked stale, letting kill <id> report success without signalling the process. Trim a single surrounding quote from each token before comparing; POSIX ps output is unquoted so this is a no-op there and preserves the #1770 token-boundary guard. --------- Co-authored-by: 0xghost42 <nikhilbajajj01@gmail.com> |
||
|
a5b277971d |
feat(settings): add settings-based subscription override and agy terminal support (#1731)
* feat(settings): add settings-based subscription override and agy terminal support * feat(provider): rebrand Gemini wizard setup steps to Google AI / Gemini * fix(settings): restrict subscription override to trusted sources Addresses jatmn's CodeRabbit review on #1731. P1 — untrusted settings sources could spoof subscriber state: Reads subscriptionType only from policy/flag/user/local settings, excluding project and repository settings which can be checked into shared repos. Adds getTrustedSubscriptionType() helper used by both isClaudeAISubscriber() and getSubscriptionType(). P2 — subscriptionType: "free" did not short-circuit OAuth-detected subscriber state. With a valid Claude AI OAuth token and subscriptionType: "free" in settings, isClaudeAISubscriber() still returned true. Now "free" is authoritative and returns false. P3 — test isolation: added afterEach(mock.restore()) to auth.test.ts so Bun's module mocks cannot leak into subsequent tests. Also splits the antigravity askpass test into focused cases for "agy" and "antigravity" substrings. * fix(settings): restrict subscriptionType override to user settings and return false early for free * fix(auth): clean up unused import and return type matching in tests * fix(provider): align Gemini chooser copy with the wizard's actual auth methods Addresses jatmn's review on #1731. The /provider chooser advertised "Google AI / Gemini Subscription" and "Use your Google AI Premium plan, …", but the Gemini setup wizard only offers three auth methods — API key, access token, and local ADC — with no subscription/Premium sign-in flow. Drop the "Subscription" label and "Google AI Premium plan" framing so the chooser matches the wizard (and the "Google AI / Gemini" wording used elsewhere in the file). No OAuth/subscription sign-in is planned: using Google AI/Gemini subscriptions via third-party tools violates Google's Terms of Service. * fix(pr1731): resolve review follow-ups * fix(pr1731): close review follow-ups * fix(pr1731): close antigravity and gemini follow-ups --------- Co-authored-by: jatmn <the@jat.mn> |
||
    GravireiGitHubGravireiClaude Opus 4.6coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>openhands |
aa936cda11 |
Centralize credential redaction in src/utils/redaction.ts + channel gate tests (#1711)
* feat(utils): add centralized redaction utility Single source of truth for stripping API keys, tokens, and other secrets from strings and JSON. Provider env-var coverage is generated from getKnownProviderSecretEnvKeys() so adding a new provider cannot silently create an unredacted path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(Feedback): import redactSensitiveInfo from utils Remove the inline 40-line regex implementation in favor of the centralized redaction utility, eliminating drift between Feedback and the transcript share path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(submitTranscriptShare): import redactSensitiveInfo from utils Update import path to point at the centralized utility instead of the Feedback component. Removes the implicit re-export contract that required Feedback.tsx to keep redactSensitiveInfo exported. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(log,debug): redact secrets in default error and debug output Wire the centralized redaction utility into logError and logForDebugging so secrets cannot leak into in-memory error logs or the debug file even if a caller forgets to pass through redactSensitiveInfo. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(api/logging): redact error message in logAPIError Apply the centralized redaction utility to the error string passed to logEvent so analytics events cannot capture unredacted credentials from upstream API failures. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: resolve merge conflict from upstream sync Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(channelNotification): allow null in getEffectiveChannelAllowlist signature ChannelsNotice.tsx passes getSubscriptionType() which returns SubscriptionType | null, but the signature only accepted string | undefined. Widen to string | null so the call site typechecks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(redaction): exclude specific token fields from redaction process * fix(redaction): lower AIza minimum length to {10,} Real GCP/Gemini keys are 39 chars total (4 prefix + 35 suffix), but the {35} suffix bound missed short tokens like 'AIzaSyDUMMY-secret-token' (21 chars after AIza). Lower to {10,} to match the diagnostics module and catch any AIza-shaped value. Same precision trade-off the diagnostics redaction makes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(redaction,log): address review feedback - Drop quotes from ANTHROPIC/OPENAI key negative lookarounds so JSON-shaped values like "sk-ant-..." redact. - Add private_key pattern to GENERIC_HEADER_FIELD_PATTERN and privatekey to SENSITIVE_FIELD_SUBSTRINGS. - logError now builds a sanitized Error (redacted message + stack) before passing to the sink and queue, not just the in-memory log. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(redaction): consolidate into single module + add channel gate tests Address the three P2 review findings on the central-redaction PR: [1] Consolidate four redaction modules into src/utils/redaction.ts. Previously lived in: - src/utils/redaction.ts (logs/bug reports/transcript shares) - src/utils/urlRedaction.ts (URL display) - src/utils/statusRedaction.ts (/status output) - src/utils/diagnostics/redaction.ts (doctor reports) The four surfaces share the same regex set / credential lists but had drifted into separate per-domain files. Merged into one module; deleted the three shim files. Updated six direct consumers (openaiShim.ts, ProviderManager.tsx, status.tsx, requestSizeBreakdown.ts, diagnostics/issueReport.ts, scripts/system-check.ts) and three test files to import from redaction.js. [2] Add gateChannelServer() test coverage. src/services/mcp/channelNotification.test.ts: 13 cases for the six gate paths (capability, runtime, session, marketplace, plugin allowlist, server-entry dev) plus end-to-end register. Mocks channelAllowlist.js (GrowthBook-backed) so tests stay independent of feature-flag state. [3] Apply jsonRedactor in transcript share. src/components/FeedbackSurvey/submitTranscriptShare.ts now does redactSensitiveInfo(jsonStringify(data, jsonRedactor)) — the key-aware redaction applies during serialization, and the text pass stays as defense in depth for free-form fields. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test(channelNotification): cover findChannelEntry multi-candidate branch Regression test for the disambiguation path in `findChannelEntry` (channelNotification.ts:201-230): when two same-name plugin entries exist in the allowed-channels list with different marketplaces, `pluginSource` must select the matching entry before the marketplace and allowlist gates evaluate. Without this branch being exercised, the gate could lock onto whichever entry sorts first and either skip the user's real installation or wrongly authorize a typo-squatted one. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(redaction): align URL fallback regex + add path-prefix boundary check Two related redaction correctness fixes: [1] URL fallback regex covers the same parameter set as the primary path. The malformed-URL branch in `redactUrlForDisplay` previously had a hand-rolled alternation of credential parameter names that could drift behind `SENSITIVE_URL_QUERY_PARAM_TOKENS`. New `MALFORMED_URL_PARAM_PATTERN` derives from that same list, so the two paths can never diverge. Tests cover the full credential set (`api_key`, `access_token`, `refresh_token`, `signature`, `sig`, `secret`, `password`, `apikey`) plus a non-sensitive `model` that must survive. [2] `redactPathForStatus` now requires a path-separator boundary after the home prefix. The previous `startsWith` check matched `/home/alice2/project` against `/home/alice` and emitted `~2/project`. The fix requires the character at `normalizedCandidate.length` to be `/` or `\` so `alice` no longer matches `alice2` or `alice.bak`. Test pins the false-positive paths and the true-positive (`/home/alice/project` → `~/project`). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(channel,redaction): restore dev-channel warning + align URL fallback Two related security fixes: [1] Restore DevChannelsDialog when --dangerously-load-development-channels is passed and the channels feature is enabled. The previous logic skipped the dialog when OAuth was absent, which was safe only while gateChannelServer() blocked no-OAuth sessions. With the OAuth/org- policy gates removed in this PR, an API-key session could pass the flag, skip the warning, and still register the dev channel. The only remaining skip is the genuinely-disabled feature case (`!isChannelsEnabled()`), where the dialog is moot. [2] Malformed-URL fallback now uses the same substring predicate as the primary `URL` parser path. The previous regex matched only exact parameter names (`api_key=`, `access_token=`, …), so `my_api_key=SECRET` and `x_access_token=TOKEN` slipped through unchanged even though `shouldRedactUrlQueryParam` flags them as sensitive. New `redactMalformedQuery` walks the query pairs and runs the predicate on each key. Three new tests cover prefixed keys, non-sensitive keys, and fragment preservation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(redaction): widen key boundary class + tighten dev-channel comment Two small follow-ups from the latest CodeRabbit review: [1] Boundary class on key-prefix patterns widened from `[A-Za-z0-9]` to `[A-Za-z0-9_-]` so a raw key embedded in a JSON string value (`"sk-ant-..."`, `"AIza..."`, `"ghp_..."`, etc.) is still caught. Quotes act as delimiters, not blockers — the previous boundary class was correct for unquoted text but let quoted keys slip through. [2] Tighten the dev-channel dialog comment in interactiveHelpers.tsx so future readers don't misread the security boundary. Skip condition is `isChannelsEnabled()` (the channels feature flag gate), not KAIROS / KAIROS_CHANNELS as the previous wording implied. Comment now matches the code. Skipped with reason: - getEffectiveChannelAllowlist divergence from gateChannelServer allowlist — by design; the effective-list override is a UI hint consumed only by ChannelsNotice for the org-override indicator. Trust boundary is enforced by gateChannelServer() reading the hardcoded ledger. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(redaction,channel): address P1/P2 review findings P1 - malformed URL fallback secrets: - Decode percent-encoded query param keys via decodeURIComponent() before applying shouldRedactUrlQueryParam (e.g. %74oken -> token) - Stop userinfo regex at ? and # delimiters to avoid consuming query params when matching @ signs in email addresses or fragment delimiters P2 - channel notice/gate allowlist sync: - Remove org override path from getEffectiveChannelAllowlist() so ChannelsNotice startup guidance uses the same ledger source as gateChannelServer's runtime enforcement - Simplify ChannelsNotice to drop unused sub/policy params and the source === 'org' conditional * fix(channel): apply marketplace matching to permission relays, remove stale OAuth/org-policy blockers, add dev-channel dialog coverage P1: Thread runtime pluginSource through filterPermissionRelayClients so findChannelEntry disambiguates same-name plugin entries from different marketplaces before sending permission request previews. P2: Remove stale noAuth and policyBlocked branches from ChannelsNotice that would render '--channels ignored' before reaching the listening message, confusing non-OAuth users. P2: Add test coverage that mocks isChannelsEnabled() both true and false, verifies DevChannelsDialog appears with onAccept marking entries dev:true in the enabled case, and verifies the disabled branch registers entries directly without dialog. * test(dev-channel): clarify count assertion comment + add afterEach with mock.restore() * fix(channel): mirror marketplace gate in permission relay + restore mock Two follow-ups from the latest review: [1] Permission relay predicate no longer relies on findChannelEntry alone. After resolving the entry, the predicate now requires a runtime pluginSource whose marketplace matches the session entry's marketplace for plugin-kind entries — mirroring the gateChannelServer check at channelNotification.ts:303-312. A `plugin:slack@evilcorp` client whose session allows `plugin:slack@anthropic` is now rejected instead of piggy-backing on the approved entry to receive permission-request previews. Server-kind entries still match on bare name. [2] bugfixes.test.ts now re-registers the real channelAllowlist module in afterEach via a cache-busted reference, so the neighbor channelNotification.test.ts continues to import getChannelAllowlist after this suite runs. mock.restore() does not clear module-level mock.module() overrides in bun (the registry is process-global). Pattern matches compact.test.ts:27-36. Also expanded the dev-map count comment in bugfixes.test.ts to document the security invariant (a dev entry must never be confused with a production entry in the allowlist check) per CodeRabbit's request. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(redaction): consolidate into single module + add channel gate tests Address the three P2 review findings on the central-redaction PR: [1] Consolidate four redaction modules into src/utils/redaction.ts. Previously lived in: - src/utils/redaction.ts (logs/bug reports/transcript shares) - src/utils/urlRedaction.ts (URL display) - src/utils/statusRedaction.ts (/status output) - src/utils/diagnostics/redaction.ts (doctor reports) The four surfaces share the same regex set / credential lists but had drifted into separate per-domain files. Merged into one module; deleted the three shim files. Updated six direct consumers (openaiShim.ts, ProviderManager.tsx, status.tsx, requestSizeBreakdown.ts, diagnostics/issueReport.ts, scripts/system-check.ts) and three test files to import from redaction.js. [2] Add gateChannelServer() test coverage. src/services/mcp/channelNotification.test.ts: 13 cases for the six gate paths (capability, runtime, session, marketplace, plugin allowlist, server-entry dev) plus end-to-end register. Mocks channelAllowlist.js (GrowthBook-backed) so tests stay independent of feature-flag state. [3] Apply jsonRedactor in transcript share. src/components/FeedbackSurvey/submitTranscriptShare.ts now does redactSensitiveInfo(jsonStringify(data, jsonRedactor)) — the key-aware redaction applies during serialization, and the text pass stays as defense in depth for free-form fields. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(test): align malformed URL fragment expectation with preservation behavior * fix: address review findings P1 and P2 [P1] Enforce dev flag for server-kind entries in permission relay predicate, matching gateChannelServer() behavior. Add coverage for both dev and non-dev server relay paths. [P2] Drop fragments in malformed URL fallback (redactMalformedQuery) to match the valid-URL path, preventing credential leaks via fragment-carried tokens. Update existing tests and add regression for fragment-only malformed URLs. * test(relay): add plugin-kind marketplace regression tests * fix: address review findings P1 and P2 [P1] Add PEM private key redaction pattern to redactSensitiveInfo so multi-line PEM values are fully consumed instead of leaking after the first whitespace. Add [ to generic header pattern's value exclusion set to prevent re-consuming [REDACTED] tokens. [P2] Use truthy check (Boolean()) for claude/channel capability in filterPermissionRelayClients to match gateChannelServer's behavior, rejecting explicit false capabilities. * fix(debug): redact before JSON-stringify multiline messages Reorder logForDebugging so redactSensitiveInfo runs before jsonStringify, ensuring PEM/private-key patterns match the raw (unescaped) message text rather than the JSON-encoded form where colons and quotes are escaped. * test(debug): add end-to-end regression for multiline PEM redaction in logForDebugging Uses mock.module on process.js to capture stderr output and exercises the full logForDebugging path with multiline PEM private_key input, verifying the redact-before-JSON-stringify ordering produces redacted output. * fix(test): preserve original process.env.DEBUG and process.argv in logForDebugging test hooks * fix: address PR review findings P1-P3/P5-P7 - P1: clear isDebugMode/isDebugToStdErr memoize caches in test beforeEach + cache-busting query param for fresh debug.ts imports - P2: restore mock.module afterAll instead of leaking mock + mutate err in-place in logError to preserve name/cause - P3: post-processing regex absorbs trailing bracket content after [REDACTED] - P5: (was P3) expand jsonRedactor EXCLUDED_KEYS for maxTokens etc. - P7: capture HOME/USERPROFILE per-test instead of at module scope * fix: address CodeRabbit review findings - interactiveHelpers.tsx: update dev-channel comment — OAuth/org-policy gates removed from gateChannelServer(), org policy is not enforced - channelNotification.test.ts: add afterAll mock.restore() to clean up process-global channelAllowlist.js mock - channelNotification.ts: fix comments — isChannelsEnabled() still reads tengu_harbor, not always true - log.ts: sanitize err.message and err.stack separately so message doesn't get replaced with full stack trace - redaction.ts: add 'i' flag to redactHomePath regex for Windows case-insensitive path matching * fix: address second review round - interactiveHandler.ts: [P2] redact input_preview via redactSensitiveInfo before sending to channel servers - log.ts: [P3] copy error via Object.assign(Object.create(err), err) before sanitizing instead of mutating in-place * fix: address CodeRabbit second round - channelPermissions.ts: redact before truncate in truncateForPreview so partial credentials don't leak at the 200-char boundary - interactiveHandler.ts: remove outer redactSensitiveInfo — now handled inside truncateForPreview - log.ts: derive errorInfo.error from already-sanitized sanitizedErr; fix Object.assign comment to accurately describe what is copies * fix: improve permission relay client filtering and enhance redaction functions * fix: address third review round (P1, P2, P3) - P1: update test expectations for [REDACTED_*] output format - P2: add total_tokens, prompt_tokens, completion_tokens to jsonRedactor EXCLUDED_KEYS - P3: remove ) and } from GENERIC_HEADER_FIELD_PATTERN value capture to prevent content leak after embedded parens - Fix buildKnownEnvVarPattern capture group to preserve env-var separator ([REDACTED]) - Add & to GENERIC_CREDENTIAL_ENV_PATTERN value exclusion to prevent URL query over-consumption * fix: address latest reviewer P2/P3 findings (errorLogSink redaction, X_API_KEY/AUTHORIZATION patterns, regression tests) * fix: address reviewer P1/P2 — bracketed values and multi-word header values - P1: Remove and from value captures in X_API_KEY_PATTERN, AUTHORIZATION_PATTERN, GENERIC_HEADER_FIELD_PATTERN, GENERIC_CREDENTIAL_ENV_PATTERN so bracketed secrets like are fully redacted instead of passing through unchanged. - P2: Widen header-style value captures to include spaces by removing from exclusions, using as delimiter (stops at newlines and URL query separators). Fixes multi-word leaks: , , , . - GENERIC_CREDENTIAL_ENV_PATTERN: add to negative lookbehind to prevent matching inside when the latter is already redacted. - GENERIC_HEADER_FIELD_PATTERN replacer: skip values starting with to preserve specific labels from earlier passes. - Add 7 regression tests covering both finding categories. * fix: address reviewer findings P1-P4 P1: Custom enumerable error properties now redacted in log.ts logError iterates all own enumerable properties on the original error and applies redactSensitiveInfo to string values and jsonRedactor to object values, preventing credential-bearing custom fields from leaking through the sanitized error. Regression tests added in log.test.ts. P2: Soften single-source-of-truth claim; migrate easy call sites Header comment in redaction.ts updated to acknowledge that specialized scanners (secretScanner.ts, xaa.ts) are intentional exceptions. src/services/mcp/client.ts and src/services/mcp/auth.ts now use jsonRedactor for header redaction instead of ad-hoc key checks. P3: Fix mock.restore cleanup in channelNotification.test.ts Cache-bust the real channelAllowlist module at describe-entry and re-register it in afterAll, following the pattern from bugfixes.test.ts. mock.restore alone does not clear mock.module overrides in Bun. P4: Remove unused ChannelGateResult kinds Removed 'auth' and 'policy' from the skip kind union and removed corresponding dead branches in useManageMCPConnections.ts. * fix: extract sanitizeError() to fix CI test fragility The logError tests were failing in CI due to parallel test execution racing on the module-level errorLogSink singleton. Extract the inline sanitization logic into an exported sanitizeError() helper and test that directly — it's pure, has no env-var or sink dependencies, and doesn't interact with shared mutable state. * fix: use Object.getPrototypeOf(err) instead of err as prototype in sanitizeError Object.create(err) sets the original error instance as the prototype of the sanitized copy, leaking non-enumerable own properties through the prototype chain. Use Object.getPrototypeOf(err) instead so the prototype is the error constructor's prototype (e.g. TypeError.prototype), preserving instanceof checks without exposing the original error's non-enumerable fields. Add a regression test verifying non-enumerable properties do not leak and update the prototype-chain test to assert Object.getPrototypeOf result. * fix: apply key-aware redaction and fail closed on non-serializable error props - String properties: use jsonRedactor(key, value) instead of redactSensitiveInfo(value) so keys like apiKey with innocuous values (e.g. 'my-key') are still caught via SENSITIVE_FIELD_SUBSTRINGS. - Object path: catch now replaces non-serializable/circular references with '[REDACTED]' instead of leaving the original object reference. - Add 2 regression tests for key-aware redaction and fail-closed behavior. * Update src/utils/log.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix: redact bare auth header keys in JSON/header objects - Add 'auth' to SENSITIVE_FIELD_SUBSTRINGS in src/utils/redaction.ts:109 to match URL/diagnostic redactors treatment of auth - Add regression test for bare auth header keys in src/utils/diagnostics/redaction.test.ts:88 Co-authored-by: openhands <openhands@all-hands.dev> * fix: narrow auth matching, redact nested transcript JSONL, fix channel skip message * fix: address CodeRabbit nits — comment, hint, JSONL fallback redaction * fix: key-aware malformed JSONL fallback and auth/x-auth in free-form text * fix: strengthen redactJsonLines trailing rest redaction and auth test assertions * fix: preserve non-JSON prefix in redactJsonLines fallback and redact it * fix: tighten redactJsonLines prefix test to exact output assertion * fix: redact MCP log sink payloads and errorStr before writing to disk * fix: address P1 findings — URL #-in-password, ;-delimited query params, split channel trust-boundary - Allow in URL userinfo password on malformed-URL fallback path (new URL() fails when password contains fragment delimiter). - Redact -delimited sensitive query params by splitting on both & and ; in redactMalformedQuery, plus redactSemicolonQueryParams post-processor for valid-URL output. - Restore channelNotification.ts to upstream/main to fully split OAuth/org-policy trust-boundary changes from credential redaction PR. * fix: update callers to match upstream/main function signatures channelNotification.ts was restored to upstream/main to split trust-boundary changes from the redaction PR. This commit updates the three caller sites that previously passed extra arguments: - ChannelsNotice.tsx: pass getSubscriptionType() + undefined to getEffectiveChannelAllowlist (needs 2 args upstream) - interactiveHandler.ts, channelNotification.test.ts: drop 3rd pluginSource arg from findChannelEntry (takes 2 args upstream) * fix: address reviewer findings — OAuth mock, notice states, marketplace disambiguation P1: Mock getClaudeAIOAuthTokens and getSubscriptionType in channel notification tests so they pass on CI where no real OAuth exists. P2: Restore blocked-auth/org-policy notice states in ChannelsNotice.tsx so the UI shows the correct blocker when gateChannelServer rejects unauthenticated users or orgs without channelsEnabled. P2: Add pluginSource disambiguation to findChannelEntry so same-name plugin entries from different marketplaces are matched by runtime source rather than first-match order. Add regression test with non-matching marketplace first to cover the bug. * fix: address reviewer findings — relay gate parity and allowlist regression test - Replace filterPermissionRelayClients in interactiveHandler with inline gateChannelServer call so the relay predicate checks ALL gates including disabled-channel, auth, org policy, and approved-plugin allowlist, not just session entry + marketplace. - Clean up unused imports (getAllowedChannels, parsePluginIdentifier, findChannelEntry, filterPermissionRelayClients). - Add regression test: gateChannelServer rejects marketplace-matched plugin not on approved allowlist (full-gate path). * fix: redact mixed semicolon secrets in valid-URL path and route OpenAI shim through centralized redactor P1: Pre-redact semicolon-delimited sensitive query params from the raw query string in redactUrlForDisplay BEFORE URLSearchParams encodes ; as %3B. Previously model=ok;token=SECRET leaked because parsed.toString() reserialized to model=ok%3Btoken%3DSECRET, making it invisible to the post-process pass. P1: Route openaiShim's redactUrlForDiagnostics through the centralized redactUrlForDisplay so the semicolon fix, malformed-URL fallback, and all future redaction improvements apply to OpenAI-compatible diagnostic logs too. Keep redactSecretValueForDisplay as an additional safety net after the centralized pass. Add 3 regression tests for mixed-separator queries. * Update src/utils/redaction.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix: add fragment-query credential regression test and correct dev-channel gate comments P2: Add regression test for redactUrlForDisplay with query-like credential in fragment (e.g. #debug?token=SECRET). Fix raw-query pre-processing to only extract query before the first #, preventing fragment content from being treated as query parameters. P3: Update comments in interactiveHelpers.tsx to match the actual gate order — OAuth and org-policy gates still exist in gateChannelServer() after restoring to upstream/main; the --dangerously-load-development- channels flag only bypasses the allowlist gate. * fix: add port+fragment+@ fallback test and restructure dev-channels dialog tests * fix: registerDevChannels seam, bare-host #-in-password heuristic, and coverage restructure * fix: add OAuth and org-policy gate test coverage - Refactor auth module mock to use mutable variables per test - Auth gate test: empty OAuth tokens -> kind:auth - Policy gate test: team subscription without channelsEnabled -> kind:policy * fix: prefer exact server channel entries before plugin disambiguation - Return exact server-kind candidate first when candidates include both server and plugin entries with same name - Added regression test covering mixed server/plugin --channels entries to ensure exact server opt-in is not overridden by plugin candidate - This prevents a plugin marketplace mismatch from incorrectly rejecting a server the user explicitly selected via server:plugin:slack * fix: only trust exact [REDACTED] placeholder in generic header field pattern - Changed GENERIC_HEADER_FIELD_PATTERN to only bypass exact '[REDACTED]' canonical placeholder - Prevents non-canonical placeholders like '[REDACTED_API_KEY]' or '[REDACTED_actual_secret]' from leaking through - Updated tests to expect canonical '[REDACTED]' output for generic pattern * fix: handle bare hosts in malformed URL userinfo fallback - Added regex to recognize bare hostnames (with optional port) in the fragment heuristic - Added tests for //alice:sec#ret@host and //alice:sec#ret@host:443 * fix: add relay dispatch path test for non-allowlisted plugin - Added test using full gateChannelServer predicate in filterPermissionRelayClients - Mirrors the exact relay dispatch path used in interactiveHandler - Ensures marketplace-matched plugin not on allowlist is excluded from permission preview * fix: enhance URL redaction logic to handle valid hosts before fragment * fix: refine URL redaction logic to ensure valid host checks before fragment * fix: enhance redaction logic to handle embedded URLs in free-form text * fix: update redaction logic to remove user info from OpenAI base URL in diagnostic report * fix: ensure findChannelEntry returns undefined when no exact matches are found * fix: improve URL redaction logic to remove user info and ensure proper formatting * fix: enhance redactDiagnosticUrl to preserve query-param values and trailing slashes * fix: refine redaction logic to preserve meaningful path segments and handle trailing slashes correctly * fix: enhance redactDiagnosticUrl to preserve literal path segments and handle trailing slashes correctly * fix: preserve semicolon-delimited query params during redaction * fix: update redaction logic to support semicolon-delimited query parameters * fix: enhance redactUrlForDisplay to handle bare hosts and improve fragment redaction * fix: enhance redactUrlForDisplay to correctly handle username-only userinfo with fragments * fix: address privacy findings — URL redaction in jsonRedactor, base URL redaction, diagnostic object collapsing, structural channel previews, pluginSource telemetry * fix: preserve falsey env-presence values in diagnostic redaction - false, "", and 0 under isEnvPresenceKey keys are now preserved as-is instead of misrepresented as "[set]" - Added regression test for absent/falsey env-presence inputs * fix: address CodeRabbit findings — sync describe, heartbeat emitter, responsesBody filtering, dev entry precedence * chore: remove stray Windows path artifact * fix: update redaction import path in taskReport module * fix: address CodeRabbit P1-P3 findings and rebase regressions - F1: rebase onto upstream/main, fix taskReport.ts import path - F2: Ollama native chat code recovered via rebase (6 functions) - F3: &-truncation in credential regexes fixed via post-processing pass - F4: 'tokens' added to jsonRedactor EXCLUDED_KEYS - F5: redactHomePath case-sensitivity aligned with redactPathForStatus - F6: credential metadata object preserved in issue report (sensitive-key check moved inside type branches) - F7: heartbeat tests updated for pre-drain write behavior - F8: reportTask test expects [REDACTED] (matches centralized output) - rm: stray C:\repo\ Windows path artifact * fix: address reviewer findings — generic regex &-handling and diagnostic secret-key masking - Remove & from excluded char classes in 4 generic patterns so they consume full secret values (URL-query &-splitting belongs in redactUrlForDisplay). - Remove now-obsolete &-tail post-processor pass. - Remove credential from DIAGNOSTIC_SECRET_KEY_PATTERN so issue report credential metadata objects are traversed, not collapsed. - Restore broad isDiagnosticSecretKey check before type dispatch in redactDiagnosticObjectInternal so objects/arrays under secret-marked keys (auth, password, token, etc.) are masked. - Update issue report test baseUrl expectation (no trailing &mode=test after generic redactor consumes past &). * fix: address reviewer findings — URL delimiter safety, jsonRedactor #-drop, embedded URL query redaction - Restore &#; delimiters in generic pattern value classes (F1) so safe query tails (&mode=test) survive. Re-add &-tail post-processor for non-URL abc&def case. - Gate redactUrlForDisplay in jsonRedactor to https?:// strings only (F2) to prevent #-drop on ordinary text like 'fails after #setup'. - Add URL query redaction step to redactSensitiveInfo (F3) that extracts https?:// URLs from free-form text and routes them through redactUrlForDisplay, catching signature/sig params that generic patterns miss. Skip already-redacted URLs to avoid double-redaction. * fix: add Cookie/Set-Cookie semicolon-safe redaction pass, tighten &-tail regex * fix: COOKIE_PATTERN consume comma-joined multi-cookie values * fix: address P2 findings — URL redact skip, pre-drain write promise, permission truthy check * fix: update log.test.ts expectation, add protocol-relative URL support * fix: enhance redaction for provider env-vars in URLs, preserve safe query params * fix: enhance redaction for uppercase provider keys and cookie query params * fix: enhance redaction for bare Bearer and JWT tokens in sensitive info * fix: update report task test expectations for new redaction format * fix: limit token exemption to numeric values, protect semicolon cookie query tails * test: add tests for truncateForPreview to ensure sensitive data redaction --------- Co-authored-by: Gravirei <gravirei@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: openhands <openhands@all-hands.dev> |
||
|
0xfandomandGitHub
|
c3db07f1f2 |
feat(codex-oauth): manual callback URL paste for SSH / remote sessions (#1288) (#1414)
* feat(codex-oauth): manual callback paste for SSH / remote sessions Codex OAuth required the browser to reach the openclaude host's localhost:1455 callback. On SSH / containerized installs that callback resolves to the user's workstation instead of the openclaude host, so the redirect lands on a dead URL and the CLI hangs. Add a manual-paste fallback (mirrors the xAI OAuth recovery path): after authorizing in the browser, the user copies the full redirected URL from the address bar and pastes it into the CLI. CodexOAuthService validates the state parameter against the in-flight flow, races the manual code against the loopback listener, and reuses the same authorization-code → token exchange. SSH_CONNECTION / SSH_CLIENT triggers a warning banner explaining why the loopback redirect failed; non-SSH sessions get a dim hint covering containerized / remote setups. Closes #1288 * test(codex-oauth): type manual-paste fetch mock via asMockFetch The raw `mock(...) as typeof fetch` cast no longer typechecks against the base branch's stricter `fetch` type (now requires `preconnect`). Route the manual-callback-paste test's mock through the shared `asMockFetch` helper, matching the other fetch mocks in this file. * fix(codex-oauth): mask the pasted manual callback URL input The manual-recovery field echoed the redirected callback URL verbatim, which carries the OAuth code and state query params — enough to complete the in-flight exchange. Mask it with mask="*", matching the adjacent xAI manual-code field, so it stays out of terminal scrollback, recordings, and shared sessions. * test(codex-oauth): bound state wait and cover hook manual-callback contract - Bound the while (!capturedState) wait in the manual-callback test with a 5s deadline so a regression fails with a clear assertion instead of hanging the suite. - Add a useCodexOAuthFlow test asserting the waiting status exposes submitManualCallback and delegates both success and failure results from the service back to the caller. * test(codex-oauth): stabilize onAuthenticated in the manual-callback test The new hook test passed a fresh inline `onAuthenticated: async () => {}` on every render, so the hook effect re-ran each render, restarting the flow and looping setStatus → render ("Maximum update depth exceeded" when run alongside ProviderManager.test.tsx). Hoist the callback to a stable reference, matching the other tests in the file. * test(codex-oauth): cover the ProviderManager manual-callback UI Add focused coverage for the waiting-state paste surface: the masked callback input renders, a good callback delegates to status.submitManualCallback with no inline error, the SSH banner appears when SSH_CONNECTION is set, and a rejected callback surfaces the hook's inline error. |
||
|
5afd4f4d10 |
fix(tokens): include attachments in incremental cache key (#800)
Co-authored-by: jatmn <the@jat.mn> |
||
|
0xfandomandGitHub
|
230888181a |
fix(websearch): match allowed/blocked domains case-insensitively (#1872)
hostMatchesDomain compared the request host against the caller's allowed_domains / blocked_domains entries with a raw ===/endsWith. The host is always lowercased (WHATWG new URL().hostname via safeHostname), but the domain entries come straight from tool input and are never normalized. A capitalized entry therefore never matched: - blocked_domains: ['Reddit.com'] silently failed to block reddit.com results. - allowed_domains: ['GitHub.com'] dropped every legitimate github.com hit. Hostnames are case-insensitive, so lowercase both operands before comparing. Lookalike hosts (notexample.com) are still rejected. |
||
|
0xfandomandGitHub
|
8d90849fe6 |
fix(diff): don't overcount new-file additions by the trailing newline (#1873)
countLinesChanged's new-file branch counted added lines as newFileContent.split(/\r?\n/).length. Content that ends in a newline (the normal case) yields a trailing empty element, so a 2-line file "a\nb\n" counted as 3 additions instead of 2. That inflated the tengu_file_changed telemetry and the running total-lines-changed counter for essentially every new file, and disagreed with the diff-based update path (which counts `+` hunk lines git-accurately). Drop the phantom trailing line when the content is newline-terminated so the count matches git and the update path. |
||
|
BogdanandGitHub
|
8599560b82 |
fix(websearch): add built-in provider request timeouts (#1874)
* Add timeouts for web search providers * Fix WebSearch timeout body-stall test * Address WebSearch timeout review feedback |
||
|
0xfandomandGitHub
|
db01038d5c |
feat(model-picker): surface inactive provider profiles in /model (#1119 piece 2) (#1164)
* feat(model-picker): surface inactive provider profiles in /model When a user configures multiple providerProfiles (Kimi + Z.AI + OpenRouter + SambaNova in the #1119 repro, but the pattern fits any multi-provider setup), switching the main session between them currently requires round-tripping through /provider — /model only shows the active profile's models. Make /model the single switcher: - ModelOption gains an optional `switchToProfileId`. Existing options leave it unset and behave exactly as today. - `getInactiveProviderProfileOptions` enumerates every configured profile that isn't the active one and emits a picker entry per model, labelled `<model> · <profile.name>` so the user can see the choice changes providers, not just models. - Each option's `value` is encoded with `__switch_profile__:<id>:<model>` so the picker's plain-string `value` channel stays the source of truth and same-named models under different base URLs (`gpt-4o` on multiple OpenAI-compatible endpoints) stay disambiguated. - /model's handleSelect detects the prefix, calls `setActiveProviderProfile` (same path /provider uses — applies env, persists active profile, refreshes startup file), then sets `mainLoopModel` to the bare model string. Only surfaces inactive options when `CLAUDE_CODE_PROVIDER_PROFILE_ENV_APPLIED` is set, so users who haven't opted into the multi-profile workflow at all don't see the affordance. Tests cover round-trip encoding (including OpenRouter-style colon-bearing model strings), the active-filter, the multi-model explosion, and that `getModelOptions()` 3P path includes the inactive options only when the profile env is applied. Combined invocation with the rest of `src/utils/model/` + `src/commands/model/` + `src/utils/providerProfiles.test.ts` runs clean to guard against mock-leak (per the 2026-04-30 lesson — spreads `import * as actual` for every `mock.module` factory). Refs #1119 * fix(model-picker): run fast-mode cleanup on cross-profile switch The new switch-profile branch returned before reaching the fast-mode reconciliation, so a user with fastMode latched on Anthropic Opus could switch to an OpenAI profile and silently keep fastMode on even though the new model can't support it. Extract the cleanup into a pure helper `reconcileFastModeForSwitch` and call it from both branches. Refs #1119. * fix(model-picker): decode cross-profile values before effort/display lookup Inactive-profile entries encode the picker value as `__switch_profile__:<profileId>:<model>`, but `resolveOptionModel` forwarded the raw string straight to `parseUserSpecifiedModel`. For a reasoning-capable cross-profile entry such as `gpt-5.4`, `modelSupportsEffort()` then saw the prefixed string and reported "Effort not supported", and `handleSelect` dropped the toggled effort even when the underlying model accepts it. Run `parseSwitchProfileValue` first; when it matches, hand the bare target model to `parseUserSpecifiedModel` so effort capability, default-effort lookup, and display-name resolution all key off the real model id. * fix(model-picker): include inactive profiles on local OpenAI-compatible scope The inactive-profile compute lived after the `getAdditionalModelOptionsCacheScope()?.startsWith('openai:')` early return, so users with a local OpenAI-compatible profile active (Ollama, lm-studio, any localhost endpoint) never saw the cross-profile switcher in `/model`. They still had to round-trip through `/provider` to change profile. Hoist `profileEnvApplied`, the active-profile lookup, and `getInactiveProviderProfileOptions(activeProfileId)` above the early return, and append `inactiveProfileOptions` to the local-OpenAI branch return value. Other branches (Claude.AI, MiMo, MiniMax, ant) were already either irrelevant or have their own gating. Test: new regression in modelOptions.crossProfile.test.ts pins `getAdditionalModelOptionsCacheScope` to an `openai:` value and confirms the inactive profile still surfaces with a parseable `__switch_profile__` value. * fix(model-picker): apply the allowlist to the decoded cross-profile model filterModelOptionsByAllowlist evaluated cross-profile options by their encoded __switch_profile__:<id>:<model> value, so an availableModels allowlist that permits the bare target (e.g. glm-5.1) dropped every inactive-profile entry. Check the allowlist against parseSwitchProfileValue(value)?.model ?? value, and cover both the allowed and denied cases. * fix(model-picker): only surface cross-profile switch options on the /model path The inactive-profile entries come from the shared getModelOptions() list, but only the /model command's onSelect decodes __switch_profile__ values and activates the target profile. The prompt hotkey and Settings pickers wrote the encoded value straight to mainLoopModel, sending an invalid model string. Gate these options behind a new allowProfileSwitch prop that only the /model command sets; inline pickers no longer surface an option they cannot honor. Also apply the org allowlist to the decoded target model in the /model select handler. * test(model-picker): drop flaky cross-profile allowlist case The decoded-allowlist assertion drove the org allowlist through the shared session settings cache, which is racy across bun's single-process run and could leak availableModels into sibling suites (the providerConfig cache-scope tests went red in CI). The decode itself is a one-line guard already exercised by the parseSwitchProfileValue round-trip coverage, so remove the unreliable case rather than ship CI flake. Also snapshot the real provider/auth modules before mocking so each harness call rebuilds its mock from a clean base instead of a previous test's overrides (bun live-repoints the imported namespace to the active mock). * test(model-picker): stop cross-profile mocks leaking into provider suites The cross-profile tests mock.module'd ../providerProfiles, ./providers, ../auth and ../../services/api/providerConfig per test. bun's mock.module is process-wide and mock.restore() does not undo it, so these persisted into later files — most damagingly the providerConfig mock, which replaced the module with a single-function stub and stripped resolveProviderRequest / getAdditionalModelOptionsCacheScope from providerConfig.local's suite (now adjacent after the rebase onto #1706). Install each mock once at module load, keep the full export surface, and gate the overrides on module-level flags cleared in beforeEach/afterEach so the persisted mocks are transparent passthroughs for every other suite. Same pattern as the cross-spawn / install-surfaces leak fixes. * fix(model): reconcile fast mode before activating the switched profile In the cross-profile /model switch path, reconcileFastModeForSwitch ran after setActiveProviderProfile. The reconciler gates on isFastModeEnabled(), which reads the *active* provider — so once the target profile is activated it reflects the new (fast-mode-less) provider and short-circuits to 'unchanged', leaving fastMode latched on for a model that can't use it. Compute the reconciliation before activating the profile, so it evaluates against the source provider and correctly returns 'off' for an unsupported target. Add a command-level regression test that drives handleSelect with a __switch_profile__ value while setActiveProviderProfile flips the fast-mode state, and asserts fastMode is set to false (it fails if the call order regresses). * fix(model): re-check fast mode after activating a switched profile The pre-activation reconcile gates on the source provider, so its 'on' result is stale when the target provider cannot run fast mode even though the target model name passes the source-side support check (e.g. a third-party shim exposing a claude-opus-* model). Re-evaluate isFastModeEnabled / supported / available after setActiveProviderProfile and force fastMode off when it is no longer genuinely supported. Add a command-level regression test for that path and wrap the cross-profile test cleanup in try/finally so a failing assertion still unmounts the Ink instance (jatmn review, #1119). * test(model-picker): cover cross-profile allowlist with isolated settings Re-add the regression dropped in 06a0c80: filterModelOptionsByAllowlist must evaluate the allowlist against the decoded target model, not the encoded __switch_profile__ wrapper. Uses this suite's per-test settings cache (reset in afterEach) instead of the shared cache that made the earlier version flaky (jatmn review, #1119). * test(model-picker): make the cross-profile allowlist test leak-proof The new allowlist test drove availableModels through setSessionSettingsCache, but sibling suites (ModelPicker, ProviderManager, ...) mock.module both settings.js (getSettings_DEPRECATED) and modelAllowlist.js (isModelAllowed) process-wide, so in the full sequential run the leaked stubs defeated the cache and the denied option was not filtered (smoke-and-tests red on the full suite, green in isolation). Drive the allowlist deterministically from this suite instead: install-once, gated, passthrough mocks of getSettings_DEPRECATED (the filter gate) and isModelAllowed (the per-option check), both keyed off a single activeSettingsOverride and cleared in afterEach. Same gated-passthrough pattern as the suite's existing providerConfig/providers/auth/profiles mocks and the agent.test.ts allowlist approach. * fix(model): keep cross-profile switch options out of the SDK models list getModelOptions() now returns inactive-profile entries encoded as __switch_profile__:<id>:<model>. print.ts mapped those straight into the ModelInfo list returned to SDK/web callers, exposing UI-only values that are not selectable model ids. Filter them with parseSwitchProfileValue before building modelInfos. Add ModelPicker coverage for the allowProfileSwitch filter (hidden inline, shown when allowed) and document cross-profile /model switching in the provider-profile docs. * test(model-picker): prove cross-profile switch options never reach SDK models Extract selectSdkModelOptions as the single gate the SDK modelInfos builder runs every getModelOptions() entry through, and cover it directly: an encoded __switch_profile__:<id>:<model> option is dropped while real model ids pass through. Fails if an inactive-profile affordance ever leaks into the initialize.models response again (#1119). * docs(model-picker): clarify the env gate for inactive-profile entries The inactive-profile models only appear when the provider-profile env workflow is active (CLAUDE_CODE_PROVIDER_PROFILE_ENV_APPLIED=1), not for every multi-profile setup. Spell that out and restore the local-only `--provider ollama` guidance that was folded into the paragraph. * fix(model-picker): gate SDK option filter on switchToProfileId marker selectSdkModelOptions filtered on the encoded __switch_profile__ value prefix, which also reserved that prefix for every custom model id. A real configured model whose id starts with __switch_profile__: would vanish from the SDK models response and non-switching pickers. Key the gate on the explicit switchToProfileId marker, which only synthesized switch options carry, and add the collision regression. Refs #1119 * fix(model-picker): reuse switch confirmation for cross-profile selections The cross-profile branch built its own "Switched to" message and returned before the regular path appended the selected effort and the "Billed as extra usage" notice, hiding cost-impacting feedback when a reasoning/extra-usage target was chosen through an inactive profile. Append effort and the extra-usage check to the switch confirmation. Refs #1119 * fix(model-picker): surface inactive profiles on the active Ollama path The isOllamaProvider() early return ran before the inactive-profile options were computed, so an active local Ollama profile saw only its own models and lost the cross-profile switcher, forcing the /provider round-trip this feature removes. Hoist the inactive-profile compute above the Ollama branch and append it to the Ollama returns. Refs #1119 * fix(model): surface inactive profiles on all provider branches; decode only real switch options Two follow-ups to the #1119 unified /model switcher: - inactiveProfileOptions was computed before the early-return branches but only appended on Ollama / local-scope / PAYG paths. The GitHub Copilot, NVIDIA NIM, MiniMax, Xiaomi MiMo, ant, and Claude-subscriber branches returned first, so a user with a saved profile active on any of those routes lost the cross-profile entries and had to round-trip through /provider. Append the (env-gated, so empty unless a profile is applied) inactive options on those branches too. - filterModelOptionsByAllowlist decoded any value starting with `__switch_profile__:` via parseSwitchProfileValue, even a normal custom model id that merely shares that prefix, evaluating the allowlist against the wrong inner model. Gate the decode on the `switchToProfileId` marker (the type's documented contract) so non-switch ids are checked verbatim. Extends the cross-profile harness with gated getAPIProvider / NVIDIA / subscriber overrides and adds branch-append + verbatim-allowlist regressions (red-green). * fix(model): key profile-switch handling on the marker across picker and command The allowlist/SDK paths already used the switchToProfileId marker, but two surfaces still keyed on the raw `__switch_profile__:` value prefix: - ModelPicker's inline-picker filter hid any option whose value started with the prefix, so a real custom model id like `__switch_profile__:vendor:gpt-5.4` disappeared from prompt/settings pickers. It now filters on `switchToProfileId === undefined`. - the /model command decoded parseSwitchProfileValue(model) for any prefixed string and tried to activate the encoded profile id, so selecting such a custom model activated a nonexistent profile instead of setting the literal model. It now only treats the value as a switch when the decoded profile id maps to a real configured provider profile — which every synthesized switch option does, and a prefix-colliding custom id does not. Drops the now-unused SWITCH_PROFILE_VALUE_PREFIX import from ModelPicker. Adds a picker regression (marked switch hidden, prefixed custom model stays visible) and completes the cross-profile branch coverage (MiniMax, Xiaomi MiMo, ant) so every branch that appends inactive-profile options is locked. * test(model): register target profiles in cross-profile switch tests The /model command now only treats a `__switch_profile__:` value as a switch when its decoded profile id maps to a real configured provider profile. The cross-profile switch tests set up setActiveProviderProfile but left the shared getProviderProfiles mock empty, so the new guard classified their switch values as literal models and the fast-mode / effort / extra-usage assertions no longer ran. Register each test's target profile via getProviderProfiles so the switch path executes as intended. * fix(model): gate cross-profile switches on the selected option marker Selecting a value that merely parses as `__switch_profile__:<profileId>:<model>` activated the provider whenever <profileId> existed, so a literal custom model id such as `__switch_profile__:profile_openai:gpt-5-mini` wrongly switched the active provider instead of being applied verbatim. Thread the picked option's `switchToProfileId` marker from ModelPicker.onSelect (selectOptions already carries it) and only activate a profile when the marker matches the decoded id. The effort/display resolver had the same gap — it decoded every prefixed value; gate it on a genuine marker-backed switch option too. Add a regression asserting a marker-less prefixed id is applied literally. * test(model): cover Max/Team Premium and empty-catalog switch-append paths The cross-profile branch-coverage suite exercised the populated-catalog returns but not the Max/Team Premium subscriber early return nor the empty-catalog fallbacks (NVIDIA/MiniMax/Xiaomi), which are the same paths that previously dropped the inactive-profile switch options. Lock them so every changed return that appends `...inactiveProfileOptions` is covered. * fix(model): keep inactive-profile switch options in /model discovery overrides The interactive /model command passes an optionsOverride into ModelPicker for descriptor-backed and legacy OpenAI-compatible discovery contexts, built from mergeActiveProfileModelOptions which only merges the ACTIVE profile's route models. Because the picker renders optionsOverride ?? getModelOptions(), the inactive-profile switch entries getModelOptions() appends never reached those paths, so the unified switcher vanished for provider-profile routes (OpenRouter/Kimi/MiniMax, refreshed local profiles). Re-append the same inactive-profile switch options (allowlist-filtered on the decoded target) to any override list before handing it to the picker. * fix(model): base the switch marker on the presented option, treat ties as ambiguous The picker derived switchToProfileId with selectOptions.find(value===...), and the effort/display resolver decoded when any getModelOptions() entry with the same value carried the marker. If a literal custom model id collided with an encoded switch value, the literal could borrow a different same-value option's marker and wrongly activate a provider. Add resolveSelectedSwitchProfileId, which keys on the actual presented option and treats duplicate-value matches as ambiguous (no switch), and route both the onSelect marker and the decode decision through it. |
||
 github-actions[bot]GitHubgithub-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
338f9ad85f |
chore(main): release 0.23.0 (#1870)
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>v0.23.0 |
||
|
|
b8c8b2417b |
fix(opencode-go): surface clear error on subscription quota exhaustion (#1749)
* fix(opencode-go): surface clear error on subscription quota exhaustion The opencode.ai/zen/go gateway returns 429 with FreeUsageLimitError or GoUsageLimitError in the body when a user's Go subscription quota runs out. Previously these fell through to the generic "Request rejected (429)" path, causing a mysterious stop with no actionable hint. Detect the opencode-go-specific error markers, surface a clear message with the upgrade URL (free tier) or reset duration + workspace + limit name (paid tier), and skip retry — the quota is terminal until reset. Mirrors the canonical implementation in anomalyco/opencode packages/opencode/src/session/retry.ts. * fix(api): abort retry and avoid thinking hang on quota/allotment exhaustion * fix: implement request URL precedence and add test coverage * fix(api): make 'x-opencode-request-url' header authoritative for OpenCode Go errors * fix(api): preserve OpenCode Go quota message and reuse non-stream converter in JSON fallback Addresses the two remaining P2 review items on #1749. withRetry: the early isQuotaExhausted guard wrapped OpenCode Go FreeUsageLimitError/GoUsageLimitError 429s in the generic "API quota exhausted or not enabled" message, clobbering the actionable subscribe/ reset guidance. Skip the generic guard for OpenCode Go quota errors so they fall through to the standard shouldRetry=false terminal path, which rethrows the original APIError and lets getAssistantMessageFromError surface the specific message. Consolidate detection in a shared isOpenCodeGoQuotaError predicate (errors.ts) and drop the duplicated inline header check in shouldRetry. openaiShim: the application/json fallback in openaiStreamToAnthropic hand-rolled a thin converter that dropped tool_calls, forwarded raw OpenAI finish_reason values as Anthropic stop reasons, skipped array-content normalization, bypassed <think> stripping, and lost raw text tool-call recovery. Extract the established non-streaming conversion into a shared convertNonStreamingResponseToAnthropicMessage and route the fallback through it, re-emitting the result as stream events. _convertNonStreamingResponse now delegates to the same function. Adds regression coverage: a withRetry test proving the OpenCode Go message survives the retry loop, and JSON-fallback tests for tool_calls, stop-reason mapping, <think> stripping, array content, and raw text tool-call recovery. * fix(api): terminate OpenCode Go quota in fast mode; treat empty tool_calls as absent Addresses CodeRabbit's review on #1749. - [Major] withRetry.ts: throw CannotRetryError for isOpenCodeGoQuotaError BEFORE the fast-mode 429 fallback. Previously the guard only *skipped* the generic quota throw, so an OpenCode Go 429 while fast mode was active hit the fast-mode retry/cooldown path instead of surfacing the quota message immediately. Wrapping the original APIError still preserves the OpenCode Go assistant message via getAssistantMessageFromError. Adds a fast-mode regression test (mutation-checked) and a forceFastMode option on the test helper. - [Minor] openaiShim.ts: an empty tool_calls array is truthy, which skipped the raw "Tool calls requested" recovery in convertNonStreamingResponseToAnthropicMessage. Gate on a single hasStructuredToolCalls (length > 0) check across both the string- and array-content raw-recovery paths and the structured loop. Adds a JSON-fallback regression test for tool_calls: [] (mutation-checked). - [Minor] openaiShim.test.ts: collectFallbackEvents now saves and restores globalThis.fetch in a finally block so the stub can't leak past the helper. * test(openai-shim): cover empty tool_calls raw-recovery on array content too Addresses CodeRabbit's follow-up on #1749: the empty-tool_calls regression only exercised the string-content path, but the hasStructuredToolCalls fix also gates the array-content branch. Add a companion JSON-fallback test with array-form message.content and tool_calls: [] so the array branch can't regress silently. Mutation-checked: reverting hasStructuredToolCalls to a truthiness check fails it. * fix(api): preserve OpenCode Go quota errors --------- Co-authored-by: jatmn <the@jat.mn> |
||
|
|
2edec9a140 |
fix(deps): ship a zero-warning, minimal install (#1784)
* fix(deps): ship a zero-warning, minimal install
The published package declared 62 runtime `dependencies`, but `dist/cli.mjs`
is a fully-bundled esbuild output that inlines almost all of them. End users
therefore installed ~476 transitive packages — including three subtrees the
bundle never needs at install time, each emitting an install warning:
- node-domexception (deprecated) via google-auth-library
- protobufjs (allow-scripts) via @grpc/* (already bundled into dist)
- sharp (allow-scripts) native image module
The repo's `overrides`/`allowScripts` silence these locally, but those are
root-only npm settings and are ignored when the package is installed as a
dependency — so end users saw the warnings.
Core changes:
- package.json: runtime dependencies trimmed 62 -> 3 (@orama/orama,
@orama/plugin-data-persistence, @vscode/ripgrep). Bundled packages, plus
the optional sharp/google-auth-library, move to devDependencies so they
are built/tested but not shipped.
- package.json: @anthropic-ai/sdk, @modelcontextprotocol/sdk, react and
react-reconciler declared as OPTIONAL peerDependencies — externalized by
the ./sdk bundle but bundled into the CLI. Optional peers keep the CLI
install minimal and warning-free while still resolving for ./sdk consumers.
- externals.ts: sharp, google-auth-library and @anthropic-ai/bedrock-sdk
marked OPTIONAL_RUNTIME_EXTERNALS (loaded on demand, not shipped).
- validate-externals.ts: runtime deps validate against externals; bundled
deps validate against dependencies + devDependencies.
- client.ts: load @anthropic-ai/bedrock-sdk via the runtime importer so
esbuild no longer inlines it and hoists its static @aws-sdk import into
the CLI bundle (that was a startup crash for default installs).
Optional-dependency UX (consistent, actionable errors):
- New src/utils/optionalRuntimeModule.ts exports importRuntimeModule and
importOptionalRuntimeModule. The optional variant translates a missing
package (code === 'ERR_MODULE_NOT_FOUND', specifier present in message)
into "<feature> requires "<pkg>" ... Run `npm i -g <pkg>`". Generic so
typed call sites keep their module types.
- Routed ALL optional-package load sites through it (previously only one
did): google-auth-library (client.ts, auth.ts, geminiAuth.ts),
@anthropic-ai/foundry-sdk + @azure/identity (client.ts), and the
@aws-sdk/* Bedrock paths (model/bedrock.ts, tokenEstimation.ts, aws.ts).
- imageProcessor.ts: sharp-missing error now says `npm i -g sharp`.
- docs/advanced-setup.md: new "Optional provider packages" table and a
Vertex note documenting the on-demand installs.
- Unit test for the helper (friendly error, success path, specifier match,
raw passthrough).
- knip.json: ignore google-auth-library (now loaded via runtime string).
Verified on the current tree:
- tsc, build/validate-externals, knip, and tests all pass.
- npm pack + install --omit=dev adds 8 packages, zero deprecation/
allow-scripts/funding warnings; --version/--help/mcp list run.
- With packages absent, CLAUDE_CODE_USE_BEDROCK and CLAUDE_CODE_USE_VERTEX
print the friendly `npm i -g <pkg>` error (verified end-to-end).
- ./sdk imports once its optional peers are present (24 exports, no warns).
- Bundled ajv + ajv-formats validate with no ajv installed; no unguarded
native runtime requires (fsevents absent in chokidar 4; bun:sqlite Bun-only).
Trade-off: image reads, AWS Bedrock, Azure Foundry and GCP/Vertex now prompt
a one-time `npm i -g <pkg>` instead of being shipped to every user.
Co-Authored-By: OpenClaude <openclaude@gitlawb.com>
Review fixes (CodeRabbit + jatmn):
- validate-externals: the INTENTIONALLY_BUNDLED exemption is now scoped per
bundle. The CLI exempts every bundled package; the SDK does NOT exempt
packages declared as peerDependencies (keyed on package.json, an independent
source of truth) so dropping react/@anthropic-ai/sdk from SDK_EXTERNALS now
fails validation instead of silently passing. Added an explicit minimal-
install contract check: bundled packages must be devDependencies-only — never
in `dependencies`, and only the SDK-external subset may be optional peers.
Validation logic extracted to scripts/externalsValidation.ts + tests.
- FileReadTool oversized-image fallback now loads via the shared
getImageProcessor() (not a raw import('sharp')) and re-throws
ImageProcessorUnavailableError, so a missing processor surfaces the
`npm i -g sharp` install hint instead of returning an over-budget image.
- optionalRuntimeModule: match the missing specifier as a QUOTED token, not a
raw substring, so a missing transitive package whose name contains the
requested one (sharp vs sharp-libvips, @aws-sdk/client-bedrock vs
@aws-sdk/client-bedrock-runtime) no longer triggers the wrong install hint.
Predicate extracted to isMissingSpecifierError() with regression tests.
- docs/advanced-setup.md: the Vertex auth section now shows both documented
paths (gcloud ADC and a GOOGLE_APPLICATION_CREDENTIALS service-account file).
Review fixes (round 2, CodeRabbit):
- validate-externals: assert the optional-peer install contract — every
peerDependency must be { optional: true } in peerDependenciesMeta
(validateOptionalPeers), so losing that flag fails the build instead of
silently reintroducing install warnings.
- validate-externals: hard-check OPTIONAL_RUNTIME_EXTERNALS placement
(validateOptionalRuntimeexternals). Anything esbuild can see statically must
stay external in BOTH bundles (dropping sharp/google-auth-library now fails);
the runtime-indirection-only subset (new RUNTIME_INDIRECTION_ONLY_EXTERNALS)
must stay OUT of externals so esbuild never re-exposes their static imports.
- Deeper-dig fix: @anthropic-ai/foundry-sdk was misclassified as
INTENTIONALLY_BUNDLED, but it is loaded only through the Function indirection
(esbuild never sees it, so it was never actually bundled) — its sole presence
in dist is the specifier string. Per the PR's own "Azure Foundry now prompts"
trade-off it is on-demand, so it now lives in OPTIONAL_RUNTIME_EXTERNALS +
RUNTIME_INDIRECTION_ONLY_EXTERNALS (mirroring bedrock-sdk). sandbox-runtime is
genuinely statically imported, so it stays bundled.
- Provider-routing coverage (scripts/optionalRuntimeSpecifiers.test.ts): a
static scan asserts every importOptionalRuntimeModule specifier is a declared
OPTIONAL_RUNTIME_EXTERNAL and never also INTENTIONALLY_BUNDLED — the
invariant that keeps a provider's optional package loadable on demand.
- All new validators extracted to scripts/externalsValidation.ts with tests.
Review fixes (round 3, CodeRabbit):
- client.ts: gate the Vertex google-auth-library import behind the non-skip
branch. CLAUDE_CODE_SKIP_VERTEX_AUTH (proxy/test) uses a mock GoogleAuth and
must not require the optional package; it was loaded unconditionally before.
- optionalRuntimeModule: drop the hard-coded `npm i -g`. The helper backs both
the global CLI and project-local ./sdk consumers, so the hint is now
context-neutral ("npm install <pkg>" / add -g for the global CLI).
- validate-externals: every SDK_ONLY_EXTERNALS entry must STAY a
peerDependency (a dropped peer leaves runtimeDeps while the SDK still
externalizes it); and OPTIONAL_RUNTIME_EXTERNALS must never be shipped (fail
on overlap with dependencies/peerDependencies). Both with tests + live-verified.
- optionalRuntimeSpecifiers.test: pin the EXACT set of optionally-loaded
specifiers instead of a >=5 count (a count passes even if a provider path
regresses).
- attachments: extract tryReadEditedImageAttachment() — background watched-file
image attachments DEGRADE to null on any failure (incl.
ImageProcessorUnavailableError) so a missing optional package never aborts a
turn, while the explicit FileReadTool path still surfaces the install hint.
Deterministic regression test (bad path -> null).
- docs: Bedrock row notes profile-based auth also needs
@aws-sdk/credential-providers; install-hint wording matches the new message.
Review fixes (round 4, CodeRabbit):
- attachments: stop sending the raw file path through the analytics
bypass-cast (tengu_watched_file_compression_failed). Send only the safe
file extension via getFileExtensionForAnalytics, matching the existing
tengu_file_read_dedup pattern, so no usernames/project paths can leak.
- externals.ts: corrected the OPTIONAL_RUNTIME_EXTERNALS header comment,
which still claimed all entries "remain in COMMON_EXTERNALS" — no longer
true since the indirection-only subset (bedrock/foundry) must stay OUT of
the externals lists.
(Other CodeRabbit comments on this push re-surface items already addressed in
prior commits: the peerDependenciesMeta-optional check (validateOptionalPeers),
the SDK-peers-present and optional-not-shipped validator rules, the
exact-specifier-set test, the attachments degrade contract + test, and the
context-neutral install hint are all present. The "assert every optional
external is a devDependency" suggestion is intentionally NOT applied: @aws-sdk/*
and @azure/identity are transitive devDeps via bedrock-sdk/foundry-sdk, so a
blanket assertion would be incorrect; source resolution is covered by the
build + tests that import these packages.)
Review fixes (round 5, CodeRabbit):
- attachments: stop leaking file paths via logError in the background-image
degrade path. readImageWithTokenBudget can throw path-bearing messages
(e.g. "Image file is empty: <path>") and logError persists message/stack, so
log only the error TYPE name now. (Analytics payload was already sanitized.)
- attachments: tryReadEditedImageAttachment takes an injectable reader so the
degrade contract is tested for the EXACT error types — ImageProcessorUnavailableError
and a path-bearing read error both degrade to null (not just ENOENT) — plus a
success case. No mocking.
- validate-externals: enforce the source-install half of the optional contract.
Non-transitive OPTIONAL_RUNTIME_EXTERNALS must be devDependencies so `bun
install` source builds resolve them. The new TRANSITIVE_OPTIONAL_EXTERNALS
documents the exemption (@aws-sdk/* via @anthropic-ai/bedrock-sdk, @azure/identity
via @anthropic-ai/foundry-sdk — provided transitively, not direct devDeps). A
blanket "all optionals are devDeps" check would have wrongly failed on those.
Tests + live-verified (dropping sharp from devDependencies now fails).
Review fixes (round 6, CodeRabbit + jatmn):
- optionalRuntimeSpecifiers.test: the call-site scan regex missed
generic-annotated calls (importOptionalRuntimeModule<...>(...)) in
model/bedrock.ts and tokenEstimation.ts, so the exact-set assertion was
incomplete. Regex now allows an optional generic; EXPECTED_SPECIFIERS adds
@aws-sdk/client-bedrock and @aws-sdk/client-bedrock-runtime (7 total).
- importOptionalRuntimeModule default generic is now <T = unknown> (was any),
so destructured imports are no longer silently any. Every call site now
supplies its module type — typeof import('<pkg>') where the package is
type-resolvable (bedrock-sdk, foundry-sdk, @aws-sdk/credential-providers,
google-auth-library), and a named minimal-shape alias for @azure/identity
(not a direct devDep, so typeof import can't resolve it). This gives
compile-time verification of each provider's module contract (export names,
shapes) — the structural answer to the "cover the provider branches" ask.
- attachments: tryReadEditedImageAttachment takes injectable {read,log,track};
a new test asserts the sanitized-telemetry contract directly — the logError
payload is path-free and the analytics payload carries only `ext`, never the
edited-image path.
* fix(deps): address optional runtime review findings
* test(deps): isolate optional runtime importer mocks
* fix(deps): clarify AWS optional auth labels
* fix(deps): close optional runtime review gaps
---------
Co-authored-by: jatmn <the@jat.mn>
|
||
|
Kevin CodexandGitHub
|
2a506f9f38 | added tencent hy3 to opengateway available models (#1876) | ||
|
euxaristiaandGitHub
|
4be017bd4d |
fix: resolve zai-compatible config for all GLM remote models (#1752)
* fix: resolve zai-compatible config for all GLM remote models
* fix: address PR reviews for GLM thinking continuation, custom aliases, Fireworks catalog entries, and reasoning effort preservation
* fix(integrations): gate GLM Z.AI shim to non-catalog routes
The name-based matcher applied the full Z.AI reasoning contract to any
model path containing `glm-<digit>`, overriding catalog-backed non-Z.AI
routes (NEAR AI `zai-org/GLM-5.1-FP8`, Fireworks `glm-5p2`). Now the GLM
branch fires only when there is no catalog entry; Z.AI-contract GLM
routes (opencode-go, atlas-cloud) declare the shim explicitly via
transportOverrides, and a shared ZAI_GLM_OPENAI_SHIM constant is the
single source of truth. Adds negative (NEAR AI) and positive
(opencode-go, atlas) regressions.
* test(integrations): cover the direct Z.AI vendor catalog route in the gating block
Addresses CodeRabbit's nitpick on #1752: the GLM catalog-aware gating describe
block had NEAR AI (negative), opencode-go, and atlas-cloud (positive) cases but
no direct `zai` vendor catalog positive. Add one asserting the full GLM contract
(routeId 'zai', preserveReasoningContent, thinkingRequestFormat 'zai-compatible',
requireReasoningContentOnAssistantMessages) alongside the override-based routes.
* fix(effort): extend supportsZaiReasoningEffort for provider-prefixed GLM-5.2 models
The previous implementation only matched bare 'glm-5.2' and 'zai-org/glm-5.2'. When accessed via an aggregator alias like 'openrouter/zhipu/glm-5.2', the model name doesn't start with 'glm-' or match the zai-org prefix, so supportsZaiReasoningEffort returned false and reasoning_effort was omitted from the request body. Add an endsWith('/glm-5.2') fallback to match any provider-scoped path ending in the base model name.
* fix(gateways): wire Z.AI GLM shim to atlas-cloud GLM entries and opengateway GLM 5.2
- Add enableToolStreaming to ZAI_GLM_OPENAI_SHIM shared constant
- Apply transportOverrides.openaiShim to all 7 atlas-cloud zai-org/glm-*
entries so catalog-backed routes get the full Z.AI shim contract
- Layer Z.AI-specific overrides (thinkingRequestFormat,
preserveReasoningContent, enableToolStreaming) on the opengateway-glm-5.2
entry without conflicting with the gateway-level max_completion_tokens
and removeBodyFields
Refs #1752
* refactor(gateways): derive opengateway GLM shim from shared ZAI_GLM_OPENAI_SHIM
* fix(gateways): align Atlas + OpenCode Zen GLM entries with Z.AI wire format
- Atlas Cloud GLM entries: wireFormat 'reasoning_effort' → 'zai_compatible'
- OpenCode Go catalogEntry: add reasoning + capabilities for zaiGlm models
- OpenCode Go model descriptors: add reasoning: true for GLM entries
* fix(integrations): rebase onto upstream/main, fix Atlas/OpenCode Zen GLM wiring per review
Rebased onto upstream/main (
|
||
|
euxaristiaandGitHub
|
885bd81045 |
fix(hicap): add missing hicap-claude-opus-4.7 catalog entry (#1797)
* fix(hicap): add missing hicap-claude-opus-4.7 catalog entry * test(integrations): cover hicap-claude-opus-4.7 runtime limits and catalog entry * fix(hicap): add claude-opus-4-7 alias and update provider request test * test(hicap): cover opus 4.7 catalog aliases |
||
|
euxaristiaandGitHub
|
78a4b99f68 |
[codex] Enable Windows long paths for worktrees (#1729)
* fix(worktree): enable Windows long paths * fix(worktree): gate core.longpaths behind worktree.enableGitLongPaths setting + add regression tests * fix(worktree): rename enableGitLongPaths to autoConfigureLongPaths, prevent global mock module leakage in tests * fix(worktree): isolate platform and mock dependencies in tests to prevent global mock leak * fix(worktree): map legacy worktree settings, route sparse-checkout errors through formatter, and add strict types |
||
|
0xfandomandGitHub
|
7d861743ca |
fix(diff): guard diff language detection against prototype-chain filenames (#1433)
* fix(diff): guard diff language detection against prototype-chain filenames detectLanguage() looked up the filename basename/stem in a plain-object FILENAME_LANGS map. For a file whose name collides with an Object.prototype member — constructor, toString, valueOf, hasOwnProperty, __proto__ — the lookup resolved to the inherited function rather than undefined. That non-string value was then passed to highlight.js getLanguage(), which crashes the whole diff render with "(name || '').toLowerCase is not a function" (e.g. editing constructor.css). Switch FILENAME_LANGS to a Map so missing keys return undefined and inherited prototype members can never leak into the lookup. Fixes #1430 * diff: make detectLanguage's language validator injectable for tests Per review: the #1430 regression test reached highlight.js#getLanguage() through detectLanguage, which lazy-loads and caches the full highlight.js registry into the shared Bun test process — the shared-suite pressure the module comment warns against. Add an optional third argument to detectLanguage that supplies the language validator, defaulting to the existing hljs lookup so production callers are unchanged. The regression test now injects a small stand-in validator, so the prototype-key coverage runs without loading highlight.js. |
||
|
BogdanandGitHub
|
11f4661ea9 |
fix(lsp): coalesce diagnostic bursts (#1861)
* fix(lsp): coalesce diagnostic bursts * fix(lsp): tighten diagnostic debounce coverage * test(lsp): share zero-diagnostic log assertion * test(lsp): isolate diagnostic attachment debounce coverage |
||
|
fb40d49e68 |
feat: add repo map codebase intelligence (#1867)
* feat: add Codebase Intelligence — repo map with PageRank-ranked structural summaries
Adds a new module that builds a structural map of the repository by parsing
source files with tree-sitter, building a cross-file reference graph weighted
by IDF, ranking files with PageRank, and rendering a token-budgeted summary
of the most important files and their signatures.
Surface:
- RepoMap tool the model can call on-demand, with focus_files / focus_symbols
- /repomap slash command with --tokens, --focus, --stats, --invalidate
- Auto-injection into session system context, gated by REPO_MAP=1 env var
(compile-time feature('REPO_MAP') flag stays off in scripts/build.ts)
How it works:
git ls-files → tree-sitter WASM parse → extract defs/refs →
IDF-weighted directed graph → PageRank → render top files until token budget
Files imported by many others rank highest. Common symbol names (get, set,
map, value) are down-weighted via IDF. Results cached to disk keyed by
(path, mtime, size) — only changed files are re-parsed.
Supported languages: TypeScript, JavaScript, Python.
Tree-sitter tag queries are inlined as string constants in queries.ts so
they ship inside dist/cli.mjs and work after npm install — the .scm source
files are kept for readability/Aider attribution but are not required at
runtime. A drift-guard test (queries.test.ts) asserts byte-equality between
the inlined strings and the .scm source files.
Dependencies added: web-tree-sitter, tree-sitter-wasms, graphology,
graphology-pagerank, graphology-operators, js-tiktoken.
* fix(repomap): invalidate rendered cache on file edits + Windows test fix
- computeMapHash now folds per-file mtime+size into the cache key so a
source edit (without changing the file list) no longer returns the
prior rendered map. Adds a regression test that edits a file and
confirms the second build reflects the new symbol without manual
invalidateCache().
- queries.test.ts byte-for-byte drift guard normalizes CRLF -> LF when
reading the .scm source so Windows checkouts pass. .gitattributes
also pins *.scm to LF on future checkouts.
- Externals: declare web-tree-sitter, tree-sitter-wasms, graphology*,
and js-tiktoken in scripts/externals.ts so build validation passes.
* fix(repomap): expand directory focus paths
* fix(repomap): satisfy deadcode check
* Fix repo map review findings
* Resolve remaining repo map review findings
* fix(repomap): address review findings
* fix(repomap): address review findings
* fix(repomap): resolve smoke and review follow-ups
* fix(repomap): preserve cached tag order
* fix(repomap): resolve review follow-ups
* fix(repomap): satisfy query promise lint
* Fix repo map context timeout cleanup
* fix: address repo map review findings
* fix: cancel timed-out repo map context builds
* fix(repomap): preserve git file path whitespace
* fix(repomap): handle graph and parsing edge cases
* fix(repomap): preserve shell token positions
* fix(repomap): respect configured cache home
* fix(repomap): address review findings
- Add explicit 10000ms timeout to the feature-flag-off context test to avoid cold-import flakes.
- Add --focus-symbols flag to /repomap and forward it to buildRepoMap, matching the RepoMap tool.
- Add parsing/command tests and docs coverage for --focus-symbols.
---------
Co-authored-by: gnanam1990 <gnanasekaran.sekareee@gmail.com>
|
||
|
8369f2018e |
feat(provider): add AI/ML API provider (#863)
* feat(provider): add AI/ML API integration Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(provider): preserve bootstrap model fallback * fix(provider): complete aimlapi env-only routing * test(provider): keep first-run preset assertions visible * fix(provider): align aimlapi attribution and setup docs * fix(provider): use aimlapi rebate attribution headers * fix(provider): report current integration version * fix(provider): prioritize dedicated aimlapi credentials * fix(provider): complete AI/ML API attribution headers --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Lookoff123 <bataryshkinairina@gmail.com> |
||
 3kin0xandGitHub
|
f292b057b5 | fix: await main() in cli entrypoint to prevent premature exit in Node 24.x (#1697) | ||
|
|
766d3f8794 |
fix(compact): count string message content (#847)
* fix(compact): count string message content * test(compact): assert string content token estimate --------- Co-authored-by: jatmn <the@jat.mn> |
||
 
|
1bd273d4d3 |
fix: isolate OpenClaude config from Claude Code (#1875)
* Rename .claude paths to .openclaude * test: update skill watcher paths for openclaude * fix: preserve default secure storage key Co-authored-by: Cursor <cursoragent@cursor.com> * chore: address config isolation review comments * fix: canonicalize secure storage config paths * test: isolate secure storage config override * Fix keychain service name config dir handling Update macOS keychain service naming to honor `OPENCLAUDE_CONFIG_DIR` by resolving the env override directly before falling back to the default config home lookup. Adjust secure storage platform tests to import `envUtils` and keychain helpers dynamically with the same module suffix and restore module mocks correctly, keeping test state isolated and consistent. * Align diagnostics and keychain with OpenClaude Updates several utilities to use OpenClaude defaults and naming consistently. Doctor diagnostics now always checks a package name (falling back to `@gitlawb/openclaude`), macOS secure storage service names and related tests now use `OpenClaude`, and keychain prefetch docs were updated to match. This also removes an unused `homeDir` option from local install dir candidates and treats `.claude.json` as a dangerous filesystem target. * Fix doctor npm uninstall package fallback Update doctor diagnostics to generate npm global uninstall guidance using a single package-name variable. When `MACRO.PACKAGE_URL` is not set, it now falls back to `@gitlawb/openclaude` instead of `openclaude`, so the suggested cleanup command matches the scoped package install. * Protect legacy .claude paths from writes Add .claude to DANGEROUS_DIRECTORIES and sandbox denyWrite lists, extend isClaudeSettingsPath to cover legacy .claude/settings.json paths, and update README to clarify CLAUDE_CONFIG_DIR is not used for background-session storage. * Protect custom Claude config dir from sandbox writes * Protect legacy Claude config roots --------- Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
euxaristiaandGitHub
|
4a60c0f30f |
fix: guard convertToLogOption against empty transcript (#1723)
* fix: guard convertToLogOption against empty transcript * test: add regression test for convertToLogOption empty transcript guard |
||
|
e2bbb0295a |
feat: smart auto-routing (per-turn simple-vs-strong model selection) (#1734)
* feat(smart-routing): add smartRouting settings schema and reader * feat(smart-routing): resolve role keys to a SmartRoutingConfig * feat(smart-routing): wire per-user-turn routing into the query loop Classify once per user turn (transition===undefined), pin the decision in a loop-local, and apply the model-only route before the blocking-limit math. Enforce the org allowlist by calling isModelAllowed directly (coerce disallowed to strong; disable for the session if strong is also disallowed). Strip thinking history on a model change only under the provider gate (preserve-reasoning providers are left untouched). Export stripThinkingBlocksIfProviderAllows. * feat(smart-routing): add routed-error fallback to the strong model A simple-routed turn whose model call hits a retryable error retries once on the strong model, reusing the existing attemptWithFallback retry loop. Aborts and 4xx client errors propagate. Adds a session routing tally (simple/strong counts and simple->strong escalations) for the observability surface. * feat(smart-routing): add /smartroute command and env defaults /smartroute shows status and sets/toggles the simple and strong roles from agentModels keys, warning when the simple model is not first-party-cheaper than the strong one. OPENCLAUDE_SMART_ROUTING(_SIMPLE/_STRONG) provide startup defaults; an explicit settings block overrides env. * feat(smart-routing): show routing summary in /cost Appends a session routing summary (turns simple/strong, simple->strong escalations) to /cost, with an estimated-savings line gated on first-party pricing and annotated unavailable for unknown third-party pricing. Per-turn cost is already attributed to the routed model via the existing per-model breakdown. * fix(smart-routing): re-pin to strong after a routed-error fallback Without this, a turn's later continuation passes re-applied the pinned simple model after a fallback, re-triggering the same failure each pass. Re-pinning to strong keeps the rest of the turn on the recovered model. * fix(review): provider-swap guard, tally reset, notice-storm, env docs - Add the KTD6 provider-swap guard: drop the per-turn routing pin when a mid-turn provider-fallback swap changes the active provider, so the old provider's model id is not replayed at the new endpoint (adversarial P1). - Reset the routing tally in resetCostState() so /cost does not show stale cross-session counts. - Don't emit the disabled-for-session notice on every turn when no sessionId is available (suppress instead of storm). - Document OPENCLAUDE_SMART_ROUTING* in the openaiShim env-var header. - Add tests: provider-swap-safe pin, undefined-session silence, /smartroute strong arm and no-value guard. * docs(smart-routing): document /smartroute, settings, and env vars Register /smartroute in the web command catalog, add the smartRouting setting and OPENCLAUDE_SMART_ROUTING* env vars to the configuration reference, add a docs/smart-routing.md usage guide, and link it from the README. * fix(review): clear tally on /login, extract+test swap predicate, cap disabled set - /login used the raw bootstrap resetCostState, leaking the routing tally across an account switch; switch it to the cost-tracker wrapper. - Extract the provider-swap drop check as a pure, tested shouldDropPinForProviderSwap() and use it in the query loop. - Cap the disabledSessions set so a long-lived host can't grow it unbounded. - Document the 404/429 retry-by-design rationale; add tests for it. - Clarify the routedFallbackUsed per-turn scope and the apply-after-guard comment; document cross-provider role rejection and the re-enable path. * test(smart-routing): make allowlist tests robust to cross-file module mocks The decideTurnModel allowlist tests spied the global settings singleton, which let another file's leaked mock.module of modelAllowlist (agent.test.ts) flip isModelAllowed out from under them in the full suite. Spy isModelAllowed directly and restore it in afterEach so the tests are deterministic regardless of suite ordering. * fix(smart-routing): address CodeRabbit review and green CI - index.test.ts: pin the allowlist in the three happy-path decideTurnModel tests so they no longer inherit a leaked cross-file isModelAllowed mock (the CI test failure) - smartroute/index.test.ts: narrow the LocalCommandResult union via an expectText helper instead of reading .value off the union (the CI typecheck failure) - conversationRecovery.ts: route deserialize's thinking-strip gate through stripThinkingBlocksIfProviderAllows, removing the duplicated provider detection - conversationRecovery.test.ts: replace the two as-any fixtures with a shared typed factory * fix(smart-routing): scope cost claims to first-party reference pricing Smart routing's savings estimate and "simple isn't cheaper" warning were derived from the static first-party MODEL_COSTS table via getKnownInputCost, with no knowledge of the active provider, gateway, or account pricing. For a multi-provider user whose model ids happen to exist in that table but bill differently, the /cost summary and /smartroute warning stated a savings figure as if it reflected what they are actually charged. Narrow the copy instead of inventing provider-aware pricing the code cannot verify: the /cost line, the /smartroute warning, and docs/smart-routing.md now label the numbers as first-party reference pricing and note the active provider may bill differently. Tests assert the qualifier on every reworded branch so it cannot silently regress. No routing logic changed. * fix(smart-routing): clarify simple role wording * Fix smart routing review findings * fix(smart-routing): honor env roles and non-text turns * test(smart-routing): cover non-text skip path --------- Co-authored-by: jatmn <the@jat.mn> |
||
|
e9a3c308fc |
feat(skills): add PDF generation skill with native TypeScript implementation (#1718)
* feat(skills): add PDF generation skill with native TypeScript implementation
- Adds /pdf bundled skill for creating PDF documents from structured content
- Pure TypeScript PDF generator (~350 lines) with zero external dependencies
- Supports headings, paragraphs, bullet/numbered lists, code blocks, tables, images, HRs, spacers
- PDF spec 1.4 compliant with WinAnsiEncoding for common special characters
- Text wrapping, font sizing, and page layout handled automatically
- Uses embedded files pattern: pdfgen.ts extracted to CLAUDE_SKILL_DIR at runtime
- Model writes a TS script using the library, executes via bun run
- No binary dependencies, no pdfgen Rust tool, no system packages needed
* fix(pdf): address review feedback — escape template interpolations, fix object numbering, remove stub merge/split
* fix(pdf): address remaining review feedback — CLAUDE_SKILL_DIR substitution, image support removal, auto-pagination
- Remove all ${CLAUDE_SKILL_DIR} references from prompt and getPromptForCommand;
bundled skills prepend 'Base directory for this skill: <dir>' instead of
substituting this variable. Import examples now use relative './pdfgen'.
- Remove unimplemented image support:
- Remove { type: 'image' } from PDFElement in both prompt and pdfgen.ts
- Remove ImageData interface and all image XObject handling in PDFWriter
- Remove basename import (only used by image case)
- Remove 'Use relative paths for images' rule from prompt
- Add automatic multi-page continuation for overflowing content:
- Rename buildPageStream → buildPageStreams (returns PageStreamResult[])
- When y < maxY, flush current page and continue on a new page
- Code blocks that don't fit start on a new page
- PDFWriter.build creates a separate page object per content stream
- Replace el.rows.indexOf(row) with index variable for O(n) table rendering
Addresses review feedback from jatmn (round 2): R2-P2 x3
* fix(pdf): address CodeRabbit review — fonts, header/footer, CLI safety
- Create 8 distinct font objects (Helvetica/Bold/Oblique/BoldOblique +
Courier/Bold/Oblique/BoldOblique) instead of one shared Helvetica.
F1..F8 now reference separate objects (3..10) so bold/italic/courier
variants actually render correctly.
- Remove unused header/footer fields from PDFPage interface in both the
prompt and pdfgen.ts. These were silently dropped since buildPageStreams
never received them.
- Fix CLI --spec mode: outFile is now the last non-flag argument excluding
the spec file path, preventing data loss from overwriting the input JSON.
Previously args.find() could pick spec.json as outFile.
* fix(pdf): address R3 review — absolute import path, table cell wrapping
- P2: Replace relative './pdfgen' import with '<skill-base-dir>/pdfgen'
placeholder in prompt example and task instructions, instructing the
model to save and run scripts from the extracted skill directory
- P2: Replace silent .substring(0, 50) truncation with proper text
wrapping via wrapText() for table cells, with dynamic row heights
based on the tallest cell in each row
* fix(pdf): anchor multi-line table cells from row top to prevent downward overflow
- Compute cellStartY from row top (y + rowH - 4) instead of row bottom
so wrapped text flows downward within the cell boundary
* fix(pdf): split table rows that are taller than one page
R5-P2: Jatmn review — rows with very long wrapped cells could overflow
past the page bottom because pagination only checked once per row.
- Pre-compute wrapped lines for all cells in a row up front
- Render row in page-sized chunks, tracking linesRendered offset
- When a chunk fills the page, flushPage() and continue remaining
lines on the next page, drawing per-chunk backgrounds and borders
- Cells with fewer lines than the tallest cell simply have no text
drawn for the excess lines (no blank-line artefacts)
Fixes: Jatmn R5 finding (review 4450066815)
* fix(pdf): wrap overlong tokens and preserve WinAnsi characters
R6-P2 findings from Jatmn review (4451901963):
1. wrapText() now hard-splits tokens exceeding charsPerLine into
chunks, preventing long URLs/IDs/hashes from rendering off-page
or off-cell boundary as invisible text.
2. escapePdf() no longer calls toWinAnsi() again. The caller already
passes WinAnsi-encoded text; the double-pass was dropping mapped
characters (e.g. em-dashes, bullets) because the second pass treated
WinAnsi byte values as unsupported Unicode and silently dropped them.
* fix(pdf): encode table headers through WinAnsi and wrap long code lines
R7-P2 fixes:
- Table headers now pass through toWinAnsi() before escapePdf(), matching
the body text path. Headers containing em dashes, bullets, euro signs, and
other WinAnsi characters now render correctly instead of emitting raw
Unicode codepoints into the content stream.
- Code block lines are now wrapped to the available page width using the
existing wrapText() helper with Courier metrics. Long URLs, hashes,
minified lines, and other overlong tokens no longer extend past the page
boundary.
* fix(pdf): write PDF streams as latin1 bytes
* fix(pdf): address remaining review feedback - empty page, table validation, header overflow
* fix: resolve PDFElement type, support A3 page size, and enable Windows longpaths for worktrees
* fix: address CodeRabbit feedback on PDF skill allowedTools, worktree error propagation, and unit tests
* fix: restrict allowedTools for pdf skill and restore worktree files to main
* fix(skills): normalize base-dir to forward slashes for Windows import safety
Bundled skills receive a prompt prefix "Base directory for this skill:
<baseDir>". On Windows, <baseDir> is a backslash path like
C:\Users\...\pdf, and the skill prompt instructs the model to do
import { createPDF } from '<skill-base-dir>/pdfgen'
Embedding a backslash path into a single-quoted JS string breaks Bun
resolution because backslashes are treated as escape characters.
Normalize baseDir to forward slashes in prependBaseDir() before
building the prefix. Forward-slash paths work cross-platform in
TypeScript/Bun import statements, so the model can safely interpolate
the path verbatim.
Addresses jatmn's review on #1718 (Windows import path).
---------
Co-authored-by: SuperDuperZed <superduperzed@users.noreply.github.com>
|
||
 
|
214ee3dd2e |
feat(skills): add local skill CLI support (#1162)
* Add inspectable local skill CLI support OpenClaude Skill Hub needs the runtime repo to treat project skills as first-class local assets before registry installation exists. This wires native .openclaude skill directories into discovery, preserves .claude compatibility, and adds list/show subcommands so users can inspect resolved local skills. Constraint: Keep registry install, website catalog, and community governance out of this first runtime slice. Rejected: Replace the existing skills loader wholesale | the repo already has working bundled, plugin, MCP, dynamic, and legacy command skill paths. Confidence: medium Scope-risk: moderate Directive: Keep .claude skill loading compatible while .openclaude adoption rolls out. Tested: bun test src/skills/loadSkillsDir.test.ts src/commands.test.ts Tested: bun run build Tested: node dist/cli.mjs --bare skills list Tested: node dist/cli.mjs --bare skills show debug Tested: git diff --check * Add local skill validation and removal Skill Hub needs local package hygiene before registry install can be safe. This adds validation for SKILL.md directories and local removal for project or user skills without introducing remote registry behavior yet. Constraint: Registry install and update flows are still out of scope for this slice. Rejected: Implement install first | install needs the same validation and local removal semantics to avoid copying unsafe or unmanageable skill folders. Confidence: medium Scope-risk: moderate Directive: Keep validation conservative; loosen individual checks only with explicit registry policy coverage. Tested: bun test src/skills/loadSkillsDir.test.ts src/commands.test.ts Tested: bun run build Tested: node dist/cli.mjs skills validate .openclaude/skills/demo-skill Tested: node dist/cli.mjs skills list Tested: node dist/cli.mjs skills show demo-skill Tested: node dist/cli.mjs skills remove demo-skill Tested: git diff --check * Suppress startup banner for skills CLI Skills management commands are meant to be script-friendly inspection operations. Printing the interactive startup screen before the list/show/validate output makes the command noisy and hard to read. Constraint: Keep the interactive startup screen for normal OpenClaude sessions. Confidence: high Scope-risk: narrow Tested: bun run build Tested: node dist/cli.mjs skills list Tested: git diff --check * Make skills list readable for daily CLI use The default skills list output was a metadata-heavy dump, which made bundled and local skills difficult to scan. This changes the human formatter to an aligned table with wrapped descriptions while keeping machine-readable metadata behind --json. Constraint: Default list output must stay compact and human-readable while JSON remains script-friendly. Rejected: Keep version and trust columns in the default table | those fields add noise and remain available through --json/show. Confidence: high Scope-risk: narrow Directive: Keep the default list formatter focused on scanability; add metadata to --json or detail commands instead of widening the daily table. Tested: bun test src/cli/handlers/skills.test.ts src/skills/loadSkillsDir.test.ts src/commands.test.ts Tested: bun run build Tested: node dist/cli.mjs skills list Tested: node dist/cli.mjs skills list --json * Stabilize skills tests under CI The PR introduced skills tests that passed in focused runs but failed under the full GitHub Actions Bun test job. The formatter test now uses bun:test consistently, and skill directory tests explicitly restore the setting-source state they rely on. Constraint: CI runs the full Bun suite, so tests must avoid node:test interop and shared setting-source leakage. Confidence: medium Scope-risk: narrow Tested: bun test src/cli/handlers/skills.test.ts src/skills/loadSkillsDir.test.ts src/commands.test.ts Tested: git diff --check Not-tested: Full local bun test still has unrelated provider/OAuth failures on this machine. * Isolate user skill precedence test state The full CI suite can mutate process-wide config state while this test is running, so the user-vs-project precedence assertion now runs in a child Bun process with its own CLAUDE_CONFIG_DIR. Constraint: getSkillDirCommands reads global config/env state, so this precedence test needs process isolation under the full suite. Confidence: medium Scope-risk: narrow Tested: bun test src/skills/loadSkillsDir.test.ts src/cli/handlers/skills.test.ts src/commands.test.ts Tested: git diff --check * Stabilize conversation arc perf checks The CI runner was failing the conversation arc benchmarks because they used shared persisted knowledge graph state and strict wall-clock thresholds. The tests now isolate graph storage in a temporary config directory and keep only coarse regression limits suitable for noisy shared runners. Constraint: GitHub Actions shared runners can have variable storage/indexing latency. Rejected: Remove the benchmark coverage entirely | the tests still provide useful regression signals when isolated and coarse-grained. Confidence: medium Scope-risk: narrow Directive: Keep performance tests isolated from persisted user/project graph state. Tested: bun test src/utils/conversationArc.perf.test.ts src/skills/loadSkillsDir.test.ts src/cli/handlers/skills.test.ts src/commands.test.ts Tested: bun run smoke * Let users install skills from registries and local sources The skill hub CLI could list, inspect, validate, and remove local skills, but it had no supported install path. This adds a project/global install command that accepts local directories, raw SKILL.md files or URLs, and registry IDs with checksum validation when registry metadata provides one. Constraint: The companion openclaude-skills repository currently publishes SKILL.md files without riskLevel metadata, so validation keeps riskLevel optional while preserving required identity/source fields. Rejected: Require the external skills repository to be cloned into openclaude | install should work from registry metadata or explicit local paths without coupling the repos. Confidence: high Scope-risk: moderate Directive: Keep --json/list behavior machine-compatible; install output should remain human-readable and validation should not reject normal security-review prose. Tested: bun test src/cli/handlers/skillsInstall.test.ts src/cli/handlers/skills.test.ts src/skills/loadSkillsDir.test.ts src/commands.test.ts Tested: bun run build Tested: bun run smoke * Stabilize skills install tests in the full suite The install tests used shared console and cwd globals, which passed in isolation but raced with unrelated test files under Bun's full parallel suite. This makes the tests assert on installed files directly and injects the project directory into the handler for deterministic test isolation. Constraint: The CLI still resolves project installs from the runtime cwd; projectDir is only used by direct handler tests. Confidence: high Scope-risk: narrow Tested: bun test src/cli/handlers/skillsInstall.test.ts src/cli/handlers/skills.test.ts src/skills/loadSkillsDir.test.ts src/commands.test.ts Tested: bun run build Tested: bun run smoke * Keep skills install coverage in the existing skills suite The new standalone install test file changed Bun's parallel test scheduling and exposed unrelated global-state races in CI. Moving the coverage into the existing skills handler test file keeps the install behavior covered without adding another parallel test unit. Constraint: Some existing tests mutate cwd/config globals under full-suite parallelism. Confidence: medium Scope-risk: narrow Tested: bun test src/cli/handlers/skills.test.ts src/skills/loadSkillsDir.test.ts src/commands.test.ts Tested: git diff --check * Harden skill install paths before validation The install path used registry or SKILL.md names to create temporary and target directories before validation rejected unsafe names. This validates the install name before path construction, keeps the temp root as an explicit cleanup target, and resolves install targets under the selected skills root before copy or force removal. Constraint: Registry and raw SKILL.md sources are untrusted until validation completes. Rejected: Rely on validateSkillPath after temp construction | unsafe names can affect filesystem paths before validation runs. Confidence: high Scope-risk: narrow Tested: bun test src/cli/handlers/skills.test.ts src/skills/loadSkillsDir.test.ts src/commands.test.ts Tested: bun run build Tested: bun run smoke Tested: git diff --check * Hide bundled skills from human skills list The default skills list is meant for skills users can inspect and manage in the current environment. Bundled skills remain available internally and in JSON metadata, but the human table now omits bundled rows and removes the Source column. Constraint: --json remains machine-readable with full source metadata. Confidence: high Scope-risk: narrow Tested: bun test src/cli/handlers/skills.test.ts src/skills/loadSkillsDir.test.ts src/commands.test.ts Tested: bun run build Tested: node dist/cli.mjs skills list Tested: node dist/cli.mjs skills list --json Tested: git diff --check Tested: bun run smoke * Include home dir in config cache key Tests can mock homedir while leaving CLAUDE_CONFIG_DIR unset, so caching only by the env override can leak a temporary .openclaude root into later config/profile tests. Include homedir in the memoization key so config path resolution follows both inputs. Constraint: Keep getClaudeConfigHomeDir memoized for hot callers. Confidence: high Scope-risk: narrow Tested: bun test --max-concurrency=1 src/utils/openclaudePaths.test.ts src/utils/providerProfile.test.ts src/utils/knowledgeGraph.stress.test.ts tests/sdk/sdk-context-isolation.test.ts Tested: bun test src/cli/handlers/skills.test.ts src/skills/loadSkillsDir.test.ts src/commands.test.ts Tested: bun run build Tested: node dist/cli.mjs skills list Tested: bun run smoke Tested: git diff --check * Hide bundled skills from public skills commands Bundled skills are internal helpers, so the public skills CLI should only expose installed skills that users can inspect or manage. Filter bundled skills from JSON output and command lookups, and use a generic not-found response for hidden bundled names. Constraint: Installed project and user skills remain listed, inspectable, removable, and available in JSON. Confidence: high Scope-risk: narrow Tested: bun test src/cli/handlers/skills.test.ts src/skills/loadSkillsDir.test.ts src/commands.test.ts Tested: bun run build Tested: node dist/cli.mjs skills list --json Tested: node dist/cli.mjs skills show batch Tested: node dist/cli.mjs skills remove batch Tested: bun run smoke Tested: git diff --check * Stop config path leaks across tests The full PR check can load config-path helpers after tests have mocked homedir or changed global session state. Explicit CLAUDE_CONFIG_DIR now bypasses the default-home memoization cache, and SDK contexts now treat sessionProjectDir: null as an intentional context value instead of falling back to stale global state. Constraint: Keep default config-home resolution memoized for hot callers. Confidence: high Scope-risk: narrow Tested: bun test --max-concurrency=1 src/utils/openclaudePaths.test.ts src/utils/providerProfile.test.ts src/utils/knowledgeGraph.stress.test.ts tests/sdk/sdk-context-isolation.test.ts Tested: bun test src/cli/handlers/skills.test.ts src/skills/loadSkillsDir.test.ts src/commands.test.ts Tested: bun run build Tested: bun run smoke Tested: git diff --check * Stabilize config-sensitive tests in CI The PR check showed provider profile tests sharing process.env/CWD-sensitive state and a knowledge graph stress test assuming a fixed config-root persistence path. Mark the profile tests that mutate global process state as non-concurrent and assert the corrupted Orama rename relative to the actual persistence path under test. Constraint: Production behavior is unchanged; this only tightens test isolation. Confidence: high Scope-risk: narrow Tested: bun test --max-concurrency=1 src/utils/openclaudePaths.test.ts src/utils/providerProfile.test.ts src/utils/knowledgeGraph.stress.test.ts tests/sdk/sdk-context-isolation.test.ts Tested: bun test src/cli/handlers/skills.test.ts src/skills/loadSkillsDir.test.ts src/commands.test.ts Tested: git diff --check * Explain skill remove scope mismatches Removing a user-global skill without --global looked like a missing skill even though skills list showed it. Detect when the requested skill exists in the other local scope and print the exact removal command hint while keeping bundled/internal skills hidden as generic not found. Constraint: Bundled skills remain hidden from public skills commands. Confidence: high Scope-risk: narrow Tested: bun test src/cli/handlers/skills.test.ts src/skills/loadSkillsDir.test.ts src/commands.test.ts Tested: bun run build Tested: node dist/cli.mjs skills remove pr-review Tested: node dist/cli.mjs skills remove batch Tested: bun run smoke Tested: git diff --check * Clarify empty skills list state The public skills list now hides bundled/internal skills, so an empty result means there are no installed user or project skills. Use clearer copy to avoid implying internal skills do not exist. Constraint: Bundled skills remain hidden from public skills commands. Confidence: high Scope-risk: narrow Tested: bun test src/cli/handlers/skills.test.ts src/skills/loadSkillsDir.test.ts src/commands.test.ts Tested: bun run build Tested: node /home/anaxy/Projects/openclaude/dist/cli.mjs skills list from empty temp project Tested: bun run smoke Tested: git diff --check * fix(skills): preserve namespaced local installs * Keep skills CLI independent of provider startup Skills management commands need to work when provider configuration is broken, because they are local/script-friendly maintenance commands. Route skills subcommands before provider profile hydration and validation, including supported leading global flags such as --bare. Constraint: Provider startup validation must still run for normal interactive and provider-backed commands. Rejected: Import full main.tsx for the skills fast path | that loads optional bundled Chrome modules and re-couples the local skills path to interactive startup. Confidence: high Scope-risk: narrow Tested: bun test src/entrypoints/cli.skills.test.ts src/cli/handlers/skills.test.ts src/skills/loadSkillsDir.test.ts src/commands.test.ts Tested: bun run build Tested: CLAUDE_CODE_USE_OPENAI=1 OPENAI_BASE_URL=https://api.openai.com/v1 OPENAI_API_KEY= node dist/cli.mjs skills list Tested: CLAUDE_CODE_USE_OPENAI=1 OPENAI_BASE_URL=https://api.openai.com/v1 OPENAI_API_KEY= node dist/cli.mjs --bare skills list Tested: bun run smoke Tested: git diff --check * Preserve reviewed skill install hardening after rebase Rebasing PR #1162 onto current main flattened an earlier merge commit that carried reviewed Skill Hub hardening and regression coverage. This restores those final-tree changes as a normal linear commit so the rebased PR keeps the same behavior reviewers approved without retaining merge commits or mainline noise. Constraint: Keep the PR branch linear for maintainer review while preserving the reviewed final tree from the conflict-resolved integration branch. Rejected: Push the plain rebase result | it would drop registry sha256/version/trust metadata handling and associated tests from the reviewed PR state. Confidence: high Scope-risk: narrow Directive: Do not remove the registry sha256 requirement or install-path regression tests without another security review. Tested: final tree compared against fix-pr-1162-conflicts before verification * Fix skills CLI review follow-ups * Fix skills CLI review findings * Address skills CLI review follow-ups * Fix skills tests under bare-mode CI state * Clear bare argv in skills tests * Harden skills remove and loader tests * Pin cwd state in skills remove test * Use explicit project dir for skills removal * Avoid skill remove test name collision * Use fs abstraction for skills removal * fix skills CLI review findings * Fix skills CLI startup bypass and test isolation * Fix skills CLI review findings * Fix remaining skills CLI review findings * Fix skills CLI review findings --------- Co-authored-by: OpenClaude Worker 3 <worker-3@openclaude.local> Co-authored-by: jatmn <the@jat.mn> |