mirror of
https://github.com/Gitlawb/openclaude.git
synced 2026-11-06 20:58:14 -04:00
6d09260291595c2bfcd4cd50d6660ded342aa9b2
1214
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  
|
6d09260291 |
chore(types): reduce any usage with safety tooling + 7-batch phase 1 (#2208)
* chore(types): add any-usage baseline + lint budget gate (Phase 0)
Phase 0 of a multi-phase plan to reduce explicit `any` usage safely.
No source files modified in this PR.
- Add eslint@9 + @typescript-eslint deps (lint only, no fix rules).
- New minimal flat eslint.config.js enabling only
`@typescript-eslint/no-explicit-any` as warn. Stub plugins registered
for legacy `custom-rules`, `eslint-plugin-n`, and `react-hooks`
references in existing `// eslint-disable` comments so the comments
stay valid (no rule name resolution errors).
- New `scripts/any-usage-report.ts`: walks src/, tests/, scripts/ and
buckets `any` usage into `: any` annotations, `as any` casts,
`[key: ...]: any` index signatures, `// @ts-ignore` directives, and
generic `any` args. Emits `reports/any-usage.{json,md}`. `--check`
mode exits non-zero if the count grows vs the baseline.
- Lock the baseline at 1,031 occurrences across 198 files.
- Add `lint`, `lint:any-budget`, `check:strict` scripts.
- Plan: docs/plans/imperative-hugging-sphinx.md
Verification:
- `bun run typecheck` clean
- `bun run lint` exit 0 (845 warnings, 0 errors)
- `bun run lint:any-budget` ok (current=1031 baseline=1031)
* chore(types): narrow any in 6 small src files (Phase 1, batch 1)
Phase 1 of the plan to reduce `any` usage. Targets only files with
1–2 lint hits where the concrete type is reachable in scope.
- src/services/api/withRetry.ts: isQuotaExhausted signature `any` → `unknown`,
add local structural narrowing for `.message`/`.status` access.
- src/utils/validation.ts: assertFunction predicate signature `any[]`/`any` →
`unknown[]`/`unknown`.
- src/utils/conversationArc.ts: extractTextFromContent — replace `(block: any)`
in filter/map with explicit type predicate narrowing.
- src/utils/optionalRuntimeModule.ts: dynamic-import wrapper returns
`Promise<unknown>` instead of `Promise<any>`.
- src/entrypoints/mcp.ts: tighten block mapping with the existing structural
type already in scope; replace `(finalResult as any).isError` with a
narrow cast.
- src/components/EffortPicker.tsx: drop redundant `: any` on useAppState
selector — the function's generic infers `AppState` for free.
Verification:
- bun run typecheck clean
- bun run lint: 835 warnings (was 845)
- bun run lint:any-budget: 1022 / 1031 baseline (delta -9)
- bun test src/utils/optionalRuntimeModule.test.ts + conversationArc.test.ts:
36 pass / 0 fail
- bun test src/services/api/withRetry.test.ts: 44 pass / 0 fail
* chore(types): narrow any in WebSearchTool + adapters (Phase 1, batch 2)
Phase 1 continues. These are all boundary code (HTTP responses from
external search APIs), so every `any` is at a module boundary per the
plan. Each adapter now types its response with `unknown` + structural
narrowing instead of `any`.
- providers/types.ts: firstMatch/normalizeHit accept `unknown` and narrow
to `Record<string, unknown>` before index access.
- providers/timeout.ts: fetchJsonWithWebSearchTimeout returns
Promise<unknown>.
- providers/{bing,brave,jina,linkup,mojeek,tavily,you,exa,custom}.ts: each
adapter's response data is cast to a minimal shape (`{ results?: unknown }`
etc.), then array elements are narrowed to `Record<string, unknown>` with
`typeof` checks per field.
- WebSearchTool.ts: getCodexSources and extractCodexWebSearchFailure now
take `Record<string, unknown>`; nested `action`/`error`/`result` are
cast locally to `Record<string, unknown>` so `?.field` chains type-check.
Verification:
- bun run typecheck clean
- bun run lint: 805 warnings (was 835, -30)
- bun run lint:any-budget: 992 / 1022 baseline
- bun test src/tools/WebSearchTool: 138 pass / 0 fail across 9 files
* chore(types): narrow any in 6 mid-size src files (Phase 1, batch 3)
Phase 1 continues. Each replacement uses a precise structural cast
(`as { uuid?: string }` etc.) or `unknown` with typeof/Array.isArray
narrows at use sites.
- src/utils/settings/types.ts: zod preprocess callback `any` → `unknown`,
copy via `Record<string, unknown>`.
- src/entrypoints/sdk/v2.ts: initialMessages param/var `any[]` → `Message[]`;
local cast at the boundary with stripChainFields.
- src/entrypoints/sdk/query.ts: msg `.uuid` access uses
`as { uuid?: string }`; `fileHistoryCanRestore` etc. accept `UUID` (from
`crypto`); `mcpServerStatus` accesses `client.error` / `client.config`
directly since the union members already have them.
- src/commands/cache-probe/cache-probe.ts: getField reduce `any` → `unknown`
with typeof guard; catch (err: any) → `err: unknown` with
`err instanceof Error`; mainMakeUsage/mainConvertChunkUsage take `unknown`
and read fields via typeof checks.
- src/services/compact/snipProjection.ts: snipMetadata / uuid accessed via
narrow structural casts, no longer `as any`.
- src/utils/messages.ts: tool_use block destructuring for extra_content uses
`as { extra_content?: unknown; [k: string]: unknown }` instead of
`as any`.
Public SDK surface (entrypoints/sdk.d.ts, entrypoints/sdk/index.ts
SdkMcpToolDefinition<Schema = any>) is intentionally permissive for
consumer ergonomics and is left untouched per the plan's do-not-touch
list.
Verification:
- bun run typecheck clean
- bun run lint: 782 warnings (was 805, -23)
- bun run lint:any-budget: 969 / 992 baseline
- bun test (sdk v2/factories + compact + messages): 91 pass / 0 fail
* fix(types): correct BetaContentBlock cast in messages.ts (Phase 1 fix)
Verifier caught that the `as { [k: string]: unknown; extra_content?: unknown }`
cast on a `BetaToolUseBlock` did not sufficiently overlap (BetaToolUseBlock
lacks an index signature), so the destructured spread of `restBlock`
yielded a malformed union member that failed to assign to BetaContentBlock[].
Fix: cast through `unknown` first, then assert the final return value as
`BetaContentBlock` (the declared element type of the outer `.map`). This
narrows from the prior `as any` while keeping the previous structural
intent (preserve all block fields, optionally inject `extra_content`).
Verification:
- bun run typecheck clean
- bun test src/utils/messages/: 79 pass / 0 fail
* chore(types): narrow any in 5 dense src + 1 script (Phase 1, batch 4)
Each replacement preserves runtime behavior while using concrete types
where reachable and `unknown` with structural narrowing where not.
- src/components/ClaudeMdExternalIncludesDialog.tsx: 4 helper updaters
(acceptProject/User, declineProject/User) take `ProjectConfig` from
`src/utils/config.ts` instead of `any` (matches the
`saveCurrentProjectConfig` updater signature).
- src/components/ProviderManager.tsx: env-clearing objects in
`activateGithubProvider` and the GitHub disable path used
`undefined as any`. The intent is "delete this key on merge" but the
zod schema coerces to string. Replaced with
`undefined as unknown as string` (honest about the lie) and added a
comment explaining intent.
- src/services/compact/snipCompact.ts: introduced a structural
`SnipMessage` type for the public function params. Tests construct
fake messages with `uuid: 'u4'` (not branded UUID), so a strict
`Message[]` would reject them; the structural type accepts both
production messages and test fixtures. `snipCompactIfNeeded` is
generic `<T extends SnipMessage>` so callers get the same shape back.
Internal content blocks use `unknown` + type-predicate narrowing.
- src/query.ts: yield site for the boundary message casts to `Message`
since the value is constructed as a real `Message` in production.
- scripts/grpc-cli.ts: proto descriptor, stream, and message callback
typed as `unknown` / `ClientDuplexStream<unknown, unknown>`. The
dynamic `.proto` payload is cast once at the boundary into a
structural shape so per-field access (`text_chunk.text` etc.) is
tracked explicitly.
- scripts/generate-sdk-types.ts: the JSON-Schema-walking converter
takes `unknown` everywhere and narrows via typeof / Array.isArray at
each access. `Map<unknown, string>` for placeholder identity
comparison, `Set<string | undefined>` no longer used here.
Verification:
- bun run typecheck clean
- bun run lint: 728 warnings (was 782, -54)
- bun run lint:any-budget: 916 / 969 baseline
- bun test src/services/compact: 91 pass / 0 fail
* chore(types): remove 3 eslint-disable no-explicit-any suppressions
Tighten three small call sites where the `any` was used as a lazy
escape hatch and a real type was reachable in scope.
- src/main.tsx: `(global as any).require('inspector')` becomes
`(global as unknown as { require: (s: string) => { url: () => string } })`.
The dynamic require is being phased out, but while it remains the
narrow structural cast documents the shape we actually consume.
- src/ink/reconciler.ts: catch (error: any) → catch (error: unknown);
`error.code === 'ERR_MODULE_NOT_FOUND'` accesses the code via
`(error as { code?: unknown }).code`.
- src/components/wizard/WizardProvider.tsx:
`createContext<WizardContextValue<any> | null>(null)` →
`createContext<WizardContextValue | null>(null)`. The
`WizardContextValue` generic already defaults to
`Record<string, unknown>`, so the `<any>` was redundant; consumers
that need a tighter type can supply it via
`useContext<WizardContextValue<T>>`.
Note: most remaining lint warnings come from file-level
`/* eslint-disable @typescript-eslint/no-explicit-any */` blocks at the
top of do-not-touch files (src/types/{message,utils,tools}.ts,
src/constants/querySource.ts, src/entrypoints/sdk/controlTypes.ts) and
from public SDK surface (src/entrypoints/sdk{,.d.ts,v2.ts}). Both are
intentional per the plan.
Verification:
- bun run typecheck clean
* chore(types): refresh any-usage baseline
* fix(types): address PR review feedback on any-reduction tooling
- Pin eslint to resolved version (9.39.5) so lint warning totals stay
stable across lockfile updates.
- Wire lint:any-budget into the regular `check` gate (was only in
check:strict), so any-usage regressions block PRs.
- any-usage-report: use lstat and skip symlinks so a self-referencing
link under src/tests/scripts can't stall the budget scan.
- any-usage-report --check: exit nonzero when the baseline file is
missing or invalid so the gate can't be silently disabled.
- Remove dangling references to docs/plans/imperative-hugging-sphinx.md
from eslint.config.js and scripts/any-usage-report.ts headers.
- mcp.ts: defensively skip non-object tool-output entries (primitives
/ null) instead of crashing on boundary data.
- messages.ts: realign the mis-indented tool_search-off branch so the
blockExtra + return block reads at the right level.
No budget delta (913 -> 913). Typecheck, lint, budget, and the affected
unit tests all green.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* chore(types): narrow any in src/grpc/server.ts (5 warnings)
- protoDescriptor cast: replace `as any` with a structural type that only
declares the AgentService shape we actually use.
- handleChat: ServerDuplexStream<any, any> -> <unknown, unknown>; the
clientMessage is narrowed locally where it's read.
- previousMessages: any[] -> Message[] (already imported, already used
as the type of this.sessions).
- catch (err: any) -> catch (err); err.message is now gated behind an
instanceof Error check.
Typecheck, lint, budget, and the 2 grpc unit tests all green.
Lint 728 -> 723 (-5). Budget 913 -> 909 (-4).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* chore(types): narrow any in src/utils/knowledgeGraph.ts (18 warnings)
Add explicit LegacyEntity / LegacyRelation / LegacySummary / LegacyData
types so the migration path stops round-tripping through `any`.
- LegacySource.data, mergeLegacySources, normalizeLegacyData,
readLegacySqliteStore, SqliteReadResult.data, and doMigration all take
the new LegacyData shape.
- LegacyData.entities is typed `Record<string, LegacyEntity> | LegacyEntity[]`
because older JSON dumps stored entities as an array. normalizeLegacyData
converts the array form to the record form so downstream code can keep
using Object.entries. (Old loop relied on the same `id ?? entryKey`
fallback, so behavior is preserved; verified by all 20 unit tests.)
- SQLite row accessor now returns `Record<string, unknown>` (was `any[]`);
per-row readers narrow `row.id`, `row.attributes`, `row.keywords`,
`row.content` from `unknown` and the existing typeof guards preserve
the previous fail-soft behavior on malformed rows.
Lint 723 -> 705 (-18). Budget 909 -> 894 (-15). All 20 knowledgeGraph
unit tests pass; typecheck clean.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* chore(types): narrow SDK tool() generic and handler signature (12 warnings)
The public SDK MCP tool() helper previously defaulted its Schema generic
to `any` and typed handler args as `any`. Switch both to `unknown`:
- SdkMcpToolDefinition<Schema = unknown>
- tool<Schema = unknown> with handler (args: Schema, extra: unknown)
- annotations?: any -> ToolAnnotations (imported from MCP SDK types)
This is a strictly tighter API: callers who pass a concrete schema get
the same behavior; callers who relied on implicit any now have to
validate handler args (which they should have been doing anyway).
All 4 SDK MCP tool wiring tests pass; typecheck clean.
Lint 705 -> 695 (-10). Budget 894 -> 884 (-10).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix(types): address PR feedback on SDK + grpc type hardening
Apply 4 of 5 CodeRabbit review items (skipped item 5: SDK resume flow
JSONL validation, deferred as larger refactor).
- sdk.d.ts: switch MCP type imports to the package's public
`@modelcontextprotocol/sdk/types.js` subpath (was the internal
dist/esm/types.js path).
- sdk.d.ts: handler return Promise<any> -> Promise<CallToolResult> in
both SdkMcpToolDefinition and the tool() helper, matching the
runtime contract and the sibling declarations in sdk/index.ts and
sdk/v2.ts.
- grpc/server.ts: introduce a local ClientMessage interface (matching
the openclaude.proto oneof payload) and type the duplex-stream
request as ServerDuplexStream<ClientMessage, unknown>. The handler
body was already reading these fields by name; the type now
documents them.
- grpc/server.ts: ServiceDefinition generic was being passed a
ServerDuplexStream, which isn't a valid T (T must be a record
keyed by method name). Switch to ServiceDefinition<{ Chat: never }>
so the call to addService accepts a real implementation-map key.
Typecheck, lint, budget, and the 6 affected unit tests all green.
Lint 695 -> 693 (-2). Budget 884 -> 882 (-2).
* fix(types): address 6 reviewer findings on PR #2208
Six findings (four P2, two P3) from the second review pass.
1. knowledgeGraph legacy array normalization: previously dropped entries
whose `id` was neither string nor number, breaking recovery for
idless array entries. Use the array index as the fallback key so
mergeLegacySources' existing `id ?? entryKey` logic continues to
recover the entry. All 20 unit tests still pass.
2. SDK tool() generic conflated schema descriptor with invocation
value type, breaking the test fixture (and any external consumer)
that declares the handler with a concrete value type. Restore
`args: any` for the handler across the factory, SdkMcpToolDefinition,
and the .d.ts declaration; keep `Schema` as the generic for
inputSchema typing.
3. sdk.d.ts now declares ToolAnnotations and CallToolResult locally
instead of importing from @modelcontextprotocol/sdk, preserving
the optional peer contract. A consumer without MCP installed no
longer gets TS2307 on a no-op SDKSession type import.
4. any-usage-report now strips comments, string literals, and
template-literal contents before applying the annotation regex,
so `// example: any`, `': any'`, and similar non-type occurrences
no longer count. @ts-ignore / @ts-expect-error / @ts-nocheck
directives are preserved for the ignore category. Budget went
from 882 to 776.
5. generate-sdk-types: schemas module cast as Record<string, () => unknown>
was wrong because HOOK_EVENTS is a non-callable; cast as
Record<string, unknown> and narrow per-access. def.getter() and
Reflect.get(schema, 'description') both narrowed to a callable /
object shape before use. Generated output unchanged (diff empty).
6. any-usage-report walk(): readdir failures now propagate instead
of being silently swallowed, so a partial scan cannot publish a
too-low replacement baseline or pass --check.
Typecheck, lint, budget, and 26 affected unit tests all green.
* chore(types): refresh any-usage baseline after upstream rebase
Baseline 776 -> 779. The +3 comes from upstream #2196 (Command Code
gateway) test mocks merged to main after the baseline was recorded:
2x (payload: any) in ProviderManager.test.tsx, 1x 'as any' in
providerProfiles.test.ts. Symptom-level narrowing of another merged
PR's test fixtures is out of scope; re-baseline to the rebased tree.
---------
Co-authored-by: Gravirei <gravirei@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
|
||
 0xfandom
|
35d7edc2f6 |
fix(api): validate API_TIMEOUT_MS in the native Anthropic clients (#2169)
* fix(api): validate API_TIMEOUT_MS in the native Anthropic clients
The native Anthropic client path parsed API_TIMEOUT_MS with a raw
parseInt: `parseInt(process.env.API_TIMEOUT_MS || String(600 * 1000), 10)`
in client.ts and `parseInt(process.env.API_TIMEOUT_MS || '', 10)` in the
non-streaming fallback. Because `||` only substitutes the default for an
empty/unset value, any non-empty malformed value ("abc", "25ms") is truthy
and parses to NaN, and a negative value passes straight through. A NaN or
negative timeout on the SDK client is undefined behavior, and an oversized
value overflows the 32-bit setTimeout ceiling into a tiny/immediate delay.
The OpenAI-compatible shim already routes API_TIMEOUT_MS through
getApiTimeoutMs, which rejects non-digit/negative input, falls back to a
default, and clamps to the 32-bit ceiling. Reuse that single parser in the
native clients too. getApiTimeoutMs gains an optional fallbackMs argument so
the non-streaming path keeps its context-specific default (120s on remote,
300s otherwise) while still validating and clamping the override; existing
shim callers are unaffected (the default remains 600s).
Tests cover the fallback argument in the parser and assert the constructed
native client's timeout for malformed, negative, zero, unset, valid, and
oversized values.
* test(api): cover the non-streaming fallback timeout and pin route env
Add direct coverage for getNonstreamingFallbackTimeoutMs, which the previous
tests only exercised indirectly through the shared parser. It now asserts the
context-specific defaults survive the change (300s local, 120s remote) for
missing and invalid API_TIMEOUT_MS, that a valid override is honored in both
contexts, and that an oversized one is clamped. The function is exported for
this; test files are knip entry points, so it does not read as dead code.
Also clear every provider route-selection variable explicitly in the native
client timeout test. The suite's afterEach already restores them, but pinning
them up front keeps an inherited CLAUDE_CODE_USE_OPENAI (or another route
flag) from diverting getAnthropicClient into the shim and measuring the wrong
client's timeout.
* test(api): pin the native client path and document the timeout contract
The native timeout test cleared many provider variables but left
NEARAI_API_KEY untouched, and the suite never snapshotted it at all.
NEARAI_API_KEY alone selects the env-only Near AI route, and clearing the
competing vendor keys removed the very credentials whose presence suppresses
it, so an inherited value routed getAnthropicClient to the OpenAI shim. The
shim is returned as a cast object with no SDK timeout, so the assertions read
undefined and the regression silently stopped being covered. CLINE_API_KEY had
the same gap.
Snapshot, clear, and restore both alongside the other route-selection inputs,
and assert the constructed object is an Anthropic instance before reading its
timeout, so a stray variable or a future env-only route fails loudly instead
of weakening the test. Verified by injecting NEARAI_API_KEY: the guard now
fails on the shim object rather than passing on undefined.
Also refresh the API_TIMEOUT_MS reference, which still said the Anthropic SDK
kept its own timeout handling. It now describes both consumers: the
headers-deadline semantics for OpenAI-compatible paths, and the native
Anthropic SDK request timeout plus the non-streaming fallback's own defaults
(120s remote, 300s otherwise), under the same validation and clamping.
|
||
 Bogdan
|
decad0e172 |
fix(openai-shim): request streaming usage from compatible local providers (#2144)
* fix(openai-shim): request streaming usage from local providers * test(openai-shim): verify cached streaming usage accounting * fix(openai-shim): retry strict streams without usage options |
||
|
0xfandom
|
cbbe10ff8a |
fix(frontmatter): anchor the closing --- to the start of a line (#2027)
* fix(frontmatter): anchor the closing --- to the start of a line The opening delimiter was anchored but the closing one was not, and [\s\S]*? is lazy, so parsing stopped at the first `---` appearing anywhere -- including inside a value. A description reading "Reviews code --- thoroughly" ended the block early: the value was truncated and the remaining frontmatter lines plus the real delimiter leaked into the body that is sent to the model. A `---` line inside a block scalar did the same. Requiring the captured block to be empty or to end at a line break pins the close to a line start. The m flag would also work for the close but would un-anchor the open, letting a horizontal rule partway down a body be read as frontmatter. Tightened the trailing run to spaces and tabs as well, since \s* matched a newline and accepted a bare `---` with no line terminator after it. Affects every .md frontmatter consumer: agents, skills, slash commands, output styles and memory files. * test(frontmatter): fold the delimiter cases into the existing suite A separate test file adds a slot to bun's sequential file ordering, which shifted which suite runs before which and surfaced an unrelated mock leak in CI (taskReport git metadata and the /ads command failed on both Node versions, neither touching frontmatter). The cases belong next to the existing parser coverage anyway. * test(frontmatter): cover tabs after both delimiters * fix(frontmatter): accept CRLF delimiter lines Windows-authored Markdown terminates the `---` delimiters with `\r\n`. The `[ \t]*\n` matcher rejected the carriage return, so such files were treated as having no frontmatter. Accept an optional `\r` on the open, captured body, and close line endings. * fix(frontmatter): keep consuming blank separator lines after the close Anchoring the closing delimiter narrowed its trailing match from `\s*` to just the delimiter's own line ending, so a conventional blank line between the frontmatter and body survived as a leading newline in the parsed content. Skill and plugin-command bodies are fed into prompts untrimmed, so that became a stray newline in the model instructions. Consume trailing whitespace-only lines again after the (still strictly validated, line-terminated) close, which cannot revive the mid-document false close since it only runs once a valid terminator matched. * fix(frontmatter): close empty frontmatter at the first delimiter line The anchored close made the captured block's outer quantifier greedy, so empty frontmatter followed by a body horizontal rule (`---\n---\n---`) read the second delimiter as YAML and closed on the third, swallowing the rule. Make the outer capture lazy so an empty block closes at the first `---` line as it did before anchoring; real frontmatter still matches on backtrack. Fixes the same shape hard-failing `claude plugin validate`. |
||
|
0xfandom
|
c3930ca5ad |
fix(mcp): resolve server names by own-property, not the prototype chain (#1983)
* fix(mcp): resolve server names by own-property, not the prototype chain
getMcpConfigByName and the mcp remove handler look names up in plain
object maps built from JSON config with a bare servers[name] truthiness
check. Reserved names ('constructor', '__proto__', 'toString', …) resolve
to inherited Object.prototype members, which are truthy, so:
- `mcp get constructor` skips the not-found guard, prints a fabricated
record, and hands the Object constructor to the health check;
- `mcp remove constructor` reports the reserved name as present in
multiple scopes and prompts to pick one instead of "No MCP server found".
The same leak reaches runAgent and the print handlers, which all route
through getMcpConfigByName. Gate every lookup on Object.hasOwn so only
real, own-property server names resolve.
* test(mcp): restore project mcpServers in afterEach for isolation
Save and restore getCurrentProjectConfig().mcpServers the same way the
global config is handled, rather than hardcoding undefined, so the test
never leaks state into suites that share the in-memory test config.
* fix(mcp): cover the remaining prototype-name mutation and doctor paths
The own-property gate only covered unscoped removal and getMcpConfigByName.
The scoped paths still used bare lookups:
- removeMcpConfig's project/user/local existence checks accepted inherited
members, so 'mcp remove constructor -s user' reported a successful removal
while leaving the configuration untouched;
- addMcpConfig's already-exists checks rejected valid 'mcp add constructor';
- doctor's servers[name] and activeServers[name] fabricated definitions for
prototype names.
Gate all of them on Object.hasOwn. Also reject the name '__proto__' at add
time: it passes the character check but assigning it on a plain object hits
the prototype setter instead of creating an own property, so the server
would be reported as added and silently vanish.
Restore NODE_ENV in the test teardown alongside the config state.
* fix(mcp): report a reserved __proto__ entry in file config
A hand-authored .mcp.json can contain a server named "__proto__": JSON.parse
gives it a real own key, but it cannot be copied onto a plain object, so the
schema's rebuild dropped it before validation ever ran. The entry simply did
not exist and nothing said why, and with own-property lookups the scoped
removal path now correctly reports it as absent -- so there was no way to
learn the name was the problem.
Detect it on the raw parsed config and surface a fatal validation error
naming the entry, matching the rejection addMcpConfig already performs for
the same name. The rest of the file still parses.
* test(mcp): serialize the proto-name suite on the shared mutation lock
The suite swaps NODE_ENV and the process-wide global and project MCP
configurations without holding sharedMutationLock, so bun can run it
alongside another state-mutating file: that file observes the injected
realserver/locallyreal fixtures, or its own updates are overwritten when
this teardown restores its snapshots.
Acquire the lock in setup and release it from a finally in teardown, the
same shape officialRegistry.test.ts uses.
Also cover the reserved __proto__ entry arriving from parsed file config.
* fix(mcp): reserve constructor alongside __proto__
With own-property lookups in place, `mcp add constructor` persists an entry
the config schema then rejects -- and it rejects the whole mcpServers
object, so the newly added server is unusable and every other server in that
scope stops loading with it.
Refuse the name at add, and name it at file ingress: the only diagnostic
before was a generic "does not adhere to schema" against `mcpServers`, which
does not say which entry is at fault. The scan runs ahead of the schema
parse because that path returns early. `__proto__` and `constructor` are the
only two names that fail a write/read round trip -- toString,
hasOwnProperty, valueOf and the rest all persist and read back correctly.
* fix(mcp): reject a config with a reserved server name outright
parseMcpConfig flagged a fatal reserved name but, for __proto__, still returned
a usable partial config because zod's rebuild silently drops the key and the
schema parse succeeds. Callers that branch on the config -- the --mcp-config
ingress in main.tsx -- then took the partial config and discarded the error,
starting with the bad entry quietly gone. Return config: null whenever a fatal
reserved-name error is present, matching the constructor path where the schema
already fails, so the invalid input is rejected at every consumer.
* fix(mcp): keep reserved-name findings visible in all-server doctor output
doctorAllServers builds server reports only for names that survived parsing or
are active, and attaches server-keyed findings to those. A fatal reserved-name
error is keyed by a name that never survives parsing, so its finding was built
into no report and dropped -- mcp doctor --config-only read clean while the
invalid config was present. Surface any finding whose server has no report as a
global finding.
* fix(mcp): do not mutate a fatally poisoned project .mcp.json
A reserved server name (`__proto__`/`constructor`) now fails the whole
parse and returns config: null, so getProjectMcpConfigsFromCwd reports an
empty map even though valid siblings remain on disk. Project-scope add
rebuilt the file from that empty snapshot and wrote only the new server,
silently dropping every sibling; project-scope remove reported a valid
sibling as "not found". Refuse to mutate the file when it fails to parse,
naming the offending entry, instead of treating the empty map as truth.
* fix(mcp): surface reserved-name errors in single-server doctor output
doctorServer set report.findings to only the global findings and never
applied the orphan fold doctorAllServers uses, so a fatal reserved-name
error in a poisoned scope was dropped when the requested target lived in
another scope -- exit 0 with the real fault hidden. Promote findings for
names other than the reported one, and skip the state.not_found branch
when a validation finding already explains the requested name so the
report no longer carries two blocking findings with contradictory text.
* fix(mcp): honor the fatal-parse contract across every scope
The config:null contract was only enforced on project-scope .mcp.json.
Extend it so a fatally poisoned scope is never silently mutated:
- Re-check the project snapshot immediately before the write rebuild, not
only at the existence check -- the file can go fatal in between, and
rebuilding from the resulting empty map would drop valid siblings.
- Refuse user/local add and remove when that scope parses to config:null,
so the CLI no longer reports success for a scope that then loads nothing
(or mutates a sibling in a raw map that stays unloadable).
- Gate enterprise exclusive control on the managed file's presence, not on
a successful parse, so a poisoned managed-mcp.json can't silently drop
the policy lock and let user/project servers take over.
Generalize the guard to assertMcpScopeWritable(errors, scopeLabel).
* test(mcp): pin allowedSettingSources so the local-scope guard is hermetic
getMcpConfigsByScope() returns an empty writability-error list when a scope's
setting source is disabled, and allowedSettingSources is a process-wide global
other suites mutate. In bun's file order a suite that leaves localSettings
disabled (e.g. one exercising --setting-sources) would make the 'fatally
poisoned local scope' case skip its guard, so addMcpConfig(...'local') resolved
instead of rejecting -- the intermittent CI failure at config.protoName.test.ts.
Pin the full source set in beforeEach and restore it in afterEach, matching the
NODE_ENV and MCP-config snapshotting this suite already does.
* fix(mcp): guard scope mutations on raw parse errors, not the source filter
assertMcpScopeWritable read errors from getMcpConfigsByScope()/
getProjectMcpConfigsFromCwd(), which suppress errors to an empty list when a
scope's setting source is disabled. addMcpConfig/removeMcpConfig still mutate
the raw mcpServers maps, so under a narrowed --setting-sources set a fatally
poisoned scope could be written or deleted (clobbering valid siblings) while the
CLI reported success. Add getScopeMutationErrors(), which parses each scope's
raw source directly, and route every write guard through it.
* fix(mcp): detect unscoped-remove membership from parsed scope views
Unscoped `mcp remove` read local/user membership from the raw
getCurrentProjectConfig()/getGlobalConfig() maps while project used the parsed
view. A fatally poisoned local/user scope has no loadable servers yet still
holds the raw entry, so the handler listed the server as living there and then
removeMcpConfig refused with 'Cannot modify … config'. Read all three scopes
through getMcpConfigsByScope() so detection matches what is actually removable.
* fix(mcp): read the raw project scope for mutations, not the source view
getScopeMutationErrors only covered the write guard's errors; the project
existence check and .mcp.json rebuild still read servers from the
source-gated getProjectMcpConfigsFromCwd(). With projectSettings excluded from
--setting-sources that returned an empty map, so an add rebuilt the file
without the valid siblings and a remove mis-reported the server as absent.
Drop the source gate from getProjectMcpConfigsFromCwd() (a mutation-only read)
so it always sees the real file, and fold the project write guards back onto
its returned errors. Regression covers add/remove with projectSettings off.
* test(mcp): re-pin the poisoned local fixture atomically per mutation
The local-scope poison regressions flaked under the full parallel suite:
sharedMutationLock serializes other lock holders but not every writer of the
process-wide test project config or NODE_ENV, so a stray async task from another
suite could clobber the fixture during an await gap between the setup and the
assertion. addMcpConfig/removeMcpConfig reach their scope guard with no awaited
work in between, so re-establishing NODE_ENV, the enabled sources, and the
poisoned config synchronously immediately before each mutation (pinPoisonedLocalScope)
keeps the guard's read atomic regardless of what ran during earlier awaits.
* fix(mcp): surface managed-mcp.json parse errors in enterprise mode
Enterprise exclusive mode engages on the managed file's presence, so a
malformed or reserved-name managed-mcp.json fail-closes every other MCP source.
getClaudeCodeMcpConfigs returned errors: [] for that branch, and
getSettingsWithAllErrors aggregated only user/project/local, so startup and the
MCP UI saw an empty server list with no diagnosable reason. Propagate the
enterprise scope's parse errors as generic-error PluginErrors from the exclusive
branch and include the enterprise scope in the settings error aggregation.
* fix(mcp): detect unscoped-remove membership source-filter-independent
Reading membership from getMcpConfigsByScope applied the --setting-sources load
filter, so an unscoped `mcp remove foo` reported "No MCP server found" for a
clean user/local/project entry that removeMcpConfig(foo, scope) still mutates
when that source is narrowed out. Read each scope the same way removeMcpConfig
resolves existence -- the raw getCurrentProjectConfig()/getGlobalConfig() maps
and getProjectMcpConfigsFromCwd() -- still own-property gated so inherited
members are not treated as present.
* fix(mcp): surface managed-mcp.json errors in headless startup
The headless (-p) MCP consumer destructured only servers from the
config promise and discarded errors. A fatal managed-mcp.json
fail-closes every file-based MCP source, so scripted users saw an empty
server list with no diagnostic — indistinguishable from an intentionally
empty configuration. Emit the config errors on stderr in non-interactive
sessions (interactive already surfaces them via the MCP error UI).
* fix(mcp): enforce enterprise-exclusive boundary in named lookups
getMcpConfigByName() gated plugin-only mode but not the enterprise-
exclusive policy. A present managed-mcp.json takes exclusive, fail-closed
control of MCP (its mere presence engages the policy, even when the file
is malformed), and getClaudeCodeMcpConfigs() drops every non-enterprise
source accordingly — but the named lookup fell through to the user/
project/local maps. A user with a locally configured server plus an
invalid managed file could therefore still resolve that server by name
via 'mcp get <server>' (checkMcpServerHealth) or an agent definition's
named-server reference (runAgent), bypassing the intended policy.
Return only an enterprise-owned entry (or null) whenever
doesEnterpriseMcpConfigExist() is true, before consulting other scopes,
mirroring the boundary getClaudeCodeMcpConfigs() already enforces.
|
||
 fancivez
|
d16318a47f | fix(xai): validate OAuth state before settling callbacks (#2228) | ||
 JATMN
|
e2b021d8bb |
fix(plugins): keep Windows marketplace cache on copy ENOENT (#2220)
* fix(plugins): keep marketplace cache when Windows copy hits ENOENT (#2183) ChromeDevTools marketplace add cloned successfully then failed finalize because recursive fs.cp throws on unreadable nested files. Keep the validated clone and join tilde plugin-cache paths so Windows does not glue the home directory onto .openclaude. * fix(plugins): keep marketplace cache if dest cleanup throws (#2183) Copy-fallback recovery still failed add when removing a partial dest threw, and getMarketplace refetch did not persist a keep-temp cachePath. * fix(plugins): use persisted installLocation after marketplace refetch (#2183) Keep-temp recovery can change cachePath on disk while same-call getPluginById and install-by-name still returned the pre-refetch path. * fix(plugins): persist keep-temp installLocation only for remote sources getMarketplace refetch was rewriting file/directory installLocation to the marketplace root, which can make later remove delete a user directory. |
||
|
JATMN
|
055e5d551a |
feat(launcher): support --max-old-space-size-percentage (#2219)
* feat(launcher): support --max-old-space-size-percentage heap sizing Convert Node's percentage heap flag into --max-old-space-size from constrained or total RAM so small containers and large workstations can avoid the fixed 8192 MB cap on every supported Node version. Fixes #2213 * fix(launcher): reject unconstrained memory sentinels in heap percentage Ignore libuv UINT64_MAX constrainedMemory values, clamp sub-megabyte percentage results to 1 MB, and keep spaced heap-flag values on background Node launches. * fix(launcher): report unresolved heap percentage instead of silent MB fallback A valid percentage with unknown available memory no longer looks like an unrelated megabyte env or default success; the launcher warns once, docs state full precedence, and child NODE_OPTIONS tests call the real helper. * fix(launcher): strip launcher heap flags when Node already has a cap Commander rejected --max-old-space-size-percentage on native heap+gc and disable-relaunch paths. Strip those argv tokens before import, and do not treat a following non-percentage word as the flag value. * fix(launcher): propagate execArgv heap caps to subprocess NODE_OPTIONS Child tools inherit NODE_OPTIONS, not the parent process.execArgv. When the CLI already started with a numeric --max-old-space-size cap there, copy that value into NODE_OPTIONS instead of defaulting to 8192 MB. Also strip launcher-only heap flags on the OPENCLAUDE_HEAP_RELAUNCHED early-return path and harden launcher integration tests with Node 22-safe execArgv fixtures. |
||
 
|
2e687df4ec |
fix(image): allow large/metadata-less screenshots through on resize failure (#1964) (#1970)
* fix(image): allow large/metadata-less screenshots through on resize failure (#1964) Stop Alt+V paste from falling through to "No image found in clipboard" when the native image connector returns undefined metadata or a screenshot exceeds 1568px. - Reorder the metadata guard so an undefined metadata object is handled before reading metadata.format (previously threw TypeError and was swallowed into a "no image" result). Fall back to magic-byte format detection. - Drop the `!overDim` gate from the catch-block allow-through so a dimensionally-large but base64-small image passes through. The API resizes images >1568px server-side; only the base64 size is a hard limit. - Keep IMAGE_MAX_WIDTH/HEIGHT at 1568; only the crash and gate logic changed. - Add imageResizer.test.ts covering the undefined-metadata path, the overDim<=5MB allow-through regression, the >5MB throw, and the happy path. * fix(image): enforce many-image dimension limit in resize-failure fallback (#1970) When the native image processor crashes, the fallback previously returned the original buffer unchanged whenever its base64 size was <= 5MB, with no dimension check. That let oversized images (e.g. 3840x2160 PNG/JPEG/WebP) stay in conversation history and later break many-image requests with a 400 "image dimensions exceed ... many-image" error. - Add IMAGE_MANY_IMAGE_MAX_WIDTH/HEIGHT (2000px) to apiLimits. - In the resize-failure fallback, detect images exceeding the 2000px many-image limit via magic-byte dimension parsing (PNG IHDR, WebP VP8/ VP8L, JPEG SOF0/2/3, GIF screen descriptor) and, when possible, downsample with platform Canvas APIs (awaiting a fully-decoded source). If downsample is unavailable or the result still exceeds the 5MB payload budget, reject with ImageResizeError instead of returning the oversized original. - Unknown/unparseable dimensions fail safe (treated as exceeding the limit). - Correct two pre-existing WebP dimension-offset bugs surfaced during review. Adds regression tests for PNG/JPEG/WEBP/GIF over the many-image limit. * fix(image): address review - media-type subtype, WebP offsets, Canvas DOM resolve, metadata-less budget - ResizeResult.mediaType is the subtype ('png'/'jpeg'), not a full MIME type. The Canvas downsample branch wrongly returned 'image/png'/'image/jpeg'; corrected to 'png'/'jpeg' to match every other return in the file. - WebP dimension parser corrected against real sharp-encoded frames: VP8L transform header reads from byte 21 (bits[0..13] width, [14..27] height); VP8 lossy width/height read from bytes 26/28. Previous offsets were off and produced wrong dimensions. - Canvas fallback now resolves createElement/Image from document (browser/ Electron) with a globalThis fallback, so it actually engages where Canvas is available. - metadata-less + oversized branch now tries progressive JPEG quality [80,60,40,20] and only returns an in-budget result; otherwise fails via the user-facing limit error instead of returning an oversized buffer that validateImagesForAPI would reject (the regression the PR recovers from). - Add WebP/VP8L fixtures and tests asserting exact parsed dimensions and in-limit acceptance. * fix(image): address 4 P1 review findings in resize-failure fallback - Metadata-less branch now enforces the many-image 2000px limit via a shared enforceManyImageDimensionLimit helper (extracted from the catch-path guard), instead of letting compact high-resolution screenshots pass through unchecked. - VP8 lossy WebP dimensions are stored directly (no -1 bias per RFC 6386); only VP8L uses minus-one encoding. Fixed parser and test fixture. - Add VP8X (extended WebP) dimension parsing, needed for alpha/animation. - DOM Image fallback now constructs with no args and assigns src after installing onload/onerror, since the Image constructor takes width/height, not a URL. * fix(image): require raw dimensions before canvas downsample in many-image guard readImageDimensions can return null for unparseable buffers; falling back to 0,0 made tryDownsampleToManyImageLimit treat the image as 1x1 and silently downsample to a single pixel instead of throwing ImageResizeError. * fix(image): keep in-budget 4K pastes when Canvas resize fallback is unavailable The many-image 2000px guard rejected Windows clipboard screenshots whenever image_processor_napi failed and Canvas was absent, recreating #1964. Pass those images through, invoke DOM canvas methods with their owners, and skip JPEG fill/TEM bytes before reading segment lengths. * fix(image): reject API-hard 8000px edges and surface paste resize errors Passthrough after a processor crash was sending images the API hard-rejects and still mapping ImageResizeError to "No image found". Keep 4K single-image paste, fall through from createImageBitmap to Image, and show the real resize error on Alt+V. * fix(image): document paste resize errors and test native rethrow Clipboard ImageResizeError was still easy to treat as a missing reader on the native path, and the Alt+V vs generic notification contract had no focused tests. Share the paste error helpers, document the 5MB/8000px prompt, and cover native resize rejection without enabling NATIVE_CLIPBOARD_IMAGE. * test(image): stabilize clipboard mock module identity * fix(image): close resize fallback review gaps * test(image): cover JPEG canvas fallback subtype * fix(image): downsample before enforcing API edge * fix(paste): settle rejected image path reads * test(paste): isolate rejection regressions * fix(image): report payload limit after fallback * test(image): restore mocked modules after suites Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(images): enforce many-image limits at Anthropic request boundary Count retained history and tool-result images, verify resized payload dimensions, and report actionable failures before dispatch. Exercise mock restoration deterministically. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(images): preserve resize failure diagnostics at request boundary The many-image validation catch replaced the original image-processing error with a generic RequestImageDimensionsError without logging it or keeping a cause chain, so logAPIError and the assistant error path only ever saw the wrapper message. - Log the underlying processing failure via logError before wrapping. - Rethrow an existing RequestImageDimensionsError unchanged instead of reconstructing it. - Attach the original error as the wrapper's cause; ImageResizeError and RequestImageDimensionsError now forward ErrorOptions to super. - Expand usesAnthropicImageLimits coverage to the reachable isFirstPartyBaseUrl:false input for bedrock/vertex/foundry. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix: count partner documents in request image limits --------- Co-authored-by: jatmn <the@jat.mn> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> |
||
 alexverify
|
eb3c5902eb |
feat(skills): add verify for revocations and eyebrow drift checks (#2215)
* feat(skills): add verify for revocations and eyebrow drift checks * apply code reviews fixes * fix(skills): read execute bits through the fs seam, skip symlink tests where unsupported * fix(skills): check X_OK through the fs seam, report symlink tests as skipped |
||
  fancivezandfancivez
|
0b68e579b9 |
fix(profiles): apply context limit to all profile models (#2201)
* fix(profiles): apply context limit to all models * fix(profiles): include saved model in context limits * test(profiles): cover saved model context lifecycle * fix(profiles): align context limit keys with runtime models --------- Co-authored-by: fancivez <fancive@gmail.com> |
||
|
alexverify
|
1afeb4b1e7 |
docs: add a skills guide (#2211)
* docs: add a skills guide * docs: imprvements on skills doc * docs(skills): address review on the skills guide * docs(skills): correct verify exit code descriptions * docs(skills): state what each check covers * docs(skills): align validate wording and name each install path condition |
||
|
GravireiandGravirei
|
ec850d2d1a |
docs(web): add llms.txt for LLM discovery (#2205) (#2209)
* docs(web): add llms.txt for LLM discovery (#2205) Hosts an llmstxt.org-spec summary of OpenClaude at the website root so LLMs (ChatGPT et al.) have an authoritative source and stop conflating the project with unrelated tools named "Openclaw". * docs(web): address review feedback on llms.txt - Clarify the License entry: MIT applies only to OpenClaude contributors' modifications, not the underlying Claude Code (Anthropic proprietary). Addresses Greptile's misstatement flag. - Convert the bare npm one-liner in the Install section into a Markdown linked list entry per the llmstxt.org spec. Addresses CodeRabbit's "link-only sections" note. * fix(web): make AUR entry in llms.txt hyperlink-first The 'AUR package: [openclaude](...)' entry put text between the list marker and the hyperlink, which makes the reference llms.txt parser fail with AttributeError ('NoneType' has no attribute 'groupdict') and aborts parsing of the whole document. Use link-first syntax preserving destination and meaning. Verified with the reference parser (parse_llms_file): 4 sections, 21 links, all URLs correct. web:build passes and web/dist/llms.txt matches source byte-for-byte; web:typecheck clean. --------- Co-authored-by: Gravirei <gravirei@users.noreply.github.com> |
||
|
JATMN
|
0abfca30e5 |
fix(sdk): preserve async generator session context (#2204)
* fix(sdk): preserve async generator session context * fix(sdk): complete async context boundaries * test(sdk): assert queryAsync init isolation and close V2 sessions. Replace the source-text queryAsync check with a behavioral observation of init, and release concurrent V2 sessions so the async-context coverage matches the runtime contract. |
||
|
JATMN
|
3609d751c0 |
fix: identify OpenCode Go requests (#2203)
* fix: identify OpenCode Go requests * fix: harden OpenCode Go identity headers * fix: pin OpenCode session across retries |
||
|
JATMN
|
5b9daef605 |
feat(providers): add Command Code hybrid gateway (#2196)
* feat(providers): add focused Command Code hybrid gateway Register Command Code as a descriptor-first OpenAI-compatible aggregating gateway with a hybrid catalog, dedicated CMD_API_KEY credentials, and the same surgical sibling wiring used by LLMTR. * fix(providers): reject Command Code placeholder keys and restore saved secrets on relaunch Treat CMD_API_KEY placeholders as unusable at request time, and migrate a profile-owned persisted OPENAI_API_KEY into CMD_API_KEY on canonical Command Code restart when dedicated keys are missing. * fix(providers): re-apply Command Code when the dedicated key drifts Profile apply already writes CMD_API_KEY, but alignment only compared OPENAI_API_KEY, so a stale dedicated secret could keep winning request auth. * fix(providers): isolate Command Code from leftover OpenAI keys Selecting --provider commandcode left a distinct generic OPENAI_API_KEY in-process because Command Code shared LLMTR's fallback path. Mirror the dedicated credential or clear the generic key on the canonical endpoint. * fix: enforce Command Code transport contract * fix: apply model override before validation * fix: support inline model overrides * fix: align early model parsing with CLI * fix: address Command Code review feedback * fix: validate Command Code profile models * fix: keep startup model display in sync * fix: avoid echoing rejected Command Code models * fix: share effective model flag parsing * fix: preserve teammate provider overrides * fix: isolate command code startup state * fix: align command code startup state * fix: close command code route gaps * fix: preserve teammate route precedence * fix: align launcher and root option parsing * fix: preserve model and credential semantics * fix: reject Claude aliases on Command Code * fix: preserve root --model after a consumed delimiter Required-value options such as --system-prompt consume a following -- token, so early Command Code model recovery must scan the full argv instead of truncating at the first delimiter. * fix: reject Command Code Claude models in /model and lists The shared Chat Completions predicate only inspected whole strings, and /model still applied OpenClaude aliases. Reject list members and block those selections on the Command Code route. * fix: ignore nested --model when applying --provider Command Code preflight was last-wins on full argv, so a nested aimlapi/auto-mode --model could abort provider selection even when the root session model was valid. * fix: unmount Ink if /model wait times out The Command Code /model tests left the Ink instance registered when waitForCondition rejected, which can leak into later tests. * fix: enforce Command Code contracts at profile write time Reject incompatible models and capture dedicated CMD credentials in toProfile so Console OAuth env adoption and other add/update callers cannot persist Chat Completions-invalid Command Code state. Align background session metadata with the same root --model argv slice used at CLI startup. * fix: persist Command Code dedicated keys over leftover OpenAI env Env adoption that copies OPENAI_API_KEY must not store a generic secret on a canonical Command Code profile when CMD_API_KEY is present. Document that the route still requires explicit selection. * fix: test background session root --model from launch argv Replace the source-text assertion with the shared metadata helper used by handleBgFlag so nested aimlapi/auto-mode --model flags cannot silently become the recorded session model. * fix(commandcode): unify route validation contracts * fix(commandcode): preserve explicit profile credentials * fix(cli): preserve explicit provider routing * fix(cli): align provider option consumers * fix(commandcode): align credential and discovery contracts * docs(commandcode): clarify Claude model support * fix(bg): align persisted provider metadata * fix(commandcode): preserve model and base defaults |
||
|
alexverify
|
0ea8eefbd0 |
feat(skills): enforce registry revocations.json on install (#2187)
* feat(skills): enforce registry revocations.json on install The registry ships a revocations.json kill switch that nothing reads. Installs now fetch it from beside the registry (or OPENCLAUDE_SKILLS_REVOCATIONS_URL) and refuse a skill whose id, version, or sha256 matches an entry. A missing list revokes nothing; a malformed list fails the install. * fix(skills): fail closed and validate revocation entries Review follow-up: only a confirmed-absent revocation list (ENOENT or HTTP 404) is treated as empty; other read failures block the install. The list must be a JSON array and every entry is schema-checked (non-empty id/version strings, 64-hex sha256, at least an id or a digest). Adds tests for the OPENCLAUDE_SKILLS_REVOCATIONS_URL override, invalid entries, non-array lists, and unreadable lists. * docs(skills): add docstrings to revocation helpers * test(skills): isolate revocation override and cover remote failure paths |
||
 euxaristia
|
3451187aed |
fix(ctx): let explicit overrides beat discovered context windows (#2082)
* fix(ctx): prefer known model limits over generic 128k discovery Proxies such as OmniRoute often report context_length 128000 when per-model metadata is missing. That discovery value was beating known descriptors and settings.modelLimits, so 1M models auto-compacted against a 128k budget. Prefer larger known descriptors over the generic 128k discovery default, and let settings/env overrides beat discovery. Document the override path in /set-context-window help. Refs #2081 * test(ctx): cover OmniRoute + gpt-5.6-sol (user-reported gpt sol) The Discord report used GPT-5.6 Sol, not gpt-5.4. Regression now pins the catalog-less custom-route rescue to the 272k descriptor for gpt-5.6-sol. Refs #2081 * fix(ctx): let explicit overrides beat discovered context windows Addresses both review findings on #2082. P1 — drop the generic-128k rescue from the previous commit. The custom gateway discovery mapper copies an endpoint's own context_length / context_window into ModelCatalogEntry.contextWindow, and nothing synthesizes 128k on our side, so the numeric value carries no provenance: a flat 128000 is indistinguishable from a real per-deployment or tenant cap. Budgeting a known-larger descriptor over that cap traded an early auto-compact for a late context-window API failure. Discovery is authoritative over the model descriptor again. What remains is the safe half of the fix: an env prefix override and settings.json modelLimits now sit above discovery, so a user on a gateway that advertises the wrong window can pin the real one. Docs and /set-context-window help point at that path. Tests cover a gateway-advertised 128k and 200k window staying put for a model the catalog knows is larger, plus env and modelLimits overrides beating it. P3 — isolate the discovery-cache fixtures. getClaudeConfigHomeDir() ignores CLAUDE_CONFIG_DIR by design, so both the new modelLimits test and the pre-existing withTempConfigDir helper were writing fixtures into the caller's real config dir. Both now use setClaudeConfigHomeDirForTesting with save and restore, and clear the discovery cache on teardown so the in-memory sync snapshot cannot leak into a later suite. The modelLimits test now asserts the isolated cache resolves on its own before asserting the settings pin wins. Refs #2081 * test(ctx): cover env prefix overrides over discovery cache CodeRabbit on #2082 asked for a regression where CLAUDE_CODE_OPENAI_* prefix keys beat a seeded discovery cache for both contextWindow and maxOutputTokens, not just exact keys and settings.modelLimits. Refs #2081 * docs(ctx): Clarify permanent overrides for catalogued models Keep the documented workaround aligned with resolver precedence so users do not expect modelLimits to override built-in catalog entries. Refs #2081 |
||
|
Bogdan
|
aceacf0e59 |
feat(zai): add GLM-5.3-Flash Coding Plan support (#2185)
* feat(zai): add GLM-5.3-Flash Coding Plan support * test(effort): preserve scoped xhigh context * fix(zai): scope Coding Plan route metadata * fix(integrations): prefer explicit runtime endpoints * fix(integrations): preserve override runtime limits |
||
 github-actions[bot]
|
6c7efdeaf5 |
chore(main): release 0.30.0 (#2165)
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>v0.30.0 |
||
 Recoordinate
|
b4af6f1884 |
test(settings): unit-test the multi-source merge customizer (#2176)
* test(settings): unit-test the multi-source merge customizer settingsMergeCustomizer is exported for testing but had no direct coverage, despite being the seam that combines policy, user, and project settings. Cover the documented behaviour: permission arrays concatenate with deduplication under target priority, modelPricing entries replace atomically per model id on a null-prototype map (including Object.prototype-named ids), and all other keys defer to lodash's default deep merge. * test(settings): assert the merged modelPricing map is null-prototype The previous assertion checked a fresh unrelated object, which would pass even if the customizer returned a plain object carrying the keys. Assert Object.getPrototypeOf on the merged map itself. |
||
 euxaristiaandeuxaristia
|
aaccb57721 |
fix(integrations): keep managed AIMLAPI attribution over caller headers (#2179)
* fix(integrations): keep managed AIMLAPI attribution over caller headers getRouteDiscoveryHeaders spread options.headers after resolveAimlapiAttributionHeaders had already sanitized the descriptor headers, so caller-supplied values won every collision and undid the sanitizer: a partner id from ANTHROPIC_CUSTOM_HEADERS overrode the one resolvePartnerId had just re-stamped, and attribution headers were re-added after the non-canonical filter stripped them. Fold caller headers into the sanitizer's input so both branches see them, and keep the previous descriptor-then-caller precedence for every non-aimlapi route so ordinary custom headers are unaffected. Refs #2178 * fix(integrations): Strip AIMLAPI attribution headers case-insensitively. Fetch combines duplicate header names, so a caller `x-aimlapi-source` could ride alongside the managed `X-AIMLAPI-Source`. The shared resolver now removes every managed name case-insensitively before restamping a single canonical value, and the proxy path still drops the whole set. Refs #2178 --------- Co-authored-by: euxaristia <euxaristia@users.noreply.github.com> |
||
|
0xfandom
|
5f1ab9b80c |
fix(plugins): anchor marketplace hostPattern against lookalike hosts (#2177)
strictKnownMarketplaces hostPattern entries were compiled with new RegExp(pattern) and applied with regex.test(host). RegExp.test is a substring search, so an admin pattern that is not fully anchored matched any host merely containing it. Host authority reads right-to-left, so this is not just a missing leading anchor: a policy of `github\.mycompany\.com` is satisfied by an attacker-controlled `github.mycompany.com.evil.example`, which a leading `^` alone would still admit. It is also satisfied by `evil-github.mycompany.com`. isSourceAllowedByPolicy gates whether a marketplace may be installed at all, and installation leads to plugin code execution, so a bypass defeats the enterprise lockdown before anything is fetched. Anchor the pattern as `^(?:<pattern>)$` so it must match the entire host. The non-capturing group preserves a top-level alternation (`a\.com|b\.com` must not become `^a\.com|b\.com$`), and a pattern that is already fully anchored — the form the schema documents — behaves exactly as before. This tightens matching, so a deliberately loose pattern that relied on substring behavior now needs an explicit wildcard (`.*\.mycompany\.com`). That is the intended contract, and it can only ever narrow the allowlist, never widen it. The schema description now states the whole-host requirement. pathPattern is deliberately left alone: paths nest left-to-right, so its documented prefix form (`^/opt/approved/`) is correct and anchoring the end would break it. |
||
|
0xfandom
|
8db8830666 |
fix(settings): stop proto-named permission rules from aborting validation (#2170)
getCustomValidation looks a tool name up in the plain-object TOOL_VALIDATION_CONFIG.customValidation with a bare bracket access, so the prototype chain is live. validatePermissionRule gates tool names on an uppercase first character, which rejects the lowercase prototype members (constructor, toString, ...), but the double-underscore members slip through because '_'.toUpperCase() === '_'. A rule like "__proto__(x)" therefore reaches the lookup, which returns the inherited Object.prototype value (or, for "__defineGetter__(x)", a real inherited function). The caller treats that truthy value as a configured validator and invokes it, throwing an uncaught TypeError. filterInvalidPermissionRules runs that validation over every permissions allow/deny/ask entry when a settings.json is loaded. Its whole purpose is to skip one bad rule without rejecting the file, but the throw escapes: for a project .claude/settings.json the caller's catch discards the entire file's settings, and parseCommandOutputAsSettings (MDM) has no catch at all. A single hostile or fat-fingered rule in an untrusted cloned repo's settings thus silently wipes that file's configuration. Guard the lookup with Object.hasOwn so an unrecognized tool name resolves to undefined and the rule validates as an ordinary (inert) rule instead of crashing. Mirrors normalizeLegacyToolName, which already hardened the sibling LEGACY_TOOL_NAME_ALIASES lookup against the same class. |
||
|
Bogdan
|
30090c70e1 |
fix(api): scope Anthropic attribution to compatible request paths (#2147)
* fix(api): scope Anthropic attribution to compatible request paths * test(api): expand Anthropic attribution route coverage * fix(api): align managed OAuth attribution credentials * fix(api): preserve bare attribution auth * fix(api): align SSH proxy attribution auth |
||
|
Bogdan
|
ca7c3efb6e | fix(bg): identify sessions with persisted process markers (#2163) | ||
 Cal
|
e8026263ca |
fix(tui): proper Unicode/IME input handling for composed sequences (#2018) (#2154)
* fix(tui): proper Unicode/IME input handling for composed sequences (#2018) * test(tui): address CodeRabbit review - hook-level IME coverage, full Unicode marks, astral code points, timeout regression tests |
||
|
Cal
|
54f963d006 | fix(openai-shim): drop synthetic tool-results marker and guard echoes (#2039) (#2153) | ||
|
JATMN
|
39c3850c2e |
docs: tighten PR review expectations in CONTRIBUTING and AGENTS guides (#2151)
* docs: tighten PR review expectations in CONTRIBUTING and AGENTS guides - Add drift caveat to CodeRabbit findings: verify suggestions against PR intent before applying; decline out-of-scope with justification or ask a maintainer; never silently ignore findings - Add Keep Your Branch Current subsection (rebase duty, fix-churn warning) - Require full local CI-equivalent suite green before every push, with cross-platform exception - Add ignored/filler PR template submissions to close-without-review list - Expand follow-up guidance: multi-round review is normal, repeated fix requests signal root-cause investigation and better agent prompting - Mirror all of the above in AGENTS.md for coding agents * docs: address CodeRabbit findings on review-expectations guides - Make CONTRIBUTING.md Validation the single authoritative pre-push validation contract mirroring .github/workflows/pr-checks.yml exactly: --frozen-lockfile install, launcher compatibility checks, provider recommendation via npm as CI does, web job carve-out - Remove conflicting 'relevant subset' wording; cross-platform exception is the only carve-out from the full suite - AGENTS.md now defers to the CONTRIBUTING contract instead of defining a divergent core-checks list - Reword ambiguous 'submit the PR template ignored' bullet to 'submit a PR with the PR template ignored' * docs: align pre-push validation suite with CI semantics - Drop standalone 'bun run test:full'; bun run check already includes it - Document web workspace install (bun install --cwd web --frozen-lockfile) before web checks, matching the web CI job - Pass explicit --base/--head to security:pr-scan so local scans target the PR merge-base like CI does instead of script defaults * docs: use PR base commit ref for local security scan parity Replace git merge-base computation with origin/main and document the required invariant (fetch + keep branch rebased onto current origin/main) so the local scan matches CI's PR base.sha instead of diverging. * docs: use exact PR base for security scan * docs: make local validation contract portable * docs: scope local checks and baseline waivers * docs: harden contributor workflow guidance * docs: pin contributor safety contracts |
||
|
Bogdan
|
69aca780ea |
fix(effort): preserve known model exclusions when force-enabled (#2148)
* fix(effort): preserve known model exclusions when force-enabled * fix(effort): address review feedback * test(effort): cover provider-scoped capability overrides * fix(effort): preserve route-specific reasoning controls * fix(effort): honor scoped routing environment |
||
|
Bogdan
|
34536c6220 |
fix(settings): preserve concurrent updates (#2137)
* fix(settings): preserve concurrent updates Serialize the complete settings read-merge-write transaction under a physical-target lock with a bounded synchronous contention wait. Read the merge base fresh after ownership, preserve logical symlinks during publication, and route direct settings-sync replacements through the same lock. * fix(settings): address transaction review feedback * fix(settings): reject invalid merge bases * fix(settings): preserve lock ownership and apply outcomes * fix(settings): address transaction follow-up * fix(settings): avoid unsafe cleanup control flow * fix(settings): publish complete lock claims * test(settings): document pending lock claim * fix(settings): track lock owner process identity |
||
|
euxaristiaandeuxaristia
|
bb6d66faa3 |
feat(providers): live model lists for OpenRouter and OpenGateway (#2084)
* feat(providers): fetch live model lists for OpenRouter and OpenGateway Enable hybrid discovery so OpenGateway and OpenRouter load public GET /v1/models catalogs (with coding filters on OpenRouter), matching cairn-code and the Zero live-list fix. Refs #2083 * Address live model discovery review feedback. Remove hardcoded model allowlisting, deduplicate live MiMo routes, avoid duplicate startup probes, share mapping helpers, and strengthen provider documentation and tests.\n\nRefs #2083 * test(providers): Isolate OpenGateway picker discovery state. Prevent persisted live discovery cache entries from making the static catalog assertion nondeterministic. Refs #2083 * fix(test): restore OPENGATEWAY_API_KEY after discovery test The no-auth OpenGateway discovery test deletes OPENGATEWAY_API_KEY but originalEnv never snapshotted it and afterEach never restored it, so a worker starting with the credential set would run every later test in that worker without it. Snapshot and restore it like the other provider env vars. Refs #2084 * fix(integrations): preserve route shim maxTokensField for live-only discovered models * fix(test): drop unrelated permissions.test.ts optional-chaining tweak Not part of the OpenGateway/OpenRouter live discovery change; jatmn's review on #2084 flagged it as unrelated drift that should be dropped or split into its own PR. Refs #2084 * docs(integrations): Add JSDoc comments to model mapping helpers Add detailed JSDoc documentation for gateway model normalization, tooling and reasoning support detection, and core model mapping type guards and helpers across OpenGateway, OpenRouter, and modelMapping. Refs #2083 * fix(integrations): address review feedback on live discovery and proxy credentials Preserve caller credentials and custom headers for private route overrides, remove deep-research exclusion for text models, isolate test config directories, and align model picker assertions with upstream curated models. Refs #2083 Refs #2084 * test(integrations): test explicit openaiShim precedence and removeBodyFields merge Add unit test assertions verifying that explicit descriptor and catalog openaiShim configurations take precedence over inferred model settings and that removeBodyFields arrays merge correctly across layers. Refs #2083 Refs #2084 * Filter expired catalog entries at discovery boundary and revert permissions test hunk. Wrap static and merged route catalog model lists in filterAvailableCatalogEntries across all discoverModelsForRoute and refreshStartupDiscoveryForRoute return paths, preventing expired time-boxed catalog entries and live duplicates from resurfacing in model picker refresh, summary, or bootstrap additional options. Also restore permissions.test.ts to upstream/main without optional-chaining. Refs #2084 * Fix ModelCatalogEntry type import in model test suite. Import ModelCatalogEntry from descriptors.js rather than index.js to satisfy typecheck. Refs #2084 --------- Co-authored-by: euxaristia <euxaristia@users.noreply.github.com> |
||
|
JATMN
|
1e56d4e7b2 |
feat(providers): add focused LLMTR hybrid gateway (#2150)
* feat: add LLMTR hybrid gateway * feat: support LLMTR_API_KEY * fix: allow LLMTR provider env files * fix: protect LLMTR credential routing * fix: protect LLMTR profile credentials * fix: complete LLMTR env lifecycle * fix: select LLMTR model before client setup * fix: address LLMTR review findings * fix: route LLMTR auxiliary models correctly * fix: complete LLMTR credential boundaries * fix: clear persisted LLMTR startup keys * fix: normalize LLMTR generic credentials * fix: scope LLMTR credential support * fix: align LLMTR credential boundaries * fix: close LLMTR credential boundaries * fix: clear stale LLMTR auth state * fix: clear persisted LLMTR credentials * fix: complete LLMTR setup contracts * fix: close LLMTR lifecycle gaps * fix(providers): close LLMTR credential boundaries * fix(providers): preserve saved LLMTR profile keys |
||
|
github-actions[bot]
|
421f45992c |
chore(main): release 0.29.1 (#2143)
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>v0.29.1 |
||
 Kevin CodexandClaude Fable 5
|
31ac8a6eca |
perf: stop busting the prompt cache and slim per-turn context (#2142)
* perf: stop busting the prompt cache and slim per-turn context Benchmarked against a comparable harness on grok-4.6 (identical one-shot coding task), OpenClaude used 84k total tokens per task with near-zero cache reuse. Root causes and fixes: - Auto-memory now defaults off in non-interactive (-p) sessions (src/memdir/paths.ts). The conversation-arc append gated behind it rewrote the system prompt every request (Date.now()-relative durations, running token counters, per-turn RAG retrieval), which invalidates implicit prefix caches from byte one on chat-completions providers. An explicit settings opt-in (autoMemoryEnabled / memory.autoWrite) still enables it; also drops the ~3.2k-token memory protocol section from one-shot runs. - The OpenAI shim no longer runs compressToolHistory for providers with implicit prefix caching (OpenAI, xAI, DeepSeek, Kimi/Moonshot, Codex) (requestPreparation.ts, both call sites). Its end-relative window retro-edits already-sent tool results each turn, mutating the middle of the request prefix — the native Anthropic transport already guards against exactly this (claude.ts shouldCompressNativeToolHistory). - Remove the wall-clock-relative "Ns ago" line from the multi-turn tracking block (conversationArc.ts) — it changed on every request. - Ship the ~1.7k-token git commit/PR protocol in the Bash tool description only when the session is inside a git repository (gitSettings.ts). The probe is cached per cwd, not per process, since worktree tools and daemon/SDK processes change directories mid-life. - Add a code-robustness bullet to the Doing-tasks system prompt section: derive timing-sensitive logic from elapsed time, and wire up every element introduced (prompts.ts). Measured on the same benchmark (8 runs): 84k -> ~46.6k total tokens per task (-45%), 89s -> ~60s wall clock, per-call cache reads up from a constant 128 tokens to 12k-29k, baseline context 16.8k -> 11.7k tokens. Tests: 573 targeted tests pass, including new coverage for the non-interactive memory default and the per-cwd git probe; tsc --noEmit clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: address review findings on the prompt-cache changes - Correct the prefix-caching route ids ('moonshot'/'kimi-code', not 'kimi'/'codex' — the latter never matched a real routeId) and replace the unanchored host regex with parsed-hostname comparison so path-routed gateways are not misclassified. - gitSettings: an explicit includeGitInstructions settings value now always wins over the repo probe (recourse for bare-repo/GIT_DIR layouts), and the probe reuses the existing LRU-memoized findGitRoot on getCwd() instead of a second process.cwd()-keyed implementation — fixing stale results after Bash `cd`, `git init`, and in daemon/SDK processes serving multiple directories. - Memory gate: env-provisioned memory (CLAUDE_COWORK_MEMORY_PATH_OVERRIDE, CLAUDE_CODE_REMOTE with a mounted memory dir) counts as explicit opt-in, so Cowork/remote sessions keep extraction and indexing. - Multi-turn tracking block: render only completed turns and drop the running token totals — the in-progress turn's tool-call list and the aggregate counters changed between model requests, still rewriting the system prompt mid-turn. - Update the Kimi K3 compression test to assert the new policy (history kept uncompressed on implicit-prefix-caching hosts) and extend gitSettings tests to cover settings overrides and session-cwd tracking. 569 tests pass, tsc --noEmit clean, benchmark re-run confirms metrics hold (45.1k total tokens, 62.8s, 3 calls). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: green CI and address CodeRabbit review on the prompt-cache changes CI: the conversation-arc suites assumed the multi-turn tracking block renders with only an in-progress turn, and that auto-memory is on in the (non-interactive) test process. Both now seed a completed prior turn, mark the session interactive where they exercise interactive behavior, and assert the new cache-stability invariants directly: the in-progress turn is never rendered, and no Duration/token-total lines appear. CodeRabbit findings: - Codex transport now skips tool-history compression too: Codex talks to OpenAI Responses backends with implicit prefix caching, and the end-relative compression window rewrites already-sent tool results, busting the cache (mirrors the openaiShim/requestPreparation skip). Its compression test now pins the uncompressed behavior. - New regression tests for both compression decision paths: an implicit-prefix-caching host skips compression on chat-completions and Responses requests, while a non-caching custom endpoint still compresses. - New byte-stability test: the same turn rendered twice with an advanced clock and a grown in-progress tool-call list produces an identical system prompt. - gitSettings: cover the CLAUDE_CODE_DISABLE_GIT_INSTRUCTIONS=0 defined-falsy override outside a repository. - Focused system-prompt test asserting the new timing/wiring guidance without snapshotting the full prompt. bun run check (smoke, deadcode, full suite) passes; tsc --noEmit clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test: isolate CLAUDE_CODE_SIMPLE in the doing-tasks prompt test getSystemPrompt short-circuits to a minimal prompt when CLAUDE_CODE_SIMPLE is truthy; save, unset, and restore it around the test so the full prompt path is always exercised regardless of process-level state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
github-actions[bot]
|
ef1a462faf |
chore(main): release 0.29.0 (#2116)
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>v0.29.0 |
||
|
c461a0363d |
feat: merge knowledge graph + conversation arc into memdir (#1811)
* feat: merge knowledge graph + conversation arc into memdir
Replace the standalone KG/ARC system (SQLite + JSON + Orama storage)
with direct integration into the existing auto-memory directory.
What changed:
- New memdir/vectorIndex.ts — Orama full-text index over all memory/ .md
files, replacing the separate knowledge.orama binary
- New memdir/autoExtractFacts.ts — auto-detects env vars, paths,
versions, URLs, IPs, backtick concepts from conversation and writes
them as structured .md files into memory/.facts/ with frontmatter
- conversationArc.ts now persists arc state (goals, decisions,
milestones, phase) to memory/.arc.json sidecar instead of the KG
- knowledgeGraph.ts gutted from 728→165 lines — now a thin
compatibility layer that reads .facts/ files from memdir and
delegates vector search to vectorIndex.ts
- build.ts: enabled CONVERSATION_ARC and MULTI_TURN_CONTEXT feature
flags (previously undefined → dead-code eliminated in production)
Removed:
- src/utils/storage/ (SQLiteProvider, JSONProvider, 3 test files) —
unused after KG migration
- src/utils/knowledgeGraph.test.ts, .stress.test.ts
- src/utils/conversationArc.test.ts, .perf.test.ts
- ~1700 lines of redundant storage code
Benefits:
- Single memory system (memdir) instead of two parallel systems
- Auto-extracted facts are plain .md files — visible to the model,
discoverable by the existing Sonnet prefetch
- Vector search indexes real memory content, not a separate DB
- Arc state survives across sessions via .arc.json
- ~700 lines removed from the production bundle
* fix: type errors, add test suites, fix resetArc() disk-write bug
- Fix vectorIndex.ts: parseFrontmatter returns nested {frontmatter, content},
Orama DB typed as 'any' matching existing code pattern
- Fix resetArc(): was overwriting .arc.json on disk — now clears only in-memory state
- Add vectorIndex.test.ts (6 tests): build/search/persist/rebuild
- Add autoExtractFacts.test.ts (10 tests): env vars, paths, versions,
URLs, backtick concepts, PascalCase, React/Redux, file signatures, frontmatter
- Add conversationArc.test.ts (14 tests): arc init, persistence, goals,
decisions, milestones, phase detection, arc summary, finalize, stats
- Update verify-kg-merge.sh: add test suite check (33 tests)
* fix: remove generic type param from restore() call
* fix: address CodeRabbit findings — YAML injection, secrets leak, cache invalidation, frontmatter parsing, test weakness
* fix: redact URL credentials/query/hash in endpoint extraction; add persistence + reindex regression test
* test: add regression for URL credential/query/hash redaction in fact extraction
* feat: wire getOrchestratedMemory into query.ts prompt; remove dead promises array in autoExtractFacts
* feat: enhance memory management by adding clearArcArtifacts function and integrating it into the clear command; implement file count tracking in vector index
* feat: enhance fact extraction by adding tests for absolute paths, backtick concepts, technical terms, project file signatures, and IP addresses; implement clearArcArtifacts function in tests
* Review-fix: scoped IP tagging, scrubbedContent, arcMemoryDir null, vector-index cleanup
* Review-fix: isAutoMemoryEnabled gate, scrubbed paths, clearIndex in cleanup
* Review-fix: URL-stripped path scan, digit-key/quoted-value env redaction, rm isAutoMemory gate
* Review-fix: quoted multi-token env values fully redacted, regression test
* Review-fix: freshness check on each search, integration test, typecheck in verifier
* Review-fix: missing-index-file reinit, full-pipeline integration test
* Review-fix: gate arc/RAG on isAutoMemoryEnabled, add integration+stale-index tests
Addresses three P1/P2 findings from code review:
1. P1: Honor auto-memory opt-out before writing arc facts
- Add isAutoMemoryEnabled() checks in query.ts before calling
updateArcPhase() and getOrchestratedMemory()
- Add same check inside conversationArc.ts extractFactsAutomatically()
- Prevents .facts file writes when auto-memory disabled via --bare,
CLAUDE_CODE_DISABLE_AUTO_MEMORY=1, or memory.autoWrite: false
2. P2: Add query-level integration test coverage
- New test in conversationArc.test.ts verifies query.ts path
- Confirms arc functions called behind feature gates and results
appended to system prompt (lines 555-575)
3. P2: Add stale-index regression tests
- 6 new tests in vectorIndex.test.ts cover:
* Searching after adding files
* Searching after editing files
* Searching after removing files
* Searching when .vector-index missing
* Searching when .vector-index-meta.json missing
* Mixed stale conditions
All tests pass (31 total, 99 assertions), typecheck clean.
* Review-fix: use >= for mtime staleness check to catch same-ms edits
The verification agent discovered a timing-dependent bug in the stale
index detection. When a file edit and index save occur within the same
millisecond, latestMtime equals indexMtime, causing the check
`latestMtime > indexMtime` to return false. The stale index is not
refreshed and searches miss the updated content.
Changed line 220 from `>` to `>=` in the mtime comparison. The file
count check catches add/remove operations, but edits that don't change
the file count rely on mtime comparison.
All 12 vector index tests now pass consistently, including the
"searching after editing a file picks up changes" test that previously
failed intermittently.
* Review-fix: move auto-memory gate to persistence layer
Addresses inline review comment: query.ts was incorrectly skipping
updateArcPhase() entirely when isAutoMemoryEnabled() returned false,
leaving the in-memory arc state stale.
Fixed by:
- Removed isAutoMemoryEnabled() gate from query.ts line 449
- Added gate inside conversationArc.ts updateArcPhase() at persistence
layer (line 223), so phase advances but only persistence is disabled
- Arc state tracking now works regardless of auto-memory setting
- Only disk writes (.arc.json, .facts files, index rebuilds) are gated
The 2ms delay in vectorIndex.test.ts is kept as a pragmatic fix for the
timing race. Content-hash detection would be ideal but adds complexity;
the mtime check works reliably in production.
All 31 tests pass, typecheck clean.
* Refactor memory handling: consolidate metadata retrieval and improve auto-memory checks
* Fix: update expectation to use toEqual for prompt comparison in conversationArc tests
* fix: detect same-size content changes via content hash; handle text-block user messages in arc query
* fix: skip symlinked dirs in vector index walk; enable arc/multi-turn flags; add production-path tests
* fix: follow symlinked dirs in vector index walk per review
* fix: skip symlinked dirs in vector index walk; add symlink-boundary regression tests
* fix: skip indexing symlinked directories and ensure only files are processed
* fix: show cmd output on failure in verifier; add multi-turn coverage; clear mempath cache; cover arc reset in knowledge clear test
* fix: clear memoized auto-mem path after teardown in knowledge + conversationArc tests
* fix: isolate vector index per memdir, filter secrets from backtick facts, clean trailing ws
* fix: extend credential filter for AWS/GitLab tokens; reject all symlinks in vector index
* fix: use repo's redactSecretSubstringsForDisplay; add npm/glpat/AKIA/ASIA/xox to shared patterns; cover NPM+JWT in tests
* fix: P1 backtick credential safety + untrusted-data boundary; P2 legacy migration + non-fatal writes; P3 type safety + build regression
* fix: B1-B5, M6, M8 — no message mutation, migration data loss, empty-file, non-fatal writes, probe, rebuildIndex resilience, dead import
* fix: address 8 reviewer findings (R1-R8)
P1:
- Keep retrieved facts in DATA ONLY block with strict system instruction
- Catch lowercase config secrets (api_key=...) in env scrubber
- Migrate SQLite working store (knowledge.db) before deleting provider
- /knowledge clear atomically archives legacy sources
P2:
- Run legacy migration on getOrchestratedMemory retrieval path
- Preserve entity attributes in migration frontmatter
- Only rebuild vector index when facts actually changed
- Fix feature-flag verifier --define syntax (declare const)
* fix: close 9 memory findings — approval gate, non-fatal writes, secret scrub, per-project migration, attribute/relation preservation, WAL cleanup, cheaper index
- Gate auto fact extraction on isMemoryWriteApprovalRequired() + isAutoMemoryEnabled() so default projects cannot silently persist conversation content
- Make ensureFactsDir/writeFactMemory degrade non-fatally (no turn-breaking throw on read-only dirs)
- Scrub token-like URL/path/hyphenated segments from durable facts via looksLikeSecret (reuses providerSecrets.looksLikeSecretValue)
- Restore passive project-rule extraction as rule facts
- Honor isAutoMemoryEnabled() before legacy migration; scope the migration guard per project (Set) instead of a single global
- Preserve legacy entity attributes and relations through migration (indented attributes + relation fact file, reconstructed in getGlobalGraph)
- Clear SQLite WAL/SHM sidecars on /knowledge clear
- Replace per-turn content hashing in vectorIndex getMdStats with size+mtime metadata; drop redundant initMemdirIndex call in getOrchestratedMemory
- Add knowledgeGraph tests covering P1#2/P1#4/P2#5/P2#8 and extend autoExtractFacts tests
* test: avoid global cwd pollution in knowledgeGraph tests
Replace process.chdir with a per-test setFsImplementation mock cwd that is
reset via setOriginalFsImplementation in afterEach, so the test no longer
leaks a changed process.cwd() into other test files. Assert the auto-memory
gate via the project-specific legacy file rather than the shared resolved
memdir dir (which bun runs concurrently across it blocks).
* fix: address 15 reviewer findings (R1-R15)
P1:
- Gate saveArcToDisk/finalizeArcTurn/saveIndex/migration on memory-write approval
- Retire legacy sources after successful migration (rmSync, backup preserved)
- Slugify entity.type and summary.id in migration filenames (path traversal)
- Fall back to JSON when SQLite has zero entities
- Scrub rule-fact extraction on scrubbedContent + looksLikeSecret/redact check
P2:
- Track skipped vs completed migration; re-enable clears skip marker
- Walk back to latest human text for tool-round vector queries
- Auto-extract goals/decisions from user messages in updateArcPhase
- /knowledge clear message says durable wipe, not session-only
- Bind arc state to projectKey (re-resolve on cwd change)
- clearIndex(memoryDir?) scopes to one memdir
- yamlQuote all migration frontmatter fields
- Real SHA-256 contentHash alongside fileFingerprint for same-size edits
- Stop claiming production-pipeline coverage in tests/verifier
* test: isolate governance mock by removing afterEach clear
Remove setGovernancePolicySettingsForSourceForTesting(null) from afterEach
in autoExtractFacts, conversationArc, and knowledgeGraph test files. The
module-level mock is set in each file's beforeEach and since there is no
afterEach cleardown, parallel test execution can no longer corrupt the
mock state across files.
This fixes 26 CI test failures caused by one file's afterEach clearing
the mock that another concurrently-running file had set in its beforeEach.
* fix: isolate governance mock per async context via executionAsyncId tracking
Replace the module-level variable in governancePolicy.ts with an
executionAsyncId-keyed Map and an async_hooks.createHook that propagates
the override from parent to child async resources. This ensures each
concurrent test's beforeEach/afterEach cannot corrupt the override set
by another test file, even when test bodies directly mutate the mock.
Also restore setGovernancePolicySettingsForSourceForTesting(null) calls
in afterEach hooks (removed in 39c68376), which are now safe because
each afterEach only clears its own async context.
Fixes 26 CI test failures across knowledgeGraph (2), conversationArc (7),
and autoExtractFacts (17/22, governance-gate tests).
* Refactor knowledge graph legacy migration, secure secrets and IP octets, optimize build-time feature flags, cap conversation arc collections, and resolve all verification check gaps
* Fix governancePolicy enablement in full test runs by checking for test runner globals
* fix: ensure auto memory is enabled in conversationArc, knowledgeGraph, autoExtractFacts tests
Delete CLAUDE_CODE_DISABLE_AUTO_MEMORY and CLAUDE_CODE_SIMPLE env vars
in beforeEach hooks so tests are not blocked when CI sets these vars.
Also revert governancePolicy.ts to the simple module-level variable
(removing the async ID tracking approach that broke with Bun v1.3.13).
Fixes 33 CI test failures where isAutoMemoryEnabled() returned false
causing all disk-persistence guards to fire.
* fix: address P1/P2 review findings — redaction, bounds, legacy backup
[P1] Redact goal/decision descriptions with redactLikelySecrets before
persisting to .arc.json, session summaries, and prompt summaries so
credentials captured by auto-extraction regexes are not durably stored.
[P1] Bound multi-turn tool input serialization to 2000 chars and apply
redactLikelySecrets, preventing oversized/credential-bearing tool inputs
from overflowing the next provider request or exposing secrets.
[P2] Archive the non-selected legacy store (JSON or SQLite) and its WAL
sidecars before retiring both sources, ensuring a recoverable snapshot
exists if generated fact files are incomplete or a migration bug surfaces.
* fix: address P1 findings — safe legacy retirement, multi-turn aggregate budget
[P1] Do not retire a legacy store unless every existing source was
successfully archived. Track archived sources in a Set and skip deletion
of any source whose backup failed (knowledgeGraph.ts).
[P1] Archive the selected SQLite WAL/SHM sidecars alongside its migration
backup, since committed state may reside only in the WAL file and the
advertised recovery backup would otherwise be incomplete (knowledgeGraph.ts).
[P1] Bound the aggregate multi-turn tool replay to 10KB total and stop
appending further turns once the budget is exceeded, preventing many
Agent/MCP calls per turn from adding unbounded text to system prompts
(conversationArc.ts).
* fix: address P1/P2 review findings — rule extraction gate, SQLite read status, atomic WAL/SHAM, KG status gate, byte budgets
* fix: tighten looksLikeOpaqueToken to avoid flagging compound model names
* fix: address P1/P2 review findings — rebase, test isolation, SQLite retry, attribute redaction, entity aliases, body content, project-scoped multiturn, skip unchanged writes, knowledge list gate
* fix: address P1/P2/P3 review findings — secret scrub on migrate, lean decision gate, git-root legacy lookup, backup retention, index rebuild chaining, single vector search, drop unreferenced fixture
* fix: scrub secret entity names on migrate, exclude summary facts from entities, reset multi-turn on /knowledge clear
* fix: address P1 review findings for legacy graph redaction, recovery-safe clear, stable change guards
- Redact embedded secrets in migrated legacy knowledge-graph entities,
summaries, and rules via shared sanitizeLegacyText() policy
- Preserve legacy artifacts (json/db/wal/shm) as migration-backup before
/knowledge clear; resetGlobalGraph returns { archived, failures }
- Skip rewrite + index rebuild on unchanged turns by stripping the volatile
detectedAt timestamp (facts and arc session summaries)
- Always recompute the authoritative content hash in getMdStats so edits of
equal size with preserved mtime are detected and served correctly
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* fix: redact protocol-relative URL userinfo in legacy migration
new URL() throws for scheme-less URLs, so the catch branch now redacts
obvious //user:pass@host userinfo instead of persisting credentials.
* fix(memory): harden memdir migration and retrieval
---------
Co-authored-by: Gravirei <gravirei@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Kevin Codex <kevin@gitlawb.com>
|
||
 Kevin CodexandOpenClaude
|
6e3590303b |
feat(partners): add Concentrate and Exa to partner roster (#2141)
Adds Concentrate (concentrate.ai) and Exa (exa.ai) to the README partners table and the web landing page, with light/dark logo variants self-hosted under docs/assets/ and web/public/partners/. Co-authored-by: OpenClaude <openclaude@gitlawb.com> |
||
 
|
645d596ea4 |
fix(code-reviewer): require inline diff input and preserve read-only search in embedded-search builds (#2102)
* feat: add code reviewer agent * feat(agent): add code-reviewer built-in agent implementation and tests Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(code-reviewer): address review comments - Fix broken step numbering in system prompt (glob/grep as sub-bullets) - Remove unnecessary wrapper in getSystemPrompt - Drop unused beforeEach/afterEach lifecycle in tests - Remove redundant registration test (beforeAll already throws) - Fix CLAUDE_CONFIG_DIR leak in beforeAll (restore in finally) - Replace @ts-ignore with explicit ToolUseContext cast - Use placeholder in README agentRouting example Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci: trigger rerun — pre-existing test failures on main * fix(code-reviewer): enforce read-only contract by disallowing Bash Add Bash to disallowedTools so the reviewer cannot run shell commands regardless of parent session's acceptEdits/bypassPermissions mode. resolveAgentTools() treated undefined tools as wildcard — Bash was available and could auto-approve mkdir/rm/mv in acceptEdits mode. Remove Bash guidance from system prompt; diff must now be supplied by the caller inline. Update test to assert Bash is in disallowedTools. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(code-reviewer): deny all shell tools (Bash and PowerShell) Use SHELL_TOOL_NAMES constant instead of just BASH_TOOL_NAME to ensure all shell-capable tools are denied from the code-reviewer agent. This prevents Windows sessions with PowerShell enabled from bypassing the read-only contract. - Import SHELL_TOOL_NAMES from shellToolUtils - Use spread operator to include both Bash and PowerShell in disallowedTools - Update test to verify PowerShell is denied Fixes the finding: [P2] Deny all shell tools for the reviewer agent * fix(code-reviewer): explicit read-only allow-list; drop unrelated artifacts Switch code-reviewer to an explicit `tools` allow-list (Read, Glob, Grep) instead of relying on wildcard access minus a deny-list. resolveAgentTools() resolves only the named tools, so write-capable mcp__* server tools (and any other mutation-capable tool) can never be handed to the read-only reviewer. Keep the mutation deny-list as defense-in-depth. Remove generated/scratch artifacts unrelated to the reviewer agent: AGENTS.md, ARCHITECTURE.md, the .openlore/ .gitignore rule, temp_reference/, and the .tmp/sdk-consumer-* scratch files. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: restore temp_reference/ gitignore entry from main Entry was present on main from #1350 and was unintentionally removed during PR cleanup. Restoring it so temp_reference/ scratch directories remain untracked after merge. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(code-reviewer): require inline diff input and preserve read-only search in embedded-search builds Summary: - Update the `code-reviewer` built-in agent to require the caller to provide the diff or changed hunks inline in the prompt. - Preserve read-only search behavior in embedded-search builds by omitting `Glob`/`Grep` from the explicit tool allow-list when embedded search tools are enabled. - Keep a strict read-only policy by disallowing shell and mutation tools. Usage: - The `code-reviewer` agent is now explicitly guided to only review changes when the diff is provided inline. - In embedded-search builds, the agent only receives `Read` and cannot access `Glob`/`Grep`. - This prevents the agent from attempting shell-based diff discovery and enforces caller-provided diff input. Test plan: - `bun test src/tools/AgentTool/built-in/codeReviewerAgent.test.ts` — 12 pass, 0 fail - `bun run build` — success - `bun run smoke` — success - `bun run security:pr-scan` — success - `git diff --check` — success Credit: prior work from #1381/#1420. * fix(code-reviewer): clear cached agent definitions and markdown loader cache after test cleanup * fix(code-reviewer): address all P2/P3 review findings from jatmn - Always list [Read, Glob, Grep] in the tool allow-list; in embedded-search builds resolveAgentTools() silently drops unavailable Glob/Grep at runtime. The system prompt explicitly documents the narrowed search contract for embedded builds instead of silently degrading. - Update the code-reviewer invocation example in prompt.ts to include the diff inline, satisfying the reviewer's contract and preventing an avoidable extra turn. - Rewrite the test suite with the same isolation protocol as loadAgentsDir.test.ts: shared mutation lock, OPENCLAUDE_CONFIG_DIR, setClaudeConfigHomeDirForTesting, setAllowedSettingSources, and full env/cache restore in finally blocks. - Exercise both embedded-search branches: non-embedded tests verify Glob/Grep guidance, embedded tests verify the limited-search documentation and Read-only path. - Fix settings file path in README.md and docs/agent-routing.md from ~/.openclaude.json to ~/.openclaude/settings.json (the path the runtime actually loads). - Add blank line after fenced code block (MD031), add code-reviewer to the routable built-in agent list in both README and agent-routing docs. * fix(code-reviewer): restore prior setting sources in test cleanup, add credential security warning - Capture getAllowedSettingSources() before overwriting and restore it in the finally block instead of resetting to the default list, preventing state leakage to concurrent suites. - Add plaintext-credential security warning before the agentModels JSON example in README. - Update 'All settings-driven' to 'Configured via settings, agent frontmatter, and environment variables' for accuracy. * fix(code-reviewer): address remaining PR feedback (P1/P3) * docs: document feature gate for Explore and Plan agents * docs: document inline-diff requirement for code-reviewer agent * fix(code-reviewer): address P1/P2 review findings — teammate boundary, resume safety, lock-aware tests [P1] Reject built-in agent types from teammate spawn path to preserve read-only boundary. The teammate branch bypasses resolveAgentTools(), so built-ins like code-reviewer would receive Bash/Edit/Write tools. Guard added in AgentTool.tsx before spawnTeammate is called. [P1] Fail closed when resuming an unavailable agent type instead of silently falling back to GENERAL_PURPOSE_AGENT. A resumed code-reviewer must never gain edit-capable tools through a compatibility fallback. [P2] Snapshot environment and config state only after acquiring the shared mutation lock in codeReviewerAgent.test.ts. Moved from module- scope const to post-lock capture in beforeAll, with cleanup and lock release in afterAll's finally block. Regression tests added for all three findings. * fix(code-reviewer): guard lock release against failed acquisition Only call releaseSharedMutationLock() in afterAll when the lock was successfully acquired. Prevents releasing another suite's lock if acquireSharedMutationLock() throws on timeout. * fix(code-reviewer): remove trailing whitespace * fix(code-reviewer): reliably block built-in teammate spawns Reject built-in agent types from the teammate spawn path by looking them up in allAgents rather than activeAgents. This ensures the restriction remains intact even when built-in agents are disabled (e.g. via CLAUDE_AGENT_SDK_DISABLE_BUILTIN_AGENTS) and omitted from activeAgents. Regression test added. * fix(code-reviewer): preserve original agent identity when resuming a read-only reviewer Background-agent metadata persisted only agentType, so resuming selected whichever active definition currently had that name. A built-in code-reviewer could be started, followed by a project/SDK definition named code-reviewer taking precedence; resuming the original agent would then use the replacement's ordinary wildcard tool set and grant that reviewer transcript Bash/Edit/Write tools. Persist the agent definition source and verify it matches the resolved definition on resume, rejecting the resumption if the original was spoofed. * fix(code-reviewer): address remaining CodeRabbit feedback on test cleanup and metadata source * fix(code-reviewer): address P1/P2 issues for teammate spawns and resume safety * docs: make OpenLore prerequisite explicitly optional in AGENTS.md * docs: fix pinned OpenLore version in AGENTS.md * Fix review issues * Revert AGENTS.md changes * Restore AGENTS.md to match upstream/main * fix(agent): address maintainer feedback on teammate spawns and resume persistence * test(agent): add regression coverage for legacy source-less agent resume * fix(agent): propagation pass — batch fork regression, TeamCreate policy, trailing whitespace, verification gate docs * fix(batch): allow specific custom agent types while requiring subagent_type --------- Co-authored-by: Laurent FRANCOISE <lfrancoise@gmail.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
 மனோஜ்குமார் பழனிச்சாமிandClaude Opus 5
|
787f2a9390 |
refactor(cli): commander-authoritative argv handling for SSH / cc:// and remote bypass (recovery of #1939) (#2098)
* fix(cli): mirror programmatic args onto process.argv (hardened) Address CodeRabbit's Critical finding: cliMain() parses process.argv directly, so a programmatic main(args) call must reflect args there or it silently runs the host's argv. Restores the args->argv sync but hardened against the three bugs the multi-agent review confirmed in the earlier scoped/serialized version: - no restore (was F3: a finally-restore flipped argv out from under the SIGINT handler and bypass-safety notice while the session was live) - length-guarded exec/script slots (was F4: <2 host argv entries, e.g. node -e, dropped the flag past commander's argv.slice(2)) - no serialization chain (was F2: overlapping calls hung indefinitely) For the normal binary launch (args defaulted from process.argv.slice(2)) the assignment is a value-identical no-op. Tests: programmatic args reach cliMain, argv is not restored, and the exec/script slots are padded under a short host argv. Verified: 48 entrypoint+safety tests, tsc, build, and e2e re-probes of all former --yolo bug scenarios on the built binary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(cli): ssh strip-all dangerous-skip tokens; docs+tests for native alias Address the latest Copilot + CodeRabbit findings on the native-alias PR: - ssh argv pre-scan removed only the first dangerous-skip token, so `ssh --yolo --dangerously-skip-permissions host` (or a repeat) left a survivor that re-enabled bypass after the ssh rewrite. Strip every matching token. (connect already used the filter helper.) [Copilot] - web/src/data/cliFlags.ts: list the flag as '--yolo, --dangerously-skip-permissions' to match the commander registration, not just the description. [Copilot] - Replace the source-string-count registration test with (a) a behavioral test that the built CLI lists the alias in --help — proving the main-command registration is live, not dead code or the wrong command — and (b) a structured .option() assertion plus a check that the ssh pre-scan handles the alias (ssh --help renders root help, so the ssh --yolo path is the pre-scan, not the commander option). [CodeRabbit] - PR description rewritten to describe the native alias instead of the removed argv rewrite. [Copilot] Verified: 23 cli + 10 safety tests, tsc, build, e2e (--yolo --help lists the alias; ssh --help; mcp add --yolo names the typed flag). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(cli): runtime coverage for dangerous-skip strip; share one helper CodeRabbit (approved, follow-up): the strip-all fix was only source- checked, so the strip loop could regress unnoticed. Extract the argv scanners into src/utils/dangerousSkipFlags.ts (isDangerousSkipFlag / hasDangerousSkipFlag / stripDangerousSkipFlags) and unit-test them at runtime: both spellings detected, every token stripped (canonical + --yolo + repeats), input not mutated. Both the direct-connect and ssh rewrites in main.tsx now use the shared stripDangerousSkipFlags — the ssh path's bespoke single-splice while loop (the original survivor bug) is gone, replaced by an in-place splice(0, len, ...strip). One tested code path instead of two. Verified: helper + 23 cli + 10 safety tests, tsc, build, e2e. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * style(cli): semicolon + strip both spellings in safety-test reset Copilot review nits: - statusNoticeDefinitions.tsx: add the trailing semicolon to the parenthesized return to match the file's semicolon style. - safety.test.tsx: the beforeEach argv reset filtered only --dangerously-skip-permissions; strip --yolo too so the 'without the flag' cases can't go order-dependent if the runner is invoked with --yolo in argv. - PR description re-synced to the native-alias approach (the earlier edit had reverted to the old 'normalize via argv rewrite' wording). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(cli): notice names --yolo; move import out of boot-critical block Both bots on 50772d28: - The bypass-safety notice fires for --yolo but its text named only '--dangerously-skip-permissions is active'. Reword to '... (alias --yolo) is active' so the message matches what the user typed. Add a rendered-notice regression assertion for the --yolo case. [CodeRabbit + Copilot] - Move the dangerousSkipFlags import out of the boot-critical header block (it ran before profileCheckpoint('main_tsx_entry'), adding pre-checkpoint work in the order-preserving bundle) down to the regular internal-import group. [Copilot] Verified: 39 cli+safety+helper tests, tsc, build, e2e. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(cli): honor -- end-of-options in the raw-argv dangerous-skip scanners Copilot (3x on 5cd0e304): the pre-commander scanners matched --yolo / --dangerously-skip-permissions anywhere in argv, so a positional after the -- marker (openclaude -p -- --yolo, cc://… -- --yolo, ssh host -- --yolo) was misread as the bypass flag — enabling bypass / stripping the token / false-firing the safety notice, even though commander treats it as positional. Pre-existing for the canonical flag, but the short alias makes it far likelier. Make the shared helpers --aware in one place: hasDangerousSkipFlag and stripDangerousSkipFlags only consider option-position tokens (before the first --) and preserve everything from -- onward. Route the safety notice's hasDangerouslySkipPermissionsArg through the same helper. Regression tests: helper ignores/preserves post-- tokens; the notice does not fire for -p -- --yolo. Verified: 41 helper+safety+cli tests, tsc, build, e2e. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(cli): don't leak args via process.argv; stop simulating commander in the scanner Address jatmn's two P1 findings, both by removing a band-aid rather than adding another: - P1a: main() no longer mirrors its args onto the process-global process.argv. Verified there is no production or SDK caller that passes custom args (the sole entry is the no-arg auto-run), so the mirror only risked leaking a programmatic invocation's args (including a bypass flag) into an overlapping call or the host process. Back to the baseline signature; cliMain parses the real process.argv. - P1b: revert the end-of-options ("--") handling in the dangerous-skip scanner. As jatmn notes, correctly classifying "--yolo" (it can be a required option value like "--system-prompt --yolo", or follow a "--" consumed as a variadic value) requires commander's option-arity state machine, the exact simulation this feature was reworked to delete. The scanner now mirrors the canonical --dangerously-skip-permissions presence check exactly: both spellings behave identically, and the approximation is documented as a pre-existing limitation of pre-commander scanning. Net: every difference between --yolo and the canonical flag is now either native-commander-correct (the registration) or an identical approximation (the raw scanners). No new argv mutation, no parser simulation. Verified: 38 cli+helper+safety tests, tsc, build, e2e. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(cli): clarify main() argv comment — skills/--update do rewrite argv Copilot: the "does not mirror args onto process.argv" comment was misleading — the skills and --update fast-paths reassign process.argv to re-route to their subcommand. Note the exception; the no-mirror rule is about not injecting the caller's args into the general cliMain flow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(setup): name the --yolo alias in the root/sudo bypass error Copilot: the root/sudo safety error printed only --dangerously-skip-permissions; a user who typed --yolo saw a flag they didn't use. Mention both spellings, matching the ssh help and cliFlags. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(cli): import Command from @commander-js/extra-typings, matching prod Copilot: the alias behavioral test built its probe Command from 'commander', but production registers options via @commander-js/extra-typings. Use the same package so the test exercises the exact parser prod uses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ssh): don't treat an option value as the permission-bypass alias jatmn P1: `ssh host --permission-mode --yolo` stripped --yolo as a bypass flag (enabling bypass) and left --permission-mode valueless, whereas commander parses --yolo as the (invalid) mode value and rejects it — a silent privilege escalation. Same class affected --model / --resume / --fallback-model. Reorder the ssh pre-parser so value-taking flags consume their value — including a dangerous-skip token in the value slot — BEFORE the dangerous-skip strip runs. Extract the whole flag pre-parse into a pure, unit-tested helper (parseSshFlags) so the security-sensitive arity handling has regression coverage: escalation guards for --permission-mode/--model + a value of --yolo, plus genuine standalone --yolo still enabling bypass. Verified: 6 ssh + dangerousSkipFlags + cli tests, tsc, build, e2e. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(cli): update ssh-path source assertion for parseSshFlags extraction Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(cli): revert unrelated --bare doc; drop stray blank line CodeRabbit/Copilot on the rebased branch: - Revert the --bare help rewrite (background prefetches / CLAUDE_CODE_SIMPLE / expanded flag list) in main.tsx and web/src/data/cliFlags.ts — it is unrelated to the --yolo alias and broadens scope. cliFlags.ts now changes only the --yolo alias line. - Remove the stray double blank line before cliMain. Skipped CodeRabbit's "alias order breaks property naming" (Major): verified against @commander-js/extra-typings that '--yolo, --dangerously-skip- permissions' maps BOTH spellings to opts().dangerouslySkipPermissions (commander keys off the last long flag); opts().yolo is undefined. Bypass is not broken. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ssh): honor the -- end-of-options marker in the ssh pre-parser CodeRabbit Critical + jatmn P1 ("respect option arity/end-of-options semantics"): `ssh host -- --yolo` (or `-- --local`, `-- --permission-mode x`) parsed the post-`--` tokens as flags, so a positional --yolo escalated to bypass. parseSshFlags now parses only the prefix before `--` and keeps everything from `--` on positional. This is unambiguous here because the ssh subcommand registers no variadic options that could consume `--` as a value. The connect (cc://) path is deliberately left plain and documented: it rewrites to the main command, which HAS variadic options (--add-dir …) that commander lets consume `--` as a value, so a naive `--` split there would be the incomplete simulation flagged in P1b. That false-positive is pre-existing for the canonical flag. Skipped (both pre-existing, ported verbatim / out of scope): extractFlag mixed `--flag=x --flag y` precedence, and the setup.ts root/sudo message not naming --allow-dangerously-skip-permissions. Verified: 8 ssh + cli tests, tsc, build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor: make --yolo/bypass Commander-authoritative on cc:// connect + notice * fixup! address CodeRabbit/Copilot review findings on #2098 - Restore parseMaxTurnsCommanderArgument for --max-turns validation. - Generalize root/sudo error message for all bypass modes/flags. - Clarify cli.tsx comment about main() not leaking args into process.argv. - Detect -p/--print in all commander-accepted forms (including --print= and -p<value>). - Correct dangerousSkipFlags.ts doc: SSH strips, cc:// preserves for Commander. - Extend live help test to ssh and open subcommands. * fixup! extract hasPrintFlag helper + regression tests - Move print-flag detection to src/utils/printFlag.ts so it is unit-testable. - Add regression tests for -p, --print, --print=prompt, -pprompt, and -- separator. - Import the helper in main.tsx and drop the local copy. * fixup! use hasPrintFlag in every startup print-mode check - Replace exact-string includes in the SSH headless rejection and the main print-mode gate with the shared hasPrintFlag predicate. - Add source assertions confirming cc:// rewrite, SSH rejection, and main print-mode gate all use the same helper. * fixup! align dangerously-skip notice comment with commander-authoritative mode - Remove stale 'reads from process.argv' text; the notice now keys off the resolved permissionMode. * fixup! add SIGINT handler to hasPrintFlag source assertions - Include the SIGINT print-mode gate in the source-level consistency check. * fixup! restore maxTurns forwarding dropped during rebase - Re-add options.maxTurns to sessionConfig and the interactive REPL props for direct-connect, SSH, remote viewer, and remote creation paths (matches main). * fixup! address Copilot suppressed comments on #2098 - parseSshFlags now consumes required-arg values unconditionally, matching commander and preventing flag-like values from leaking into later guards. - Drop the now-unused isDangerousSkipFlag import from sshPreParse.ts. - Make the dangerously-skip notice text mode-agnostic so settings-driven bypassPermissions is not mislabeled as a CLI flag. * fixup: left-to-right SSH parse and fullAccess sandbox warning - Rewrite parseSshFlags as a single left-to-right arity-aware scan. Value-taking flags now consume every occurrence (including equals forms) and always consume the next token as their value, even if it resembles a flag (e.g. --permission-mode --local or --model --yolo value). - Cover fullAccess with the dangerously-skip-permissions sandbox warning and add focused regression tests for bypassPermissions/fullAccess rendering. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fixup: preserve missing SSH option values and embedded equals - Keep value-taking SSH flags in remaining when they have no available value (last token before -- or trailing), letting commander report the missing required argument. - Use slice after the prefix for equals-form values so embedded '=' characters are preserved (e.g. --model=provider=model). - Add regression tests for last-token, before--, and embedded-equals cases. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fixup: arity-aware print detection and optional --resume for SSH - hasPrintFlag now skips tokens consumed as values by preceding value-taking options (required, optional, and variadic), so --system-prompt --print=custom is no longer misclassified as print mode. - parseSshFlags treats --resume as an optional-value option: a bare --resume is forwarded, a non-option value is consumed, and following flags (e.g. --yolo) remain available for their own parsing. - Added focused regression tests for both fixes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fixup: handle inline values and -- in hasPrintFlag - Do not advance past a following flag when a required/optional/variadic option already provided its value inline via =. - Stop the scan when a value-taking option is immediately followed by --. - Added regression tests for --model=foo --print and --model -- --print. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fixup! address jatmn review: arity-aware print/SSH parsing + fullAccess notice Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fixup! correct bypassPermissions notice wording Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Address remaining CodeRabbit findings on #2098 - Sync hasPrintFlag with all root-command required/variadic/optional options, including hidden/feature-gated flags (--agent-id, --sdk-url, --channels, etc.). - Treat the SDK 'full-access' spelling as fullAccess in the dangerous-skip-permissions status notice so the stronger warning is shown. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(cli): make hasPrintFlag the single pre-Commander print classifier - Replace all startup .includes('-p')/.includes('--print') checks with the arity-aware hasPrintFlag() predicate so value-consumed tokens are not misclassified as print mode. - Centralize SSH headless detection in sshArgvImpliesHeadless(), covering both tail argv after host/cwd and flags forwarded via extraCliArgs (e.g. --resume=--print). - Soften the fullAccess status-notice wording to match runtime behavior: most consent checks are bypassed, but hard deny rules and user-interaction prompts still apply. - Add/extend tests for interactivity, SSH flag pre-parsing, status notices, and a regression check that prevents naive print-token checks from re-entering. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(ssh): preserve --resume= values and run headless guard before host extraction - Keep the inline value attached when forwarding --resume=... extraCliArgs so optional-value semantics are preserved (e.g. --resume=--print resumes a conversation named "--print", it does not enable print mode). - Move the SSH headless guard before host/cwd extraction in main.tsx so print flags that appear before the host are rejected. - Update tests: --resume=--print is no longer headless, required-value options like --model -p remain non-headless, and add coverage for print flags before the host. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(cli): remove repeatable single-value options from VARIADIC_OPTIONS --plugin-dir and --provider-env-file are registered as repeatable single-value options, not variadic. Keeping them in VARIADIC_OPTIONS made the arity model wrong and could consume extra tokens if check order changed. They are already covered by REQUIRED_VALUE_OPTIONS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
 anushka ✩
|
294bd9a1df |
Add optional Sentry error reporting (env-driven, opt-in) (#2139)
* Add optional Sentry error reporting (env-driven, opt-in) * Fix Sentry init to use dynamic import instead of require (ESM compatibility) * Document SENTRY_DSN setup in advanced-setup docs * Disable Sentry default integrations; document runtime install requirement * Wire reportErrorToSentry into top-level error handlers; add sentry.test.ts |
||
|
JATMN
|
084bc53463 |
feat(gateway): add Concentrate AI provider with dynamic model discovery (#2140)
* feat(gateway): add Concentrate AI provider with dynamic model discovery * fix(concentrate): prompt for Concentrate API key in /provider instead of pre-filling OPENAI_API_KEY * docs(concentrate): remove standalone setup guide to match other gateways * fix(concentrate): dedicated-credential-only routing, env-only identity, and credential isolation - Make Concentrate dedicatedCredentialsOnly so ambient OPENAI_API_KEY is never forwarded. Only CONCENTRATE_API_KEY authenticates the route. - Resolve Concentrate env-only route identity from CONCENTRATE_API_KEY, CONCENTRATE_BASE_URL, CONCENTRATE_MODEL, or a Concentrate-shaped OPENAI_BASE_URL. - Mirror the dedicated credential into OPENAI_API_KEY only after the route identity is established and only for the canonical /v1 inference endpoint. - Add Concentrate support to --provider concentrate, saved profiles, startup env rebuild, and .env allowlist. - Add regression tests for env-only, flag, saved-profile, and client routing. - Fix adjacent ApiSmart keyless profile leaking string 'undefined' into OPENAI_API_KEY and extend the first providerProfiles test timeout for the now-slower fresh module import. * fix(concentrate): protect credentials on noncanonical urls * fix(concentrate): drop legacy keys on retargeted profiles * fix(concentrate): remove legacy generic keys on proxies * fix(concentrate): reject noncanonical credential routes * fix(concentrate): validate env-only credentials * fix(concentrate): align env-only route validation * fix(concentrate): clear headers before client setup * test(validation): isolate Concentrate model env * fix(concentrate): honor provider precedence and model defaults * test(concentrate): cover model resolution precedence * fix(concentrate): preserve legacy model fallback * fix(concentrate): normalize early model selection * fix(concentrate): validate and select rejected models safely * fix(concentrate): reset stale route state * fix(concentrate): preserve proxy profile capabilities |
||
|
Bogdan
|
108a413493 |
fix(bg): preserve detached session terminal outcomes (#2133)
* fix(bg): preserve detached session terminal outcomes * fix(bg): harden terminal outcome routing |
||
|
Bogdan
|
e9e6beb15b |
fix(mcp): paginate discovery list operations (#2132)
* fix(mcp): paginate discovery list operations * fix(mcp): bound paginated discovery retries |
||
|
Bogdan
|
09eba26d30 |
feat(cost): support exact custom model pricing (#2131)
* feat(cost): support exact custom model pricing * fix(cost): address custom pricing review feedback |
||
|
Bogdan
|
c30578819e |
diagnostics(query): trace interruption causality (#2111)
* diagnostics(issue-1830): trace interruption causality * test(issue-1830): lock interruption ownership matrix * fix(codex): preserve stream deadline contract * fix(diagnostics): harden interruption trace lifecycle Refs #1830 * fix(diagnostics): harden interruption trace settlement Refs #1830 * fix(diagnostics): preserve interruption causality * fix(diagnostics): address interruption trace review * fix(diagnostics): preserve tracing observer contracts * fix(diagnostics): preserve interruption trace contracts * test(permissions): cover interactive hook interrupts |
||
|
Bogdan
|
ea655163d3 |
feat(zai): expand Coding Plan catalog support (#2127)
* feat(zai): expand Coding Plan catalog support Signed-off-by: chioarub <chioarub@gmail.com> * fix(zai): use supported low reasoning mode Signed-off-by: chioarub <chioarub@gmail.com> --------- Signed-off-by: chioarub <chioarub@gmail.com> |
||
|
0xfandom
|
6c7a12b2a2 |
fix(websearch): reject non-positive WEB_CUSTOM env overrides (#2124)
The custom web-search provider read WEB_CUSTOM_TIMEOUT_SEC and WEB_CUSTOM_MAX_BODY_KB with Number(x) || DEFAULT. That idiom only rescues 0 and NaN — a negative or Infinity value is truthy and passes straight through. A negative WEB_CUSTOM_MAX_BODY_KB makes the "body exceeds N bytes" check reject every POST search, and a negative WEB_CUSTOM_TIMEOUT_SEC drives an immediate abort on every request. Route both through readPositiveEnvNumber, which falls back to the default for missing, empty, non-finite, or non-positive input. |
||
|
0xfandom
|
f553d0896d |
fix(api): resolve swarm-field tool names by own-property (#2123)
filterSwarmFieldsFromSchema() looked the tool name up with a bare SWARM_FIELDS_BY_TOOL[toolName] on a plain-object map. A tool whose name collides with an Object.prototype member (constructor, hasOwnProperty, isPrototypeOf, propertyIsEnumerable) resolves the inherited function, which is truthy with .length === 1 — so the empty/undefined guard is bypassed and the subsequent for...of throws "is not iterable", failing schema construction for the entire request. Guard the lookup with Object.hasOwn so a prototype-named tool is treated as unmapped, matching the own-key guard already used for provider- supplied tool names in services/api/toolArgumentNormalization.ts. |
||
  StanandLookoff123
|
fb9102c422 |
feat(aimlapi): passwordless onboarding and resumable card top-up (3/3) (#2032)
* feat(aimlapi): add checkout state persistence and sign-in key cache
* fix(aimlapi): cover lock recovery and complete the reset receipt
* test(aimlapi): cover CAS result, reset receipt and sign-in key permissions
* fix(aimlapi): make stale-lock recovery ownership-safe across processes
* test(aimlapi): pass a file URL specifier to the lock workers
* fix(aimlapi): use proper-lockfile and harden checkout-state persistence
* fix(aimlapi): preserve issued keys and survive stale-lock steal races
* fix(aimlapi): surface swallowed lock-retry conditions
* feat(aimlapi): resume interrupted checkouts in the top-up entry points
* fix(aimlapi): keep resumable checkouts through transient and paid states
* fix(aimlapi): surface a lost settled-receipt write to the caller
* fix(aimlapi): harden checkout resume across CLI resume, idempotency and transient errors
* fix(aimlapi): resume settling checkouts and preserve records on ambiguous reads
- Treat 'exchanging' as a paid/resumable status so a run interrupted between
payment and receipt resumes the exchange instead of opening a second,
chargeable checkout (matches pollUntilPaid).
- Preserve the recorded checkout on a malformed-but-successful status read
(AimlapiApiError status 200), alongside the existing transient-error path.
- Print the full recovery key in the receipt-write-failed warning; a masked
key is useless as a last copy.
- Re-register the real client/providerProfile modules in afterAll so the
test stubs cannot leak into later files (mock.restore does not undo
mock.module).
- Bound each lock worker's exit before draining its pipes, and loosen the
held-lock timeout assertion to any errno (Windows is not always ELOCKED).
* fix(aimlapi): stop mock leak, fail closed on spent sessions, clear GUI receipt
- topup.test.ts: capture the real client/providerProfile modules through a
cache-busting query so afterAll restores the genuine module instead of the
corrupted (stub-mutated) reference. Without this the './client.js' stub bled
past afterAll and failed 25 client.test.ts cases whenever it ran first.
- resolveCheckoutSession: fail closed when a resumed session is already
'exchanged' but no settled receipt survived locally, matching pollUntilPaid,
instead of opening a second chargeable checkout for a one-shot key that is
already gone.
- provisionAimlapiKey: return a clearReceipt closure; ProviderManager now calls
it only after persistDraft actually saves, so a second GUI top-up opens a
fresh checkout instead of short-circuiting to a stale key or throwing.
- claimAimlapiTopupState: refuse to replace a stored record that still has an
open resume token for a different intent, so a changed amount cannot strand a
still-payable checkout and open a second one.
- Mask the issued key in the CLI receipt-write-failed warning; a paid-for
credential should not land in scrollback.
- index.ts: note the live CLI/GUI callers and the clear-receipt obligation.
- Regression coverage for exchanged fail-closed, the claim guard, and the GUI
receipt clear.
* fix(aimlapi): inject the topup transport instead of mocking client.js globally
The prior mock.module('./client.js') stub in topup.test.ts leaked past its
afterAll into client.test.ts (25 failures when this file ran first in CI,
bun 1.3.13). Replace it with a local injection seam:
- topup.ts exposes setAimlapiTopupTestDoubles, and both entry points create
their client / write their profile through it (defaults unchanged, so
production behaviour is identical).
- topup.test.ts injects a stub transport through that seam and no longer calls
mock.module at all, so nothing can bleed OUT to client.test.ts.
- It still loads topup.js through a cache-busting ?ts= query so it stays immune
to ProviderManager.test.tsx's mock.module('../integrations/aimlapi/index.js'),
which mock.restore() does not undo and which would otherwise replace the
shared provisionAimlapiKey binding (verified: without the query the barrel
stub reaches this file and every topup test fails).
* fix(aimlapi): converge racing checkouts and preserve state on ambiguous reads
- Close a post-claim race: two runs of the same intent converge on one payment
id, then each can open a session before either records one, and the second
save overwrote the first's resume token (two payable checkouts). Add
recordAimlapiCheckoutSession, a compare-and-swap that only records while the
token is empty; resolveCheckoutSession adopts the winner's session and
abandons the one it just opened, so pay() converges idempotently on a single
charge.
- Resume path now preserves the record on any ambiguous getSession error
(transient, malformed-200, auth/4xx) and retires it only on a definitive
404/410 gone-session, instead of clearing on every non-transient error.
- pollUntilPaid retries a malformed-but-successful (status 200) body instead of
aborting, matching the resume path.
- Regression coverage for the peer-records-first race, ambiguous-error
preserve, 404 replace, and poll-retry-on-200.
* fix(aimlapi): re-validate an adopted peer session before paying it
When two racing runs converge and this one adopts the peer's recorded session,
route that session through the same status classification as the initial
resume: return it only while resumable, fail closed on 'exchanged', and surface
a re-run error on any other terminal status instead of calling pay() on a dead
session. Add regression coverage for an adopted session that is exchanged or
cancelled in the race window.
* test(aimlapi): make abandoned-lock recovery test deterministic
The stale-lock recovery test asserted that all racing claims return the same
payment id. Under a stale-lock steal two recoverers can briefly hold the lock
and mint distinct ids, so that assertion was flaky in CI. A diverged claim is
harmless — it is refused at its next compare-and-swap — so the invariant that
actually matters is that exactly ONE checkout gets established. Drive the
workers through the full claim->record flow and assert a single winner, which
the single-slot store plus the record CAS guarantee deterministically.
* fix(aimlapi): acquire checkout-state off the interactive thread and recover fresh orphans
The interactive top-up flow acquired the checkout-state lock synchronously,
parking the Ink event loop (UI, timers, SIGINT) on Atomics.wait for up to the
5s timeout, and that timeout was shorter than the 30s stale window so a lock
orphaned by an interrupted holder could not be recovered on an immediate resume.
- Add withStateLockAsync + async variants of the state mutators, sharing the
same inner operations. It acquires with the timer-free lockSync (a sub-ms
mkdir) but yields via await between retries, so the UI stays live, and its
longer deadline (15s) covers the stale window so a fresh orphan is reclaimed
once stale rather than timing out. Shrink the stale window to 8s (sub-ms
sections never approach it) so recovery is quick.
- Route topup.ts (CLI + GUI) through the async variants; provisioned.clearReceipt
is now async, and ProviderManager awaits it best-effort so a cleanup failure
cannot surface an error that invites a retry (a duplicate provider profile).
- Regression coverage for async orphan recovery (mutation-checked: a deadline
below the stale window fails to recover). Generous per-file test timeout since
the now-async provisioning yields to a loaded runner's event loop.
* test(aimlapi): cover the async state mutators and speed up the orphan-recovery test
- Add thin contract tests for saveAimlapiTopupStateAsync (CAS accepted/rejected),
recordAimlapiCheckoutSessionAsync (compare-and-swap on the empty resume token),
and clearAimlapiTopupStateAsync (ownership-scoped clear); the CLI/GUI flow now
routes through these and only claimAsync was exercised.
- Back-date the orphaned lock to just inside the stale window so the async
recovery test reclaims it in ~2s instead of burning the full 8s window.
* fix(aimlapi): fail closed on corrupt state, deliver keys on receipt failure, add a reset action
- Fail closed when the checkout-state file is present but unreadable/schema-
invalid instead of reading it as absent: a claim would otherwise overwrite an
open/paid checkout or an exchanged key and open a second chargeable one.
- Never strand a one-shot exchanged key: a receipt-write that throws (lock
timeout / fs / corrupt state), not just a lost CAS, is caught so the CLI still
writes the profile and the GUI still returns the key; the post-delivery clear
is best-effort too.
- Add an explicit discard/reset escape hatch — discardAimlapiCheckoutState, an
"openclaude aimlapi reset" command, and a GUI "Start over" on the top-up error
screen — so a terminal checkout (whose resume token blocks a different intent)
or a corrupt state file can be cleared without editing internal files.
- Surface a failed GUI receipt retirement: retry a few times, then show a
non-blocking warning instead of swallowing it silently.
- Only chmod a config directory this flow actually created (via mkdirSync's
return); never tighten a pre-existing OPENCLAUDE_CONFIG_DIR root. The state
file's own 0600 mode protects the credential.
- Tests for each, incl. corrupt/schema-invalid fail-closed, receipt-write-throw
key delivery (CLI + GUI), discard semantics, retried/surfaced GUI cleanup, and
a POSIX check that an existing config dir keeps its mode.
* fix(aimlapi): surface the CLI recovery-receipt clear failure and cover the reset handler
- The CLI finishCliTopup clear failure only went to logForDebugging (debug-only),
invisible to the user, unlike the loud receipt-write warning and the GUI
warning. Print a [warn] line pointing at "openclaude aimlapi reset" so a
stranded receipt that blocks a different top-up is not hidden.
- Add tests for the aimlapiReset CLI handler (discards a stored checkout /
reports when there is nothing to discard).
- Assert the CLI clear-failure warning is surfaced in the receipt-write-failure
test (mutation-checked).
- Document "openclaude aimlapi reset" and the GUI Start over recovery in
docs/aimlapi-setup.md.
* fix(aimlapi): protect a settled receipt from reset and bind method into the checkout identity
- Discard (CLI reset / GUI Start over) no longer deletes a settled receipt — the
only copy of a paid-for, one-shot key — unless forced. discard now returns
'discarded' | 'kept-settled' | 'none'; the CLI adds --force and the GUI refuses
and points back at the recovering retry.
- Add the payment method to AimlapiTopupIntent so a card->crypto (or reverse)
restart is a different intent and cannot adopt the prior checkout's reused
idempotency id on the wrong rail; covered by a changed-method resume test.
- Narrow the sign-in-key cache: it is a persistence primitive for the follow-up
guided passwordless flow with no in-tree consumer, so drop it from the public
barrel and document the scope (kept in topupState.ts for that follow-up).
- Regression + mutation coverage for the settled-receipt protection and the
method-scoped intent.
* fix(aimlapi): back off receipt-clear retries, expand the discard API, and test Start over
- Add a 150ms backoff between the GUI clearReceipt() retries so the loop can
actually ride out a lock-contention window instead of exhausting three
back-to-back attempts.
- Re-export AimlapiDiscardResult and add resetAimlapiCheckoutSessionAsync so
barrel consumers can name the discard outcome and use a non-blocking reset,
matching the other mutators.
- Cover the GUI Start over recovery: it is 'r' (settings:retry) on the top-up
error screen — the Settings context has no confirm:yes and Enter closes the
panel; tests drive both the discard and the kept-settled refusal path.
- Rename the test's stale-age constant (LOCK_AGE_WELL_PAST_STALE_MS) so it no
longer reads as the source's 8s window.
* fix(aimlapi): serialize the one-shot key exchange and recover receipts before login
Address three checkout-state review findings:
- [P1] Serialize the non-idempotent key exchange behind an exchange lease so
racing same-intent processes mint and record the credential exactly once. The
elected lease holder exchanges and records the settled receipt; a peer that
loses the election waits for that receipt and resumes from it instead of
exchanging in parallel; a lease abandoned by a crashed holder goes stale (past
the client request timeout) and is reclaimed on a later attempt. The lease is
released on a failed exchange so a retry is not blocked for the stale window.
- [P1] Recover a settled local receipt BEFORE authenticating in both the CLI and
guided entry points. A run interrupted after the one-shot exchange but before
the profile write leaves the paid-for key only in that receipt; requiring a
fresh login to reach it stranded the key whenever the password had changed or
the auth service was down. The receipt read is side-effect free and needs no
token, so it now runs first and only authenticates when a checkout must be
created/resumed/exchanged.
- [P2] Fix the guided-recovery key in the setup guide: Start over is bound to r,
not Enter (Enter closes the settings panel).
Covered by state-layer lease tests (acquire/held/stale-steal/settled/gone/
release), a two-process race asserting the exchange runs exactly once with the
loser resuming the receipt, and no-auth-on-settled tests for both entry points.
* test(aimlapi): harden the exchange-lease coverage
Address review follow-ups on the exchange-lease tests (all test-only):
- Gate the two-process race on the loser's own "waiting" status signal instead
of a fixed sleep, and assert it fired, so the test deterministically exercises
the held -> wait -> resume path rather than possibly reading settled directly.
- Type seedPersistedState's overrides as Partial<AimlapiPersistedTopup> so a
misspelled/wrong-typed seed key is a typecheck failure instead of a record
that silently reroutes the test down another branch.
- Cover re-acquiring your own lease (leaseOwner === owner) so the guard that
keeps a caller from mistaking its own fresh lease for a live peer's — and
self-blocking until the stale window — cannot regress unnoticed.
* fix(aimlapi): fence a superseded profile write, protect unreadable state, resume the receipt model
Address three checkout-state review findings:
- [P1] Fence an in-flight checkout before it writes its key to the provider
profile. If a reset (or a fresh top-up) replaces the stored slot while an
abandoned flow is awaiting client.exchange(), that flow's settled-receipt CAS
now misses; previously it still went on to write its stale key and could
clobber the profile the new top-up created. recordSettledReceipt now reports
recorded | superseded | errored, and the exchange path aborts (rejecting, and
pointing the user at rotating the orphaned key) on `superseded` while still
delivering on a transient `errored` so a paid-for key is never stranded.
- [P1] Do not discard an unreadable/corrupt state file without --force. Such a
file could be a damaged receipt holding the sole copy of a one-shot key, so
discardAimlapiCheckoutState now returns `kept-unreadable` and keeps it unless
forced — matching the safety promise (and existing settled-receipt protection)
that reset never loses an issued key. The CLI and guided "Start over" surface
the new outcome and point at `reset --force`; docs updated.
- [P2] Use the settled receipt's model when a peer completed the exchange. The
settled lease branch dropped lease.state.model, so a loser resuming another
run's receipt configured its own --model instead of the one actually
provisioned; it now propagates the receipt's model through both callers.
Covered by: a superseded-mid-exchange fence test, corrupt-discard-needs-force
tests (state layer + CLI + guided GUI), and a two-process race asserting the
loser adopts the winner's provisioned model. All three are mutation-proven.
* test(aimlapi): drop the flaky third guided Start-over drive test
The guided "Start over on a kept-unreadable discard" test added a third
consecutive full Ink mount+drive to ProviderManager.test.tsx. On the CI-pinned
bun (1.3.13) that destabilises the Ink stdin harness (`stdin.ref is not a
function`), so the 'r' keypress never reaches the handler and the awaited
discard call never fires — a timeout unrelated to the code under test (it passes
on local bun 1.3.14). The two-drive configuration is green on CI.
The kept-unreadable behaviour stays covered where it is deterministic: the state
layer (kept-unreadable unforced, discarded on --force) and the CLI handler (the
`reset --force` guidance). The guided keybinding→discard→refusal plumbing is
covered by the identical-structure kept-settled drive test; the kept-unreadable
GUI branch is a trivial mirror of it. A comment records why the third drive is
intentionally omitted.
* wip(aimlapi): converge integration + CLI to the passwordless card-only flow (#1988)
Mid-port checkpoint. Rewrites the AI/ML API integration backend and CLI to the
canonical passwordless, card-only design (target PR #1988), in the current
code style. NOTE: the branch does not yet compile — the GUI (ProviderManager
passwordless rewire) and the aimlapi test suite still reference the removed
API and are the remaining work.
Done (typecheck-clean in these files):
- config.ts: add payBaseUrl + verificationBaseUrl endpoints, buildPartnerReturnUrl.
- topupState.ts: reduce to the #1988 shape — intent keyed on payBaseUrl/
verificationBaseUrl (no `method`); sync lockfile-based state; drop the
exchange-lease, discard/reset-command surface, async variants and
fail-closed-on-corrupt (corrupt reads as absent).
- client.ts: drop the password path (signup/login) and PaymentMethod/crypto;
pay() is card-only.
- topup.ts: rewrite to the passwordless phase-machine flow (checkAccount ->
code sign-in / new-account -> provision or by-key top-up; resolveTopupSession;
pollUntilPaid / pollUntilExchangeSettled / pollUntilByKeyToppedUp). Keeps the
DI test seam (no cross-file mock.module). Profile env writes AIMLAPI_API_KEY
mirror + CLAUDE_CODE_PROVIDER_ROUTE_ID.
- onboarding.ts, messages.ts (canonical copy), validation.ts, index.ts (lean
barrel), providerManagerAimlapi.ts (GUI indirection layer): new.
- CLI: aimlapiCommand.ts (registerAimlapiCommand, --email/--code/--amount, no
--method/reset), main.tsx wiring, handlers/aimlapi.ts (redacted errors).
Mandatory attribution headers wired on EVERY aimlapi request: X-AIMLAPI-Source
(agent/openclaude) + X-AIMLAPI-Partner-ID — in client.ts request() (auth/
checkout) and the config attribution path (inference/catalog).
* wip(aimlapi): port the passwordless provider-manager GUI (#1988)
Rebase ProviderManager.tsx on the #1988 passwordless aimlapi flow (email ->
6-digit code -> low-balance -> top-up / paste-existing-key -> done), replacing
the old password/method/Start-over flow and importing the aimlapi surface from
providerManagerAimlapi.js. Re-apply the current newer-main, non-aimlapi
`apiFormat: 'auto'` feature that the rebase would otherwise revert (form
metadata, toDraft default, display label, startCreateFromPreset default, the
API-format picker's Automatic option, and persistDraft's selectedApiFormat
'auto' -> undefined branch, kept alongside #1988's deferNavigation/onSaved).
Re-export resolveRouteCredentialValue from integrations/index for the GUI.
All source now typechecks; the aimlapi test suite is the remaining work.
* test(aimlapi): port integration + CLI tests to the passwordless flow
- topupState/topup/onboarding/aimlapiCommand tests: port #1988's coverage,
adapting the transport to `globalThis.fetch` stubbing and the profile/prompt
doubles to the module's `setAimlapiTopupTestDoubles` DI seam (no process-global
mock.module, which leaks across files in this repo).
- client/config tests: keep the current repo's stricter versions (complete
session contracts), pruning the removed password/crypto cases.
- Add mandatory-attribution-header coverage on all four request classes: the
client sends X-AIMLAPI-Source + X-AIMLAPI-Partner-ID on auth/checkout, and the
config attribution path sends both on inference/catalog and strips them for a
non-canonical proxy endpoint.
- Remove the reset-based CLI handler test (reset no longer exists).
Full aimlapi integration + CLI suite green (67 tests). ProviderManager GUI test
is the remaining piece.
* test(aimlapi): port the passwordless provider-manager GUI tests (#1988)
Rebase ProviderManager.test.tsx on #1988's version for the passwordless aimlapi
GUI tests (email -> code -> low-balance -> top-up / paste-existing-key), which
mock ./providerManagerAimlapi.js. Re-apply HEAD's newer-main apiFormat 'auto'
test cases (API-mode picker, token field, OpenAI/GPT-5/MiniMax presets) since
the source keeps that feature, and restore the current preset list ('LongCat')
in the test's PRESET_ORDER so navigateToPreset indexes match the real presets.
ProviderManager suite green (42 tests).
* docs(aimlapi): rewrite the setup guide for the passwordless card-only flow (#1988)
Describe the passwordless /provider flow (saved-key continue, new-user email +
6-digit code, paste-existing-key, low-balance top-up) and the card-only CLI
`aimlapi topup --email/--code/--amount` (no --method, no reset). Document the
full endpoint override set and the two mandatory attribution headers
(X-AIMLAPI-Source + X-AIMLAPI-Partner-ID) sent on every request, stripped for a
non-canonical proxy endpoint.
* refactor(aimlapi): let tests inject prompt doubles into the top-up flow
* fix(aimlapi): lock the partner id and complete mandatory-header coverage
Lock the partner id to OpenClaude's own attribution id: drop the --partner-id
CLI flag and the AIMLAPI_PARTNER_ID env override so rebate/revenue-share
attribution can never be redirected. resolvePartnerId() now always returns the
built-in id; the mandatory X-AIMLAPI-Partner-ID header itself is unchanged.
Assert the mandatory X-AIMLAPI-Source header on the catalog/discovery and
inference paths (discoveryService, bootstrap, runtimeMetadata) — the header was
already sent, only the test expectations lagged. Refresh stale password-era copy
in the interactive prompt and a top-up comment left over from the removed flow.
* fix(aimlapi): address CodeRabbit review — settlement, key-safety, redaction
- Wait for a resumed sign-in top-up to settle before returning: the account
non-exchange path now mirrors the by-key flow, so a credited balance is never
reported while the billing operation is still in flight.
- Preserve a freshly minted sign-in key when the balance read is aborted, so an
abort cannot orphan a paid credential and mint a second key on the next run.
- Clear the first-run env-key adoption markers when validation fails, so a retry
re-validates instead of short-circuiting into persisting the unvalidated key.
- Never crash the top-up success screen on an amount parse edge — fall back to
the raw entered amount after the payment has already cleared.
- Show all four API-format options (visibleOptionCount 3 -> 4).
- Document that guided provisioning requires the canonical inference endpoint.
- Add regression tests: resumed-sign-in settlement, aborted-balance key
retention, failed-env-key re-validation, and CLI error redaction.
* test(aimlapi): wait for the masked code frame instead of a fixed delay
The AIMLAPI code-screen assertion captured output after a fixed 25ms sleep,
which is too short on a slower CI runner (Node 24) and intermittently missed the
freshly rendered mask characters. Wait for the masked frame instead so the
assertion is deterministic.
* test(aimlapi): harden the provider-manager GUI flows against CI timing
The GUI top-up flow intermittently failed on a loaded CI runner: the final
keystroke on the success screen was sent in the same tick as the render, before
Ink attached the input handler, so it was dropped and the flow stranded on the
done screen. Add the same input settle the other steps already use.
Also raise the shared waitForCondition default timeout (2s -> 5s). The predicate
is polled every 10ms and returns as soon as it is met, so this only adds patience
for a slow runner and never slows a passing wait — keeping the Ink-driven flows
deterministic under CI load.
* fix(aimlapi): gate attribution headers by trusted AI/ML API host
The client sent the mandatory source/partner headers on every request, but the
auth/app/pay/inference base URLs are all env-overridable — so a request pointed
at a user proxy (notably the balance probe against an overridden inference URL)
leaked OpenClaude's partner/source identity. Send them only when the resolved
request host is aimlapi.com (production or staging, over HTTPS), mirroring the
inference/catalog stripping contract in resolveAimlapiAttributionHeaders. Adds
an isTrustedAimlapiRequestUrl predicate plus canonical-sends / proxy-withholds
regression tests.
* test(aimlapi): wait for the done screen to settle before the final keystroke
Replace the fixed 25ms delay before the success-screen keystroke with an
observable frame-stability wait, so a slow CI runner cannot drop the keystroke
before Ink has committed the render and attached its input handler.
* fix(aimlapi): durable receipts and atomic election for concurrent top-ups
- Restore the atomic checkout-session election dropped during the passwordless
convergence: recordAimlapiCheckoutSession is a first-writer-wins CAS, so two
concurrent runs of the same intent settle on ONE payable checkout — a loser
adopts the winner's token and abandons the session it just opened instead of
leaving two chargeable checkouts. Wired through resolveTopupSession (the create
branch elects then adopts; the resume branch notifies once) and both the CLI
and GUI onSession callbacks.
- Persist the settled receipt (apiKey / apiKeyId / model / settled) in the GUI
BEFORE the profile write, so an interrupted or failed write resumes with the
paid, one-shot exchanged key instead of stranding it (mirrors the CLI).
- Clear the sign-in key cache with the just-minted key id on a sufficient-balance
sign-in: persistDraft runs onSaved synchronously, so the aimlapiIssuedKeyId
state setter has not applied yet — pass the id explicitly.
Adds regression tests for the election (first-writer-wins + loser adoption), the
settled-receipt ordering, and the sufficient-balance cache clear.
* fix(aimlapi): abort on a lost election and keep the receipt write best-effort
- Treat a null recordAimlapiCheckoutSession result as "the slot was cleared by a
sibling that already completed this top-up" and abort, instead of silently
proceeding to pay a second, unrecorded checkout (both the CLI persistSession
and the GUI reportSession). Closes the residual double-charge race.
- Make the GUI settled-receipt write best-effort: the payment already cleared, so
a receipt-write failure (lock contention, full/read-only disk) must not divert
the flow into the top-up error path — the profile write is what matters.
- Align the recordAimlapiCheckoutSession test double with the real semantics:
match on intent + payment id only and return null on a non-matching slot.
Adds a regression test that a sibling clearing the checkout mid-flow aborts
before any /pay call. The sufficient-balance sign-in test now waits for the code
screen to settle before typing (the transition dropped the first keystroke).
* test(aimlapi): settle after each awaited frame so keystrokes aren't dropped
The provider-manager GUI tests type on the line after waitForFrameOutput matches
a new screen, but Ink registers input handlers in an effect that runs after the
render commits. On a loaded CI runner the first post-transition keystroke could
be dropped, stranding the flow and timing out (seen intermittently on Node 22).
Add a short settle after every frame match — returning the same matched frame,
so no assertion changes — which lets the input handler attach before the caller
types. Fixes the class instead of patching individual call sites.
* fix(aimlapi): recover a settled GUI receipt and harden checkout/key edge cases
- Recover a settled checkout receipt in the provider-manager GUI before
provisioning: if a prior run paid + exchanged the key and saved the receipt but
was interrupted before the profile write, finish that write with the retained
key instead of re-entering provisioning against the now-exchanged session
(which fails in resolveTopupSession and strands the paid credential). Mirrors
the CLI.
- Reject a non-HTTPS checkout payUrl at the client response boundary (the
validator required only "openable"), so a session is never retained with an
address the flow refuses later and then polls with no usable link.
- Do not discard a freshly minted sign-in key when its cache write fails: copy
the key into memory before persisting and make the sign-in-cache / top-up-state
writes best-effort, in both the GUI and the CLI, so a lock/permission/disk
failure can't force a second key on retry.
- Narrow the setup guide: the canonical-inference requirement applies to
new-account onboarding + key provisioning; the existing-key top-up runs against
the configured endpoint.
Adds regression tests for the settled-receipt recovery (no re-provision) and the
non-HTTPS payUrl rejection.
* fix(aimlapi): HTTPS checkout callbacks, per-email key cache, safer edges
- Require a credential-free HTTPS base for the checkout return URLs (they embed
the resumable session token) and for the browser return/landing URL, so a
cleartext AIMLAPI_PAY_URL/AIMLAPI_RETURN_URL override can't leak the token or
break the documented HTTPS return-target contract.
- Store sign-in recovery keys as an email-keyed collection instead of a single
global record, so a concurrent/interrupted sign-in for one account no longer
evicts another's key (which forced a duplicate mint). Old single-record files
migrate on read; clear stays per-email ownership-aware.
- Treat post-success receipt cleanup as best-effort in both the CLI (finishProfile)
and the GUI (resetAimlapiCheckoutIntent): the profile is already saved, so a
lock/permission/IO failure clearing the receipt must not report failure.
- Reject scientific-notation amounts: parseAimlapiAmountUsd now requires a plain
decimal with at most two fractional digits, closing the "20.001e0" sub-cent
bypass that silently rounded to a wrong charge.
- Add the pay/verification/return env vars to the config-test snapshot so a set
override can't pollute default-endpoint assertions.
Adds regression tests for each.
* fix(aimlapi): async checkout-state clear for the Ink flow + reject malformed bases
- Clear the checkout receipt through an async lock in the provider-manager GUI:
restore withStateLockAsync + clearAimlapiTopupStateAsync and fire it
best-effort (unawaited) from the save callback, so a contended lock no longer
blocks Ink input/timers/SIGINT after the profile is already saved. The CLI keeps
the sync clear (one-shot command).
- Reject a query string or fragment in the checkout/return base URLs: a base like
https://pay.aimlapi.com/#x would swallow the appended /checkout?...sessionToken
into the fragment, so the token never reaches the callback as a query param.
- Surface a non-fatal CLI note when receipt cleanup fails (the profile is already
saved and the stale receipt reconciles on the next run).
Adds regression tests: async clear ownership, query/fragment rejection, and the
legacy single-record sign-in-cache migration.
* fix(aimlapi): reject bare ?/# delimiters in checkout and return base URLs
url.search / url.hash are empty for a bare delimiter (e.g. https://pay.aimlapi.com/?
or .../#), so those slipped past the query/fragment guard and still corrupted the
appended /checkout?...sessionToken=... . Reject any raw ?/# in the candidate in
both safeHttpsBaseUrl and requireHttpsBaseUrl, and cover the bare-delimiter cases.
* fix(aimlapi): harden checkout recovery — exchange lease, retry modes, payable guard
Addresses a fresh review round on the checkout state machine:
- Restore the cross-process exchange lease (dropped in the passwordless
convergence): the one-shot key exchange is serialized so two processes resuming
the same paid sign-up session cannot both exchange and strand the credential —
the lease winner exchanges, peers wait for its settled receipt.
- Persist the exchange mode in the receipt so a retry that has since become
sign-in still exchanges the paid session instead of minting an unrelated key
and clearing the paid checkout (CLI + GUI).
- Recover the checkout URL on a pending_payment resume by re-issuing the
idempotent pay/top-up (the stable paymentSessionId prevents a double charge)
instead of polling a session the user can never open.
- Route settled-receipt recovery through persistExistingAimlapi for an existing
saved profile / AIMLAPI_API_KEY top-up, so it updates the selected profile
(preserveEnv) rather than minting a new one and copying the env key.
- Confirm before abandoning an already-open checkout: editing amount/auto-top-up
after a checkout URL was opened now requires an explicit re-submit (the old
browser tab stays chargeable and no endpoint can cancel it).
- Treat a credentials/query/fragment inference base as non-canonical so a
`.../v1#x` override cannot be written as OPENAI_BASE_URL and break the shim.
Tests: exchange-lease election + failed-exchange release, retry-exchanges-the-
paid-session, idempotent URL recovery on resume, canonical-gate rejection, and
the re-edit confirmation.
* fix(aimlapi): repair exchange-lease liveness and the re-edit abandon guard
- exchange lease: a peer that finds a live foreign lease now re-attempts on each
poll instead of only watching for a settled receipt, so it resumes the moment
the holder settles OR frees the lease (failed/crashed) rather than hanging the
full 20-minute poll window; folds the wait into the lease loop.
- exchange lease: treat a future-dated exchangeLeaseAt (backwards clock jump or
an edited state file) as stale and reclaim it, instead of reading a negative
age as perpetually fresh and deadlocking every peer.
- re-edit guard: reset the abandon acknowledgement when a new checkout opens so a
further edit to a different amount/auto-top-up is confirmed again instead of
silently abandoning the freshly-opened chargeable tab; clear the opened-checkout
tracking once payment settles so a later re-edit never warns about a paid tab.
- tests: lease release is owner-scoped and preserves a settled receipt; a
future-dated lease is reclaimed; the GUI re-edit warning re-arms after a second
edit.
* test(aimlapi): sync re-edit test on rendered amount; guard vacuous lease seed
- re-edit GUI test: submit only once the edited amount is reflected in the
rendered frame instead of after a fixed 25ms delay, so Enter is never processed
against the stale amount on a slow runner.
- future-dated lease test: assert the seed compare-and-swap actually persisted the
lease before acquiring, so the reclaim path can never pass vacuously.
- exchange lease: record a swallowed release failure via file-backed debug logging
(safe on the Ink GUI path) so a lock/permission problem behind a slow takeover is
diagnosable.
* test(aimlapi): match the complete edited amount in the re-edit frame wait
Prefix matching let "$250" match a stray "$2500" (and "$2500" match "$25000"),
so a wrong-amount input regression could pass unnoticed. Pin the complete value
with a negative lookahead on a trailing digit.
* fix(aimlapi): make the one-shot key exchange crash-durable and per-operation
Three checkout-recovery correctness fixes:
- Persist the exchanged key under the CAS BEFORE returning it. The lease winner
used to hand the /exchange key to the caller, which wrote the receipt only
afterward; a crash in between left the checkout exchanged but its only key
unpersisted, so a retry re-ran (and was rejected by) the spent one-shot
exchange. exchangeKeyWithLease now records the settled receipt via
recordAimlapiSettledKeyAsync (merges over the record, clears the lease) as soon
as the exchange succeeds.
- Use a per-operation exchange-lease owner instead of a module-global id. Two
overlapping top-ups in the same process shared one owner, which the acquire
treats as self and immediately reclaims, so both could POST the non-idempotent
/exchange concurrently. A fresh owner per operation makes the second observe
the first's lease as foreign and back off; a retry within one operation keeps
its owner and still reclaims the lease it released.
- Never serialize an empty apiKey/apiKeyId. The existing-key top-up path reports
apiKeyId: '', which the reader rejects, making the whole settled receipt (and
the paid key it records) unrecoverable. The save path now coerces an empty
key/id to absent so the receipt stays readable.
* fix(aimlapi): refuse to overwrite an unfinished checkout when the intent changes
claimAimlapiTopupState backs a single slot, so rerunning with a different amount,
auto-top-up, or endpoint used to unconditionally replace the stored record. When
the prior checkout had opened a session (a resume token — possibly already paid
but not yet exchanged) or held a settled key not yet written to a profile, that
dropped the only handle to a paid session/key and stranded it permanently.
claim now refuses a changed intent while such a record exists, with an actionable
message to finish or cancel the earlier top-up first (re-running the same intent
still resumes it). A never-advanced claim — empty resume token, unsettled, no key
— is still replaced. The CLI surfaces the message directly; the interactive flow
already clears the prior record on edit, so normal re-edits are unaffected.
* fix(aimlapi): never settle a keyless receipt; keep the paid key reaching the profile
Addresses a further review batch:
- recordAimlapiSettledKeyAsync now refuses to mark a receipt settled (and clear
the lease) when no key resolves from the call or the stored record. A keyless
settled receipt would make a peer resume from a spent one-shot exchange with no
credential; the record and its lease now survive so a retry can still exchange.
- The CLI's pre-profile settled-receipt save is now best-effort (try/catch + a dim
note), matching the earlier saves. A lock/permission/IO failure there no longer
throws before finishProfile, so the paid, exchanged key still reaches the
provider profile.
- startCreateFromPreset drops aimlapiPersistedIntentRef on a fresh flow entry
(in-memory only) so a later resetAimlapiCheckoutIntent can never clear a previous
flow's on-disk receipt against a stale payment id.
- Prompt copy: "Do you have an aimlapi.com key?" / "I already have an aimlapi.com
key" (missing article).
Tests: keyless settle is rejected and leaves the lease intact; the CLI forwards
explicit --amount/--model; the settled-receipt recovery renders the top-up (not
"ready") done copy.
* test(aimlapi): assert the exchange lease stays held on a keyless settle attempt
Tighten the keyless-settlement guard test: a "not settled" assertion also passes
if the lease were wrongly cleared (a peer would then see 'acquired'). Assert the
peer acquisition returns 'held' so the test pins that a keyless settle preserves
the lease for a retry.
* fix(aimlapi): poll the checkout token, not the auth bearer, while waiting on a resumed exchange
pollUntilExchangeSettled was called with the passwordless-auth bearer
instead of the partner checkout-session token, so it polled the wrong
resource. A terminal error there clears the recovery receipt, stranding
a paid one-shot sign-up exchange.
* fix(aimlapi): keep the checkout receipt resumable through an unconfirmed amount/auto-top-up edit
Editing the amount or auto-top-up cleared the persisted checkout intent
and durable receipt immediately, before the abandon-ack confirmation
that gates actually starting a new payment session. A user who edits
and backs out (or completes the still-open browser checkout) before
confirming lost the only mapping to that chargeable checkout, so a
later run would open a new one instead of resuming the paid session.
The reset now happens only once the user has explicitly confirmed
abandonment: claimAimlapiTopupState takes an `abandonExisting` option
that atomically overwrites the retained record under the same lock
acquisition, instead of racing a separate async clear against a
synchronous claim.
* test(aimlapi): sync on the rendered email before submitting in the new receipt-resume test
A fixed sleep doesn't guarantee the TextInput has processed the typed
email before Enter is sent; a loaded runner can drop the submit. Wait
for the typed value to actually render, matching the amount-edit sync
already used later in this same test.
* docs(aimlapi): describe checkout retention as durable, not session-scoped
The prior wording ("retained while the provider flow remains open")
undersold what topupState.ts actually does: the payment identity and
any issued key are persisted to disk, so a restart resumes the same
checkout too, and a prior paid+exchanged run finishes the profile
write on the next run instead of re-provisioning.
* fix(aimlapi): unify error-status extraction, drop dead top-up state, tighten wrappers
- Extract aimlapiApiErrorStatus as the one place that reads an HTTP
status off a caught error, structurally (not `instanceof
AimlapiApiError`) since some callers surface a duck-typed error with
a bolted-on `status` instead of the real class; use it at both call
sites that previously duplicated (and disagreed on) this check.
- Remove aimlapiPaymentSessionId/isAimlapiTopupRunning: both were
write-only state (declared with a blank destructure slot, never
read), so every setter call scheduled a render for no observable
effect.
- Switch providerManagerAimlapi.ts's wrappers to `...args` forwarding
so an implementation gaining a parameter can't silently get it
dropped by a wrapper that still names the old ones positionally.
- Stop exporting pollUntilPaid from the aimlapi barrel; nothing
imports it through there (topup.test.ts imports it directly from
topup.js), so keep it out of the public surface.
- Normalize the email key while rebuilding the sign-in key store's
collection branch on read, matching the legacy single-record
migration branch right above it - a hand-edited or older-build file
with a mixed-case key would otherwise be invisible to
loadAimlapiSignInKey and mint a duplicate key.
* test(aimlapi): cover resetAimlapiCheckoutSession, by-key top-up args, and error edges
- resetAimlapiCheckoutSession: refreshes the payment session while
preserving a minted key, and is a no-op when there's no key to
preserve.
- ProviderManager: a low-balance saved key that gets topped up charges
the EXISTING key via topUpAimlapiByApiKey (apiKey, non-empty
paymentSessionId, empty resumeSessionToken) instead of opening a new
passwordless-account checkout - previously only exercised through
the default test mock, with no assertion on the call.
- The three negative assertions in the top-up progress-frame check
tested strings that don't exist anywhere in this GUI (CLI-only or
pure invention), so they could never fail; add a check against the
real failure copy so a regression that silently fails at that point
is actually caught.
- CLI: pin the --no-open default (false) when the flag is absent, and
cover the non-Error (thrown string) branch of the handler's
credential-redaction path - both previously only exercised through
the Error/AimlapiApiError branches.
* fix(aimlapi): close checkout-state concurrency and exchange-lease races
- saveAimlapiTopupState now merges resumeSessionToken like the other
retained fields instead of spreading the caller's value verbatim. A
caller saves this record at points where its in-memory copy is still
empty (right after sign-in, before a checkout session exists); a
concurrent peer running the same intent can have already elected and
recorded a real token in that window, and the unconditional spread
was overwriting it with "", stranding the peer's chargeable checkout.
- The exchange lease is sized for a single POST (EXCHANGE_LEASE_STALE_MS,
75s) but a resumed wait-exchange holder can sit in a read-only poll
for up to POLL_TIMEOUT_MS (20 minutes) before ever reaching that POST.
Without refreshing, a peer would see the lease go stale mid-wait,
reclaim it, and risk a second concurrent /exchange on the same
one-shot session. Add refreshAimlapiExchangeLeaseAsync and call it
every poll iteration.
- When a peer finishes /exchange and records the settled key WHILE this
process holds the lease and is polling/exchanging, the poll seeing
the session flip to 'exchanged' threw a hard failure instead of
resuming from that peer's settled receipt. Re-check for a settled
receipt before releasing the lease and rethrowing.
- claimAimlapiTopupState's abandonExisting no longer drops an
already-minted (but not yet paid) existing-account key when
overwriting a retained checkout for a different amount/auto-top-up -
it now merges apiKey/apiKeyId/model in, matching
resetAimlapiCheckoutSession's retain-key pattern. A fully settled
(paid + exchanged) credential is refused unconditionally regardless
of abandonExisting, since that confirms giving up an UNPAID checkout,
never an already-paid one.
* fix(aimlapi): guard GUI checkout abandonment and receipt recovery
- The abandon-ack gate only armed once a checkout URL surfaced
(aimlapiOpenedCheckoutRef), but resolveTopupSession can already elect
and persist a resumeSessionToken before that point. Backing out in
that window then editing the amount hit claimAimlapiTopupState's
generic refusal instead of the same confirm-to-abandon flow. Extend
the gate to also cover a persisted (not yet opened) intent.
- Persist an existing-account key minted at sign-in into the top-up
receipt itself (mirrors the CLI), not just the separate sign-in-key
cache, so a restart before settlement can resume from one
self-contained record instead of depending on two files staying
consistent.
- reportSession's terminal branch (a cancelled/expired/dead session)
always fully wiped the receipt; mirror the CLI's persistSession,
which retains an already-minted key (fresh payment session, dead
token dropped) and only falls back to a full clear when there's no
key to keep.
- Submitting the email screen unconditionally reset the whole
onboarding identity, silently abandoning a chargeable checkout on an
accidental Esc-back-and-resubmit of the same email. Require the same
explicit confirmation an amount edit does when a resumable checkout
exists.
- "Set up a new key or switch account" only cleared in-memory fields,
leaving a durable receipt from an earlier interrupted top-up (this
mount's refs were never populated for it, since it may be from an
earlier process) to hit the same refusal on the next onboarding
attempt with no way to recover short of deleting the file by hand.
Force the next claim to override it once.
- existingAimlapiCredential() rejected saved-profile discovery whenever
the AMBIENT AIMLAPI_INFERENCE_URL wasn't canonical, even for a
profile that was itself saved against the canonical endpoint. Narrow
the canonical requirement to what it's actually protecting: reading
the ambient env key, and sending a saved key to a non-canonical
endpoint (the existing per-profile check).
- The post-signup success screen claimed a magic link was emailed; this
flow is passwordless email-code sign-in, no magic link is ever sent.
Point at the dashboard instead.
* docs(aimlapi): note the interactive/CLI auto-top-up default mismatch
The guided GUI flow pre-selects auto-top-up on; the CLI's --auto-top-up
only enrolls when explicitly passed. Left both defaults as-is (auto-top-up
is a real billing behavior, not something to flip unilaterally) and
documented the asymmetry so it's not a surprise either way.
* test(aimlapi): sync on the settled frame before confirming switch-account
A fixed sleep doesn't prove the Select's focus actually moved to the
second option; on a loaded runner the following Enter could land on
"Continue with your saved API key" instead and assert against the
wrong branch. Wait for the frame to stop changing, matching the
settle-poll pattern already used elsewhere in this file.
* fix(aimlapi): stop the exchange poll when a peer reclaims the lease
The periodic lease refresh added to pollUntilExchangeSettled discarded
its result (`.catch(() => false)`), so a peer reclaiming the lease
mid-wait was silently ignored: the poll kept going, returned normally
once the session left 'exchanging', and the caller walked straight
into the non-idempotent /exchange POST with no ownership check of its
own — racing whatever the peer was doing with the same one-shot
session. The comment claiming this was safe ("resolves on this
function's next outer retry") was simply wrong: there is no outer
retry on the success path, control goes directly to the POST.
Distinguish a thrown refresh (transient lock contention — best-effort,
retry next iteration) from an explicit `false` result (the lease is
definitively no longer ours) and bail out on the latter, so the
caller's existing catch block re-checks for the peer's settled
receipt (or fails the run, requiring a re-run) instead of racing it.
* test(aimlapi): require the settle-wait frame to actually differ from before the keypress
waitForCondition polls every 10ms; on a loaded runner two consecutive
polls can both land before Ink has processed the keypress at all, so
the "stable frame" check was satisfied by the unchanged PRE-keypress
frame, sending Enter before focus ever moved to the second option.
Snapshot the frame before the keypress and require the settled frame
to differ from it, not just be internally stable.
* fix(aimlapi): elect the retained key atomically, stop blocking Ink on claim
- Two concurrent sign-ins for the same intent could each mint their own
existing-account key before either save landed, and saveAimlapiTopupState
(last-writer-wins) let whichever saved last silently overwrite the
other's key on disk while both runs kept using their own in-memory
copy. Elect apiKey/apiKeyId first-writer-wins (same as
resumeSessionToken already is), re-check the receipt right before
minting so a losing run adopts the winner's key instead of minting a
second, and re-check again after a save that lost the election so the
run's own in-memory key matches what's actually on disk. Apply the
same first-writer-wins election to the separate GUI sign-in-key cache
(saveAimlapiSignInKey), which had the identical last-writer-wins gap.
- The GUI called the sync claimAimlapiTopupState directly from an event
handler; its lock retry blocks the whole event loop (Atomics.wait) for
up to LOCK_TIMEOUT_MS on contention, freezing Ink rendering, Esc, and
SIGINT — exactly what resetAimlapiCheckoutSessionAsync already exists
to avoid for the same reason. Add claimAimlapiTopupStateAsync (sharing
the same claim logic via an extracted operation function) and switch
the GUI to it, making startAimlapiTopup async.
recordAimlapiCheckoutSession (the reportSession/onSession path) has the
same sync-lock exposure but is called from a callback whose return value
AimlapiProvisionOptions.onSession drives synchronous control flow in
several places across both the CLI and GUI provisioning paths; making it
async is a larger, riskier contract change deliberately left out of this
pass.
* fix(aimlapi): never pair a new key with a stale or unrelated apiKeyId
saveAimlapiTopupState's apiKeyId fallback still read current.apiKeyId
even when current.apiKey was empty (the id-without-a-key case) or when
state carried a genuinely new apiKey with its own empty-id sentinel,
letting a fresh key get silently tagged with an unrelated leftover id.
Gate apiKeyId on the same winner apiKey came from instead of falling
back to current independently.
* fix(aimlapi): stop cross-account key leaks, lease key-minting, keep GUI CAS async
- claimAimlapiTopupState's abandonExisting carried a retained apiKey into
ANY differing intent, including a switch from account A to account B
(the GUI's forceAbandonExisting path). A B-flow restart before the
profile write could then initialize from the receipt and call the B
checkout with A's credential — crediting A while B's flow saves A's
key. Gate the carry-over on the intent's account/key identity
(`email`) matching, not just abandonExisting.
- The key-choice screen (I am a new user / I already have a key) reset
the whole onboarding identity unconditionally on either choice, even
when Esc had backed all the way out from the amount screen past an
already-opened, still-chargeable checkout. Apply the same
abandon-confirmation gate startAimlapiEmailOnboarding already uses.
- POST /v1/keys (minting an existing-account key) had no cross-process
serialization: two concurrent runs for the same intent could each
observe no retained key and both mint, orphaning whichever key lost
the first-writer-wins receipt race. Add a key-mint lease (mirroring
the exchange lease's acquire/release shape) so exactly one process
ever mints; a peer backs off and adopts the winner's recorded key.
- The interactive flow already claimed asynchronously, but still called
the synchronous saveAimlapiTopupState and recordAimlapiCheckoutSession
directly from an event handler and the onSession callback — either
could block the whole event loop for up to LOCK_TIMEOUT_MS under lock
contention, freezing rendering, Esc, and SIGINT while a payment
session is being created. Add async CAS variants and await them; this
needed widening AimlapiProvisionOptions.onSession to allow returning a
promise, since its return value drives resolveTopupSession's session
election.
- An ambient AIMLAPI_API_KEY takes the by-key route with
aimlapiExistingUsesEnv, so the eventual profile intentionally stays
keyless. The settled-receipt save before that write unconditionally
copied the env value into aimlapi-topup.json regardless, expanding a
secret's on-disk exposure surface for no recovery benefit (a restart
re-reads the same env var). Keep an env-backed receipt credential-free.
* fix(aimlapi): preserve the key-mint lease across unrelated CAS writes
saveTopupStateOperation and recordCheckoutSessionOperation merged the
exchange lease but not the key-mint lease added in the previous commit:
AimlapiCheckoutState (what every caller spreads checkoutState from)
carries neither lease pair, so an unrelated write - persisting the
exchange flag, or electing a checkout session - silently dropped an
in-flight peer's key-mint lease. A third process would then see the
slot as free and mint its own key, reopening the exact double-mint race
the lease exists to close. Fall back to the current lease the same way
the exchange lease already does.
Also: add a future-dated key-mint lease reclaim test mirroring the
exchange lease's, and align the default saveAimlapiTopupStateAsync test
mock with the real CAS (match on intent + payment id, keep the first
writer's resumeSessionToken/apiKey) so it no longer accepts a write the
real store would reject.
* test(aimlapi): preserve the key-mint/exchange lease in the mocked GUI CAS writes
saveAimlapiTopupStateAsync's and recordAimlapiCheckoutSessionAsync's
default mocks spread { ...state } as their write's base, same gap as
the real saveTopupStateOperation/recordCheckoutSessionOperation had
before the previous commit: neither lease pair survived a write whose
state didn't carry them (which is every real caller, since
AimlapiCheckoutState exposes neither).
Fixing the merge alone wasn't enough — the same two mocks' "does this
write still belong to this slot" check also compared lease fields as
if they were part of the intent identity, so a write that seeded a
lease value failed to match the just-claimed record and silently
no-op'd instead of persisting anything. Exclude both lease pairs from
that comparison too, matching the real matchingStateOrNull (which only
ever compares INTENT_KEYS + paymentSessionId).
* fix(aimlapi): recover ambiguous key-mint/exchange outcomes before releasing leases
createKey and /exchange are both non-idempotent with no server-side retrieval
path, so a lost response after the request actually committed left three
races: a retry could exchange (or mint) a second time and orphan the first
credential, or the CLI's exchange caller would surface a generic network
error instead of the accurate "already exchanged, rotate the key" guidance.
exchangeKeyWithLease now distinguishes a genuinely ambiguous transport
failure of the /exchange POST itself from other doExchange failures (a
pre-POST bail on a reclaimed lease, or a definite rejection): only the
former re-checks the session status directly, surfaces the already-exchanged
error when confirmed, and otherwise leaves the lease held instead of
releasing it into a race. mintExistingAccountKeyWithLease applies the same
ambiguous/definite split before deciding whether to release its lease.
The GUI sign-in flow had an equivalent gap one step earlier: two concurrent
code-verification races could each see an empty key cache and both mint
before either save elected a winner, so the loser never adopted the winner's
key. completeAimlapiCodeSignIn now serializes the cache lookup and mint
behind a new email-scoped lease in topupState.ts, so a losing process waits
and adopts the winner's cached credential instead of minting its own.
* fix(aimlapi): treat caller-aborted mutations as ambiguous and dedupe the transport helpers
client.request rethrows a caller-driven abort as the raw abort error instead
of wrapping it in AimlapiApiError, so the ambiguous-outcome checks added for
createKey and /exchange missed it: cancelling client-side does not stop a
non-idempotent POST from completing server-side, but the lease was still
released as if the request definitely failed, leaving the door open to a
retry racing a second mint/exchange. All three call sites (the checkout-time
key-mint lease, the exchange lease, and the sign-in key-mint lease) now also
hold the lease when the caller's own signal fired.
Extracted the duplicated abortError/sleep/isAmbiguousTransportApiError
helpers shared between topup.ts and onboarding.ts into transport.ts so the
ambiguity rule can't drift between the CLI and GUI paths. Switched the GUI
sign-in flow's cache save to the async, lock-yielding variant and logged its
lease-release failures for parity with the other leases.
* fix(aimlapi): close six checkout-state races found across the claim, lease, and recovery paths
claimAimlapiTopupState's in-progress check only looked at
resumeSessionToken/settled/apiKey, so a receipt claimed just before its
non-idempotent POST (/v1/keys or /exchange) still looked blank and
replaceable to a different intent. A competing claim could overwrite it
mid-flight, leaving the in-flight request's eventual CAS save with no
matching record to land in and orphaning the credential it was about to
mint or exchange. The claim now also refuses (unconditionally, even under
abandonExisting) while either lease is live.
The sign-in key-mint lease's 75s stale window exactly matched createKey's
worst-case duration (60s) plus the async lock's own timeout (15s) for the
cache write that follows, with zero margin for anything else. A legitimately
still-working holder could lose the lease to a peer moments before its
result was cached. It's now refreshed right after createKey succeeds, giving
the cache-write phase its own fresh window.
ProviderManager's code-verification path called the synchronous
saveAimlapiSignInKey, whose lock retry blocks the whole event loop for up to
five seconds on contention — freezing Ink rendering, timers, Esc, and SIGINT
right after a sign-in. Switched to the async variant, exported through
providerManagerAimlapi.ts alongside the other async cache operations.
The three session polling helpers typed onSession as returning void and
never awaited it, even though ProviderManager's callback is async and starts
receipt cleanup before returning. A terminal session (cancelled/expired/
failed, or a dead session) could let the UI reach the amount screen before
the durable receipt was actually reset, so an immediate retry still saw the
stale resume token and got rejected as "not yet abandoned." Both sides now
await through to completion.
The confirmed email-switch flow cleared the in-memory checkout intent and
fired an un-awaited, error-swallowing state clear, but derived its later
claim's abandonExisting only from refs that clear had just wiped — so a
slow or failed clear left the user's explicit confirmation unenforced at the
claim itself. It now sets the same one-shot force-abandon signal the
"switch account" flow already uses for exactly this kind of on-disk,
this-mount-invisible conflict.
A cached sign-in key that the server had revoked was indistinguishable from
one that was merely unreachable: both collapsed into balanceStatus:
'unknown', which re-cached the same dead key and sent the user to manual-key
entry with no way back into the guided flow short of deleting local state.
A definite 401/403 against a cached (not freshly minted) key now invalidates
the stale cache entry and mints one replacement before falling back to the
generic unknown-balance path; every other (ambiguous) failure still leaves
the cache untouched.
Extracted a shared claim/lease-liveness helper in topupState.ts and added
regression coverage for each race — including two that hold a mocked
createKey/reset call open to prove the competing operation actually waits
instead of just asserting on the end state.
* fix(aimlapi): clear the stale force-abandon flag on a fresh preset entry
aimlapiForceAbandonExistingRef is armed when the user confirms abandoning a
checkout during an email switch, then consumed by the next claim. If that
claim never runs — the switch's own onboarding fails and the user backs all
the way out to preset selection instead of retrying — the flag stayed armed.
Re-entering the aimlapi preset with an unrelated email then passed
abandonExisting: true on its first claim with no confirmation for that flow,
silently overwriting whatever unpaid checkout was still on disk.
startCreateFromPreset now resets the flag alongside the other per-flow refs
it already clears on fresh entry.
Also swapped a fixed 20ms sleep in the cross-intent concurrency test for a
signal fired from the held-open /v1/keys handler, so the test can't flake
under CI load waiting for the run to reach the point it needs to race.
* fix(aimlapi): close the remaining confirmation, cleanup, and lease gaps in checkout state
The API-key-choice screen's own confirm-abandon gate (Enter twice to accept
"a checkout from this account is still pending") reset the onboarding
identity but never armed the force-abandon signal the email-switch and
switch-account flows already use. A contended or failed pre-clear left the
next claim to hit the CAS's unconfirmed-conflict refusal despite the user
having just confirmed abandonment through this exact screen.
reportSession('')'s terminal-session handler discarded the persisted intent
ref before its reset/clear attempt settled, and swallowed any failure as
success. A lock timeout or I/O error then left the durable receipt exactly
as it was, but with no ownership left in memory to retry cleanup or to route
a later conflicting claim through the normal confirmation gate — the CAS
just rejected it outright. Ownership now only drops once the transition
actually commits; a failure is logged and the ref stays populated so the
existing gate covers the next claim.
The sign-in key-mint lease's stale window already had zero margin for its
own refresh call's lock wait (up to 15s) on top of createKey's own worst
case (60s) and the cache save's lock wait (another 15s) — 90s with nothing
left over. Widened it to 150s and lengthened the losing side's patience to
match, and stopped silently ignoring a refresh that reports lost ownership:
it's now logged for diagnosability even though the save itself stays safe
to attempt regardless (first-writer-wins makes a losing write a no-op).
Both onSaved completion paths (persistExistingAimlapi and persistAimlapiKey)
called the synchronous clearAimlapiSignInKey from Ink's synchronous save
callback, whose lock retry blocks the event loop for up to five seconds on
contention — freezing rendering, timers, Esc, and SIGINT right at
completion, the same class of bug already fixed for the sign-in save path.
Re-exported the async variant through providerManagerAimlapi.ts and switched
both call sites to fire-and-forget it instead.
* fix(aimlapi): stop the flow instead of risking a stranded key on a receipt-write failure
/exchange (and the by-key top-up) is a one-shot operation: once it succeeds,
the issued key exists only in memory until a durable copy lands somewhere.
Both the CLI and the GUI wrote the local recovery receipt right after that,
but treated a failure there as best-effort and proceeded straight into the
provider-profile write regardless. If the receipt write failed and the
profile write then also failed — or the process was interrupted between the
two — the key was gone: nothing durable ever recorded it, and a retry can't
re-exchange an already-spent session to get it back.
Both paths now treat the receipt as a required checkpoint rather than an
optional resume aid: a failure here stops the flow with a clear error
pointing at the one real recovery path (rotating the key from the aimlapi.com
dashboard) instead of silently continuing. This shouldn't cost much in
practice — the underlying CAS write already retries substantially on lock
contention before giving up, so a failure this deep signals a real problem
rather than a transient blip a fallback write would likely have hit too.
Added failure-injection coverage for both paths: the CLI test breaks the
config directory (a file where a directory is expected) right as the
exchange response lands, so the post-exchange save fails deterministically
without relying on OS-specific permission semantics; the GUI test mocks the
save to reject directly and asserts the profile write is never reached.
* test(aimlapi): strengthen receipt-write-failure coverage and document the recovery path
Both the CLI and interactive-flow tests for the post-exchange receipt-write
failure only asserted the generic error text, which would still pass if the
issued key id — the actual recovery handle the error exists to surface — got
dropped from the message later. Both now assert the id appears too. The
interactive test also confirms the screen stays usable after the error: a
retry reaches the amount-submission path again instead of the flow being
stuck.
Documented the resulting behavior in the setup guide: since the key exchange
is one-shot, a receipt-write failure after a successful payment now stops
both flows with an error naming the issued key, rather than continuing
silently — recovery is manual, via rotating that key on the aimlapi.com
dashboard.
* test(aimlapi): move to end-of-line before clearing the email field after Esc
Rebasing onto current main picked up the input layer's DEL-coalescing fix,
which now correctly respects the cursor position for a backspace run instead
of dropping it. These three tests backspaced assuming the cursor sat at the
end of the retained email text, but cursorOffset is a single state shared
across every screen's text field and was last set for the amount screen (its
default "25" is 2 chars) — going back via Esc never resets it, so the cursor
was actually stuck mid-string. Sending an explicit end-of-line sequence
before the backspaces makes the clear correct regardless of where the stale
cursor was left.
* fix(aimlapi): close six checkout/onboarding gaps from the latest review pass
Treats a malformed-but-2xx key-mint response as ambiguous (not proof of
failure) so an unusable receipt no longer releases the mint lease and risks
an orphaned credential; fences startAimlapiTopup's cancellation to an
epoch created before the state-lock await so Esc/unmount during that wait
can no longer barge back in; makes the checkout-receipt read fail closed on
a permission/IO/parse/schema failure instead of silently claiming over it;
completes (or reconciles) a settled by-key receipt instead of stranding it
at the post-payment model picker; retires a sign-in mint lease together
with its cache entry so it can't resurface as held once the cache is later
cleared; and adds a --code-stdin path plus a deprecation warning so the
passwordless code no longer has to travel through shell history or argv.
* fix(aimlapi): reconcile env-credential receipts and stop endorsing AIMLAPI_CODE as safe
reconcileSettledAimlapiTopupStateAsync matched a stale settled receipt by
its stored apiKey, but an env-sourced credential's receipt never persists
one, so that path stayed permanently stranded; it now matches on the
absence of a stored key when the caller is reusing an env credential.
Also stops recommending AIMLAPI_CODE as an equivalently safe alternative
to the deprecated --code flag, since typing it inline still lands in
shell history, and adds a lock-release assertion to the fail-closed
receipt-read tests.
* fix(aimlapi): await stale-receipt reconciliation and stop overclaiming --code-stdin's history safety
Reconciling a stale settled receipt matches by the by-key credential's
apiKey, which a genuinely new top-up for that same credential can also
produce — firing the reconcile call without waiting for it left a window
where a fresh settlement landing in that gap could be swept up by it.
Await it before moving on so the two can no longer interleave.
Also narrows the --code-stdin messaging: it only guarantees the code stays
out of this process's argv/`ps` output, not shell history in general,
since that still depends on how the caller feeds stdin.
* fix(aimlapi): keep the configured screen locked until reconciliation finishes
Clearing isAimlapiKeyValidating before the reconcile await let the
aimlapi-configured screen's Select (and its Esc binding) become
interactive while that reconcile was still running in the background —
it carries no abort signal of its own, so aborting the surrounding
controller only stops this flow from acting on the result, not the
reconcile itself. That left a window where the user could start a
competing top-up for the same credential and have its fresh settlement
caught by the still-in-flight reconcile. Both now stay gated on
isAimlapiKeyValidating through the whole wait.
* fix(aimlapi): fail closed on the sign-in cache, bind env receipts by identity, and commit minted keys
Makes the sign-in key cache and its mint lease match the checkout
receipt's fail-closed contract: only ENOENT means no record, so a
permission/IO/parse failure can no longer be mistaken for "nothing
cached, no lease held" and authorize a second createKey call or a
concurrent lease acquisition.
Reworks reconcileSettledAimlapiTopupStateAsync to match on the by-key
checkout intent's non-secret key fingerprint (already carried in the
persisted email field) instead of the raw stored apiKey or its mere
absence — an env-backed receipt never stores its key, so absence alone
couldn't tell two different env credentials' receipts apart, letting one
credential's balance check discard another's still-unrecovered payment.
Treats persisting a freshly minted existing-account key as a commit
point in the CLI's mint-with-lease path: a write failure now stops the
flow with a recovery-oriented error and leaves the lease held, instead
of continuing with the key only in memory where an interruption before
the later checkout/profile save would orphan it once the lease goes
stale.
Adds a shared isValidAimlapiSignInCode check so both the CLI and the
interactive flow reject a malformed passwordless code (empty,
non-numeric, wrong length) before it ever reaches verifySignInCode.
* fix(aimlapi): reject an array-shaped sign-in cache/lease file instead of degrading to empty
typeof [] === 'object' and [] !== null, so readJsonObjectFile's shape
check let a JSON array through as if it were a valid store. Both readers
then found no matching entries and returned {}, exactly the "no cached
key, no live lease" outcome the fail-closed contract exists to prevent —
authorizing a second createKey call or a lease acquisition over a
possibly-live one.
* fix(aimlapi): commit the sign-in key as a checkpoint and retire a completed mint's lease
mintOrAdoptSignInKey swallowed a failed cache commit and returned the
minted key only in memory — the same non-idempotent-mutation gap already
closed for the CLI's mintExistingAccountKeyWithLease. A commit failure
now stops the flow with a recovery-oriented error and leaves the lease
held, instead of risking the key becoming unrecoverable once the lease
ages out and a retry mints a second one.
Also retires the checkout key-mint lease in the same save that elects a
freshly minted key, mirroring how the exchange lease is already cleared
on settle. Without it, backing out of an unpaid checkout and confirming
a different amount right away still hit the "minting or exchanging"
refusal for the full 75s stale window even though the mint had already
completed.
* fix(aimlapi): make key-mint lease retirement owner-checked, preserve error causes
Retiring the checkout key-mint lease on a successful mint (the previous
commit's fix) cleared it unconditionally, with no check that the save
still belonged to the owner that acquired it. createKey has no refresh
mechanism, so a slow response can let the lease go stale and be
reclaimed by a peer before the original owner's save lands — clearing
the lease then would drop that peer's still-live one and let a
differently-amounted claim proceed as though minting were done while
the peer's mint was still genuinely in flight.
Replaces the raw saveAimlapiTopupState call in
mintExistingAccountKeyWithLease with a dedicated
recordAimlapiMintedKeyAsync that takes the acquiring owner and only
retires the lease while it's still theirs — mirroring how
recordAimlapiSettledKeyAsync already handles the exchange lease.
Also adds `cause` to the three recovery-oriented errors thrown on a
receipt-write failure, so the underlying persistence error stays
diagnosable instead of being replaced by the wrapper message alone.
* test(aimlapi): assert a stale owner's minted key is still persisted
The reclaimed-peer-lease regression test only pinned the lease
bookkeeping, so a variant regression that skipped the write entirely for
a non-owning caller (e.g. an early return on lease-owner mismatch) would
still pass while silently discarding a real, non-idempotently minted
key. Asserts the receipt retains it regardless of who currently holds
the lease.
* fix(aimlapi): persist and charge the endpoint a manually-entered key was actually validated against
persistExistingAimlapi's no-existing-profile fallback saved the profile
with resolveEndpoints().inferenceBaseUrl (the current ambient endpoint)
instead of aimlapiInferenceBaseUrl (the endpoint this flow actually
validated the key and will charge against). The two diverge for a
manually-entered key: after "Set up a new key or switch account" resets
aimlapiInferenceBaseUrl to the ambient default, draft.baseUrl (what
validateAndPersistAimlapiKey actually calls the balance/top-up endpoints
with) keeps the OLD profile's endpoint if it differs (e.g. a canonical
saved profile while AIMLAPI_INFERENCE_URL currently points at a proxy).
Now captures the validated endpoint into aimlapiInferenceBaseUrl as soon
as the low-balance branch is reached, and the fallback save uses that
state instead of re-resolving the ambient endpoint — so the top-up
charge and the persisted profile both follow the endpoint that was
actually validated.
* fix(aimlapi): reject stale key-mint results, make the exchange checkpoint mandatory, validate lease pairs
recordAimlapiMintedKeyAsync previously let a stale owner's delayed
createKey result land beside a peer's reclaimed, still-live lease: since
no key was recorded yet, first-writer-wins accepted the stale result
outright, so the reclaiming peer's own (equally real, non-idempotent)
mint got silently discarded once its own save landed — turning one lost
credential into two. It now rejects a result whose ownership was already
lost when nothing is recorded yet to adopt instead, surfacing a
recovery-oriented error naming the issued key id rather than risking a
second orphan. The caller now also returns whichever credential is
actually durably recorded, not always its own.
exchangeKeyWithLease's own settled-receipt commit — the only durable
record of a one-shot /exchange result until the caller's later,
separate save runs — was only logged on failure. A crash in that
window left the paid session exchanged with its key absent from local
recovery state, so a retry could only report the session was already
exchanged with no way to recover automatically. The commit is now a
required checkpoint: its failure stops the flow with the existing
recovery guidance and leaves the exchange lease held, exactly as the
analogous key-mint checkpoint already does.
The receipt schema validated the exchange lease pair but not the newer
key-mint one, and even the exchange check only verified each field in
isolation — a one-sided pair (an owner with no timestamp, or vice versa)
passed either way. A shared validator now enforces both lease pairs are
either fully present or fully absent, so a malformed or partial lease
can no longer be silently accepted as "not currently live" and let a
claim replace it out from under an in-flight mint or exchange.
* fix(aimlapi): fail the exchange when the settled-receipt commit no-ops, not just when it throws
recordAimlapiSettledKeyAsync silently returned without writing whenever the
CAS no longer matched (checkout cleared/reset mid-flight) or no credential
could be resolved to settle with. exchangeKeyWithLease only caught thrown
errors, so both no-op paths let a successfully exchanged key return with no
durable local receipt. The function now returns a boolean, and the caller
treats false exactly like a thrown save error.
Also fixes recordAimlapiMintedKeyAsync returning the raw untrimmed apiKeyId
instead of the trimmed value it actually persisted.
---------
Co-authored-by: Lookoff123 <bataryshkinairina@gmail.com>
|