Repository navigation
feat(analyst): harness robustness for data-question runs - #1441
anandgupta42 wants to merge 40 commits into
Conversation
…, scratch bridge, checks, skills Analyst mode becomes a general-purpose, verifiable data-analysis agent. Every change is engine-agnostic and applies beyond individual evaluation runs; the evaluation traces document the development history and evidence behind each item. Query execution - Default 120 s execution deadline on every warehouse (`queryTimeoutMs` per call > connection `query_timeout_ms` > env `ALTIMATE_SQL_QUERY_TIMEOUT_MS`; `0` disables). Postgres uses `statement_timeout` (never loosening a stricter server default), Mongo `maxTimeMS`. - SQLite and DuckDB run in one supervised subprocess per connection: Bun `Worker.terminate()` does not preempt a blocking native call and the legacy `duckdb` binding's `interrupt()` is a no-op, so cancellation kills and respawns the helper (lock released, state loss disclosed). The helper starts in a private 0700 directory with a minimal environment and an empty `bunfig.toml`, so project preloads and credentials never reach it. - `sql_execute` leads row-limited or clipped results with a `Preview: N of M returned rows … saved to <path>` envelope, caps the body at 50 rows / 16 KiB, spills all returned rows as typed JSONL, validates `limit` (1–10,000), and exposes `query_timeout_ms`. - Driver and provider errors are surfaced: a stream that ends with finish `other`, no content and no usage is reported as a decode failure instead of a silent empty answer (Bedrock event-stream decoding under Bun; pins `@smithy/util-buffer-from`/`is-array-buffer` to 4.2.2, see also #1425). Cross-engine analysis - Session-scoped in-memory `scratch` DuckDB plus `sql_materialize`: run a read-only query on any connected warehouse (SQL engines and Mongo reads) and store all rows as a typed scratch table, so joins across engines happen in the engine rather than by copying key lists through the conversation. - `sql_check`: executable result invariants — fan-out, join-key profile (exact vs normalized matches, format differences), grain uniqueness, null share, extraction-regex coverage — with a verdict line and numeric metadata. - `schema_inspect` appends value samples (distinct values for low-cardinality columns, quoted examples for text columns so whitespace/case variants show, null share); empty or near-empty results with a literal filter get a probe hint. Agent surface - Analyst prompt rewritten as a short analytical method (grain, population, denominator; inspect rows before joining; derive in-engine; treat inferred labels as data) ending in a delimited `**Answer:**` line that may honestly be "unable to determine". The mandatory validate/analyze ritual is gone (an A/B showed no demonstrated score benefit). - Analyst can load skills and delegate to a new read-only `data-explorer` subagent (its only allowed `task` target; bash fully denied there). New built-in `data-question` skill with synthetic worked examples. Embedded built-in skills are no longer hidden by one old installed skill. - Reusing a `task_id` from a foreign session now starts a fresh child instead of adopting that session's permissions. `run` lifecycle and traces - `--max-turns` exhaustion gets exactly one best-effort finalization generation (exit code and reason unchanged); `--format json --output` writes the completed answer instead of a placeholder. - Traces cover descendant (subagent) sessions, reasoning text when the provider exposes it, and the effective system prompt and resolved tool schemas per generation (hash-deduplicated); resume no longer overwrites earlier child evidence. Truncation hints name a subagent the current agent can call. Tests: drivers 416, opencode suite green except `mcp.headers` (fails identically on `main`); live end-to-end analyst smoke suite (`scripts/analyst-smoke.sh`) 11/11. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Four test files mock `@altimateai/drivers/duckdb` at module load. Bun's `mock.module` leaks across files in the shared test process, so every DuckDB-backed test that ran later (scratch, sql_materialize, sql_check, schema_inspect value samples) saw a fake engine returning empty rows and failed only in full-suite runs. Each mocking file now snapshots the genuine exports before mocking and restores them in `afterAll`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…, provider list retries at startup
Three defects surfaced by the driver probe during evaluation runs and by load testing:
- The `duckdb` package locates its addon through `@mapbox/node-pre-gyp`, which
parses the HOST process argv and abbreviation-matches an unrelated `--dir`
flag to its `--directory` option, joining that value onto the absolute
manifest path. `altimate-code run --dir <x>` therefore failed every DuckDB
load with ENOENT under `<x>/usr/lib/...` (the 2026-08-31 root cause; never
merged). The driver now only resolves the module path in the parent and lets
the helper process, which has a clean argv, load the binding. Real-process
regression test with `--dir` in argv.
- `SHOW`, `DESCRIBE`, read-only `PRAGMA` and `EXPLAIN` of a read were classified
as writes ("other" category), so read-only agents were hard-denied from
listing tables on DuckDB/SQLite. They now classify as reads; `EXPLAIN ANALYZE`
of a write, `PRAGMA x = v`, `SET`, `USE` stay writes.
- `run` pre-validates an explicit `--model` against `sdk.provider.list()`; a
failed list call under startup load was read as an empty catalogue and
reported as "Model not found" (the cold-start race that cost evaluation runs in
August). It now retries three times and surfaces the real error.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A `ProviderError.HeaderTimeoutError` (the first-byte deadline, 10 s on the
OpenAI path) fell through `MessageV2.fromError` to `UnknownError`, which the
session never retries — one slow provider accept ended an otherwise healthy
run with `UnknownError: Provider response headers timed out after 10000ms`.
Classify it as a retryable `APIError` ("The model stopped responding: no
response headers within Ns"). Nothing was streamed for that attempt, so
re-issuing the request is safe; other `ResponseStreamError`s keep their
non-retryable handling. Wording and constants match PR #1426 so that branch
merges cleanly on top.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Trace audit of 51 evaluation runs (Opus 5.5 and gpt-5.6-sol, analyst agent)
surfaced general defects in the data-agent harness; none is specific to an
evaluation run.
- `sql_check`: strict-schema callers (OpenAI Responses API) send every
property and fill unused ones with "" / []; the strict zod schema rejected
49 of 56 calls. The published schema now tolerates empty values, which
`dropEmpty()` removes before the strict parse.
- `sql_materialize`: the row cap was not a memory bound — a >2M-row Postgres
result was fully buffered before the cap rejected it and the kernel
OOM-killed the process (8.3 GB RSS). Pre-flight `count(*)` + 1,000-row
sample now estimates size and refuses before extraction; `max_bytes`
(default 256 MiB, hard 2 GiB) joins `max_rows`; 0 means default.
- `sql_execute`: DDL/DML on the in-memory `scratch` database no longer asks
for `sql_execute_write` (read-only agents denied 8 `CREATE TABLE` calls
that `sql_materialize` was allowed to do anyway); the hard-deny list stays.
- `schema_inspect`: a blank `schema_name` means unset, and an empty column
list is reported as "table not found" instead of a valid empty schema
(21 of 44 Postgres inspections read as success).
- `run --output`: the file now holds the text of the last model step only;
commentary from earlier steps ("Let me check all formats.") leaked into 9
of 31 answers. Everything is kept as the fallback when no reply was given.
- `bin/altimate-code`: a child killed by a signal exited 0; now 128+signal
with a stderr line, so an OOM-killed run is not recorded as success.
- analyst prompt: the final message is the deliverable — no excluded
candidates or alternative answers, exact format when one is requested,
N concrete rows for a top-N with a boundary tie.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Traces show the model computing wrong numbers silently (integer division, averages of averages, fan-out after joins, arbitrary LIMIT) and never calling the separate lint tools. `sql_execute` and `sql_materialize` now run the altimate-core linter on the executed SQL (warehouse dialect, fresh cached schema when available) and append up to three `Check:` lines from a curated allowlist of correctness rules, Warning severity and above. Advisory only: errors are swallowed, a lint over 250 ms disables further lints for the session, `ALTIMATE_SQL_EXECUTE_LINT=0` turns it off. Fired rules are recorded in the tool metadata (`lint_rules`). The allowlist already names `integer_division`, `limit_without_order_by`, `avg_of_avg` and `reserved_word_alias`, which ship in the next altimate-core release. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`schema.inspect` defaulted `schema_name` to "public", which is Postgres's default
but not DuckDB's or SQLite's ("main"): every DuckDB inspection without an
explicit schema returned no columns, and an evaluation run read that as
"table not found" while the table was queryable. Without a schema the handler
now starts from the engine default and then searches the listed schemas for
the first that has the table; an explicit schema is used as given.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- `sql_execute`: the scratch exemption covered every write, including `COPY … TO`, `ATTACH`, `INSTALL`/`LOAD`, `EXPORT` and `SET`, which reach outside the in-memory engine. Only table/view DDL and row DML (`isScratchSafeWrite`) skip the write permission now. - `run --output`: the completion check and the post-compaction nudge ask for a bare `DONE`; that acknowledgement replaced the saved answer. The collector keeps the reply it confirms; substantive text still replaces it. - `sql_check`: a coverage check's `pattern: ""` is a value (match empty strings), not an omitted field. - header-timeout `APIError` message says "timed out" so run accounting keeps attributing an exhausted retry to a timeout. - `json-output` integration test expects the final-step-only `--output`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…chema size Review follow-ups on the v2 harness changes: - `schema.inspect`: derive the engine from the connection actually used when `warehouse` is omitted; try the engine default schema first and list schemas only on a miss, skipping system schemas and probing at most 20; value profiling targets the schema the handler resolved. - `sql_materialize` pre-flight: count through a `LIMIT max_rows + 1` derived table so an oversized source is rejected after bounded work, and skip the 1,000-row sample once the cap is known to be exceeded. - `sql-result-lint`: build the lint schema only from the tables the SQL references (via `extractMetadata`), capped at 2,000 columns; the synchronous lint is now ~20 ms on a 500,000-column cache instead of blocking the loop. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Mongo sources skipped the materialize pre-flight, so `find`/`aggregate` results were buffered up to the row cap before any check: an evaluation run on the articles collection was OOM-killed at 26 GB RSS. `find` now counts with `countDocuments` bounded by `limit: max_rows + 1`; `aggregate` appends `$limit`/`$count`; both refuse before extraction when over the row cap and otherwise sample 1,000 documents for the byte estimate, exactly like the SQL path. The driver passes `limit`/`skip` through to `countDocuments`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…requests
Traces from 270 Opus 5.5 evaluation runs showed:
- every `sql_execute` against a MongoDB warehouse was denied as a write
(52 of 52): the SQL classifier reads a JSON command document as a write.
Mongo warehouses now classify by command (`find`/`aggregate` without
`$out`/`$merge`/`countDocuments`/`distinct` are reads; drops are blocked),
and the native `{"find": "coll", …}` wire shape is accepted everywhere
(`normalizeMqlCommand` in the driver, used by `sql_execute` and
`sql_materialize`, which rejected it as command "undefined").
- one evaluation run stalled 890 s on a single Bedrock request: providers without a
header timeout now default to 300 s, and the idle-stream watchdog also
covers Bedrock's `application/vnd.amazon.eventstream` bodies (both as in
PR #1426).
- analyst prompt: every clause must bite; compute both readings of an
ambiguous phrase and choose by the question's own definitions.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Three of five sol trials on one question timed out: materializing a 3.3M-row
table into scratch took 116–552 s through 500-row VALUES batches, and when a
later `sql_check` hit the 120 s deadline the local-engine restart reopened the
`:memory:` scratch database empty — every scratch table vanished and the trial
redid the work.
- scratch is now a private per-process DuckDB file (0700 temp directory,
removed on close/exit; the path is redacted from errors), so a helper
restart after a timeout reopens the same catalog.
- `sql_materialize` writes extracted rows to a temp JSONL file and loads it
with `read_json(... columns={...})` using the inferred types: 1,000,000 rows
from SQLite 11.2 s → 2.3 s (bench script included).
- analyst prompt: move the smaller side of a cross-engine join — a few hundred
keys belong in an IN list, not a multi-million-row materialization.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ization `run` had a turn budget but no elapsed-time budget, so a session under an external kill (evaluation runs invoked as `timeout 900 altimate-code run …`) could explore until it was killed with no answer — one spawned 11 labeller subagents for 905 s with a defensible partial answer minutes earlier. - `--time-budget <seconds>` (env `ALTIMATE_RUN_TIME_BUDGET_S`), validated like `--max-turns`. - at 80 % of the budget one continuation directive asks the model to stop exploring and give its best current answer (`REPLY_BEFORE_TIME_BUDGET`), delivered at the next step boundary, once. - at 100 % the session is aborted like the turn budget and gets the same bounded finalization so `--output` holds whatever answer it can give; exit code and JSON events mirror the turn-budget semantics. Subagent time counts. - `time_budget_s`, `elapsed_s` and the deadline that fired are recorded in run accounting and the trace metadata. - `--output` now writes only the harness note when a run ends without a reply (earlier commentary is not an answer). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
An evaluation run died with `UnknownError: JSON.stringify cannot serialize BigInt.` on the model request following a DuckDB `COUNT(DISTINCT …)` tool result: the output-token estimator stringifies the outgoing messages before `llm.request` is published, and a native BigInt in a tool value reaches it unconverted. - `altimate/util/json.ts`: `bigintReplacer` and `normalizeBigInts` (copies JSON-shaped data, preserving SDK media objects and buffers). - `session/llm.ts`: normalize the outgoing messages before estimation and SDK serialization; the stored tool result is untouched. - tracing, run tracing and the TUI worker serialize with the BigInt replacer; a tracing failure can no longer surface as a session error. - regression tests at the processor boundary, tracing, and the sql_execute / sql_materialize tool results with BigInt values and a lint finding. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…rkers The strict marker check passes locally only because the local `upstream/dev` ref now points at the restructured opencode v2 tree, where none of the shared files exist at their v1 paths. Checked against the pinned `v1.18.34` tree (and with no upstream remote, as CI falls back to), `run.ts`, `run-accounting.ts` and `termination.ts` had unmarked additions. Comment-only change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A read-denied directory could still be searched: `grep`, `glob` and `ls` returned matching lines and paths from files the `read` rules deny (seen in an evaluation run, where a search surfaced another run's evaluation traces). Search results now pass through the same permission evaluation the read tool uses: denied paths are dropped silently, "ask" paths are batched into one prompt covering only the returned rows, and the 100-row cap applies after filtering (grep stops at 101 permitted matches or 10,000 scanned). Tool contexts carry the session's permission ruleset; `batch` forwards it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`sql_materialize` advertises results as `scratch.<table>`, so models call `schema_inspect` with `schema_name: "scratch"` (14 failures in one evaluation run) and get "cannot find a queryable table". When `schema_name` or the leading qualifier of `table` names a configured warehouse and no such schema exists in the current one, the lookup resolves against that warehouse's default schema and says so; an explicit `warehouse` argument still wins. Not-found errors now list the warehouses and the addressing form. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Evaluation runs lost complete work when a streaming response went idle for 400-700 s after the last tool call; the run's time budget then fired with no answer to salvage. The chunk watchdog that only covered Bedrock's eventstream now wraps every streaming body (`text/event-stream` included) and aborts a silent stream with a retryable `MODEL_STOPPED_RESPONDING` error, so the session's bounded retry re-issues the request. Default stays 300 s (`chunkTimeout`, PR #844); `ALTIMATE_STREAM_IDLE_TIMEOUT_MS` overrides it and `0` disables. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
Important Review skippedWe couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 Walkthrough
Merge Risk: 🟡 Moderate · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 41.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 178 functions across 85 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Title check The title clearly identifies the main change: improving analyst harness robustness for data-question runs. It is concise and related to the broad changeset. Description check The description includes all required template sections, explains the changes and rationale in detail, records verification results and known environment limitations, marks screenshots as not applicab…
✨ Finishing Touches 💡 2
-
📝 Generate docstrings 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
A rabbit checks the rows at dawn,
Then hops through scratch from dusk till morn.
With careful joins and deadlines near,
The traces keep each step quite clear.
One last small answer lands just right,
And bun paws close the work tonight.
Comment @coderabbitai help to get the list of available commands.
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b55879c97
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A review finding of the highest severity tier: the resolved `maxTimeMS` reached Mongo reads but not the nine write paths (CRUD, collection and index operations), so a write could run unbounded. Every write now forwards the deadline while preserving its command options; override, disabled and error cases are covered. Test: `forwards the default maxTimeMS and preserves command options` across the nine write variants. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
…-aware fingerprint call
- `duckdb-lock-helper.ts`: the foreign write lock is held by the driver's helper
process, a child of the holder script, not by the script itself. A bare SIGTERM
orphaned that helper with the lock still taken until it noticed its parent was
gone, so the next open in the suite collided with it on slow runners
("locked by another process" in `keeps overlapping connectors in independent
sessions` and the read-only sharing test). The script now closes its connector
on SIGTERM, which kills and reaps the helper before exiting; `release()` falls
back to SIGKILL after 10 s.
- `telemetry-signals.test.ts`: the source-order assertion matched
`computeSqlFingerprint(args.query)` literally; the call now passes the resolved
dialect, so match the query-argument prefix instead.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1955e0d8f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
…udget lowers reasoning
Always-thinking Claude models (Opus 4.7 and later, Fable 5) reject `thinking.type: "disabled"` with HTTP 400 ("use thinking.type.adaptive and output_config.effort"), so every budget-exhausted final request failed and the best-effort reply was lost.
- `applyBudgetEffort` now returns the provider's lowest adaptive variant for those models: direct Anthropic/Vertex/Gateway get `thinking: adaptive` plus `effort: low`; Bedrock gets `reasoningConfig: adaptive/low` and matching raw `thinking`/`output_config.effort` fields, dropping any stale token budget
- Claude 4.6 and older keep the disabled-thinking path
- `ProviderTransform.anthropicOmitsThinking` is exported as the detector
- tests: adaptive-only models on direct Anthropic and Bedrock keep adaptive/low and never emit `disabled`; the 4.6 expectations are unchanged
Live check on Bedrock Opus 5.5 with an 8 second budget: before, the finalizer request failed with the 400 and the run ended in error; after, the lowered-effort request succeeded and the run ended with a final reply.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
2 similar comments
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5641c87dc9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… the SAP provider during budget finalization The SAP `modelParams` branch of `applyBudgetEffort` still forced `thinking.type: "disabled"` for every Anthropic id, so Opus 4.7+ and Fable 5 served through `@jerome-benoit/sap-ai-provider-v2` got the same HTTP 400 on the budget-exhausted final request. The branch now keeps the lowest adaptive variant for those models and only disables thinking for Claude 4.6 and older. Tests: SAP Opus 4.7 keeps adaptive/low with unrelated model parameters intact; SAP Sonnet 4.6 still disables thinking. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 254a72b8-c27a-4ed4-a5ab-ce2a2f8ccae9) |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
…es lossless, and make absolute read restrictions authoritative in search tools - `sql_materialize`: `assertReadOnlySource` runs before the `sql_execute` ask and before the workspace redirect, so a write statement or Mongo write pipeline against a shadowed warehouse is rejected instead of redirected to the integration tool - Mongo driver: BSON `Binary` normalises to `base64:<bytes>` (subtype 4 to its canonical UUID string when `toUUID()` exists); schema inference reports `uuid`/`binary`; distinct same-length binaries no longer collide in scratch tables - search read approvals (`grep`, `glob`, `ls`): the final `read` ask carries a ruleset that re-applies absolute read rules after the relative/wildcard fallbacks, so `"/project/private/**": "ask"` prompts even with a trailing `"*": "allow"`; `Tool.Context.ask` accepts an optional `ruleset`, honoured by the prompt, session-tools and legacy adapters - `grep`: candidates are built from the lexical requested path, so a relative deny on a symlink alias applies while absolute rules still apply to the canonical target - tests: shadowed DELETE / Mongo write pipeline rejected before any ask or redirect; four Binary normalisation cases with real BSON objects; six absolute-ask cases across grep/glob/ls through the real permission service (reject and once); symlink alias deny vs real path Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 60442c36-69ef-4448-acf1-8acaa85fdf57) |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 14 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/opencode/src/tool/grep.ts">
<violation number="1" location="packages/opencode/src/tool/grep.ts:88">
P1: This checks only the lexical symlink path, so a relative deny on its resolved target (for example, `private/**`) is bypassed when grep searches through an allowed alias. Check both the requested path and `path.resolve(cwd, relative)`, rejecting the candidate if either is denied.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| const full = info?.type === "Directory" ? path.resolve(requested, relative) : requested | ||
| const readable = readableSearchResults([full], ins.worktree, ctx.permissions) | ||
| if (!readable.allowed.has(full)) return false |
There was a problem hiding this comment.
P1: This checks only the lexical symlink path, so a relative deny on its resolved target (for example, private/**) is bypassed when grep searches through an allowed alias. Check both the requested path and path.resolve(cwd, relative), rejecting the candidate if either is denied.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/opencode/src/tool/grep.ts, line 88:
<comment>This checks only the lexical symlink path, so a relative deny on its resolved target (for example, `private/**`) is bypassed when grep searches through an allowed alias. Check both the requested path and `path.resolve(cwd, relative)`, rejecting the candidate if either is denied.</comment>
<file context>
@@ -85,7 +85,7 @@ export const GrepTool = Tool.define(
filterPath: (relative) => {
scanned++
- const full = path.resolve(cwd, relative)
+ const full = info?.type === "Directory" ? path.resolve(requested, relative) : requested
const readable = readableSearchResults([full], ins.worktree, ctx.permissions)
if (!readable.allowed.has(full)) return false
</file context>
| const full = info?.type === "Directory" ? path.resolve(requested, relative) : requested | |
| const readable = readableSearchResults([full], ins.worktree, ctx.permissions) | |
| if (!readable.allowed.has(full)) return false | |
| const full = info?.type === "Directory" ? path.resolve(requested, relative) : requested | |
| const canonical = path.resolve(cwd, relative) | |
| const readable = readableSearchResults([full, canonical], ins.worktree, ctx.permissions) | |
| if (!readable.allowed.has(full) || !readable.allowed.has(canonical)) return false |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c168187fd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (binary.sub_type === 4 && typeof binary.toUUID === "function") { | ||
| return binary.toUUID().toString() | ||
| } | ||
| return `base64:${Buffer.from(binary.value()).toString("base64")}` |
There was a problem hiding this comment.
When a non-UUID BSON Binary contains bytes at or above 0x80, binary.value() returns an ISO-8859-1 string rather than raw bytes—the MongoDB BSON API specifies that passing true skips this conversion—so Buffer.from(...) re-encodes those characters as UTF-8 and changes the payload before base64 encoding. Consequently sql_execute and sql_materialize silently store incorrect binary keys and values. Fresh evidence beyond the earlier length-placeholder finding is that the replacement base64 branch still omits the raw flag; use binary.value(true) or the content slice of binary.buffer instead.
Useful? React with 👍 / 👎.
| } | ||
| // altimate_change start — time exhaustion during a silent reply still gets the bounded finalization | ||
| // At most an ordinary silent reply followed by one budget finalization. | ||
| for (let replyAttempt = 0; replyAttempt < 2 && !answered() && (assistantStarted || timeExhausted()); replyAttempt++) { |
There was a problem hiding this comment.
When the soft-budget interruption closes a partially emitted text part, answered() becomes true if that part belongs to the current step and no tool ran afterward. If the synthetic reminder then stalls before publishing another step-start and the hard deadline fires, this loop is skipped, so takeBudgetFinalizationAllowance() is never called: the run exits with the partial preamble instead of making the promised bounded best-effort finalization request. Hard-budget exhaustion should enter finalization regardless of whether interrupted work happened to emit text, or forcibly closed partial text must not satisfy answered().
Useful? React with 👍 / 👎.
| const query = mongo ? boundedMongo(sql, maxRows + 1) : boundedSql(sql, type, maxRows + 1) | ||
| const result = await executeSource(connection, input, query, maxRows + 1, maxBytes) | ||
|
|
||
| if (result.rows.length > maxRows || result.truncated) { |
There was a problem hiding this comment.
When a Mongo find command includes a positive limit and more documents match than that value, the Mongo driver fetches one lookahead row, returns exactly the requested number of rows, and sets truncated: true. This condition interprets that semantic lookahead as exceeding max_rows, so a query such as {"find":"events","limit":100} fails to materialize whenever the collection has more than 100 matches, even with the default two-million-row cap. Distinguish truncation at the command's explicit limit from truncation at the materialization safety cap.
Useful? React with 👍 / 👎.
|
|
||
| // A table with no columns does not exist: the handler reports an empty column list as | ||
| // success, which read as a valid empty schema (21 of 44 inspections in one evaluation run). | ||
| if ((schemaResult.columns ?? []).length === 0) { |
There was a problem hiding this comment.
For MongoDB, describeTable infers columns by sampling documents and returns an empty array for an existing collection containing no documents. This unconditional conversion of an empty column list into “Table was not found” therefore misreports valid empty collections and prevents callers from distinguishing an empty dataset from a misspelled collection. Confirm existence through listTables before producing the not-found error, or preserve an explicit existence signal from the dispatcher.
Useful? React with 👍 / 👎.
…r result before routing Moving the read-only validation ahead of the permission ask and workspace redirect made a refused statement throw out of `execute()`; the tool contract (and the existing `sql-materialize.test.ts` case) is an error result. The early refusal now returns the same `Materialize: ERROR` result shape, with no redirect annotation, whether or not the warehouse is shadowed. The routing tests assert the result instead of a throw. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 90a3c663-76c8-4707-9b66-f07e52c3bbb4) |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f5e9cf5b5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let permitted = 0 | ||
| const readPatterns = new Set<string>() | ||
| // altimate_change end | ||
| const result = yield* ripgrep.grep({ |
There was a problem hiding this comment.
When a matching file is governed by a read: ask rule, this launches ripgrep and collects the matching text before the later ctx.ask call at line 117. Rejecting the prompt prevents the result from being returned, but the protected file has already been read, unlike the regular read tool which requests permission before file access. Pre-authorize the search scope or otherwise obtain approval before ripgrep reads ask-scoped candidates.
Useful? React with 👍 / 👎.
| const response = await connector.execute(query, undefined, undefined, { | ||
| noLimit: true, | ||
| queryTimeoutMs: args.query_timeout_ms, | ||
| }) |
There was a problem hiding this comment.
When a PostgreSQL check source invokes a user-defined function that performs DML, readSql still classifies the outer SELECT as a read, but this execution omits the connector's readOnly option. A diagnostic sql_check can therefore mutate warehouse data despite requiring only read permission and advertising that it never writes; pass readOnly: true for PostgreSQL as the materialization path already does.
Useful? React with 👍 / 👎.
| connector.execute(params.sql, params.limit, undefined, { queryTimeoutMs: params.query_timeout_ms }), | ||
| // The driver enforces the deadline and cancels the work itself; this outer | ||
| // bound only covers engines that cannot, after allowing the driver to win. | ||
| deadline.ms === undefined || engineOwnsDeadline ? undefined : deadline.ms + HARD_DEADLINE_GRACE_MS, |
There was a problem hiding this comment.
When a remote connection or ALTIMATE_SQL_QUERY_TIMEOUT_MS specifies a value at or above the timer maximum, resolveQueryTimeout deliberately clamps it to 2_147_483_647, but adding the 5-second grace here pushes the delay past the signed 32-bit timer range. Node-compatible timers then reduce it to an immediate timeout, so a deliberately very long deadline abandons the query almost at once while it may continue running on the server. Clamp the sum to MAX_TIMER_MS before passing it to setTimeout.
Useful? React with 👍 / 👎.
| const ps = s.typed.get(def.type) | ||
| if (ps) yield* PubSub.publish(ps, payload) | ||
| // altimate_change start — typed trace subscribers are the only recipients of private events | ||
| if (def.private) return |
There was a problem hiding this comment.
When tracing sessions handled by the TUI worker or serve, their TraceConsumer subscribes only to the ordinary wildcard /event stream, while only the run command opens the new private trace-event channel. Returning here for LLMRequestEvent therefore prevents those persistent consumers from ever seeing the request snapshot, so their trace files omit the effective system prompt and resolved tool catalog. Add a typed private subscription for those consumers and route the event to logLLMRequest without exposing it on the public wildcard stream.
Useful? React with 👍 / 👎.
| ): Promise<any[]> { | ||
| if (!shared) throw new Error("DuckDB connection not open") | ||
| try { | ||
| return await shared.engine.call<any[]>("all", { sql, params, connectionId, maxResultBytes }, timeoutMs) |
There was a problem hiding this comment.
When two connectors share this engine and one connector's query times out, LocalEngine kills the helper process that owns both DuckDB sessions, but the next call from the other connector silently respawns it here. That peer loses temporary tables, settings, and any open transaction without receiving the stateLost timeout error; for example, its uncommitted work is rolled back and its next statement can run in autocommit before a later COMMIT fails. Track the shared engine generation and reject every affected connector once with an explicit state-loss error before executing more SQL.
Useful? React with 👍 / 👎.
This branch has not been deployed
1 participant
Issue for this PR
No GitHub issue; tracked internally (analyst harness robustness, Oct 2026).
Type of change
What does this PR do?
These 25 commits make analyst runs produce usable answers despite slow providers, large transfers and tool-contract failures, and close SQL read-permission bypasses and scratch/search isolation holes. There is no workload-specific logic anywhere in the branch. This description covers HEAD
07249b8d47.Problems described below were observed in evaluation traces of long-running analyst sessions; the internal trace analysis is available on request.
1. Provider and request robustness
Observed: a header timeout became a terminal
UnknownError, a stalled Bedrock request consumed a trial, and a BigInt tool result broke the next request (commitb27419790c). Some sessions also went silent early in an 860 s budget and stayed silent until the time-budget abort.Retry header timeouts as API errors; keep the header timeout opt-in for non-OpenAI providers. Extend the Bedrock-eventstream idle watchdog to streaming bodies from every provider: silence raises
MODEL_STOPPED_RESPONDING, with bounded retries only before that attempt emits any output. Retrying after text or tool calls could execute a tool twice;f5ea25b9aacloses that hole. Preserve the 300-secondchunkTimeoutdefault from PR #844;ALTIMATE_STREAM_IDLE_TIMEOUT_MSoverrides it, and 0 disables it (1679f90d6a). Normalize BigInts at request/trace boundaries, surface empty decode failures, and retry startup provider-list failures instead of reporting a missing model.Cap traced user/synthetic attachment text at 4,000 characters and serialize API errors without response headers or unbounded bodies (
f5ea25b9aa).2. SQL tools and scratch storage
Observed: 49/56
sql_checkcalls were rejected, 21/44 schema inspections returned empty success, scratch writes were denied, and 52/52 Mongo reads were classified as writes; large transfers exhausted memory or lost scratch tables after a timeout.run --dir; classify metadata statements as reads.E'\'--'; DELETEcould run as a read. Add the string-literal-awaresql-tokenize.ts, strip comments per parsed statement, and reject parser failures as reads.sql_check,sql_materializeand result lint require a single parser-reported query;sql_executeaccepts classifier-proven reads, including read metadata statements, but refuses multi-statement reads. Writing PRAGMAs require write permission (fec392c583).sql_checkinvariants for fan-out, key matching, grain, nulls and coverage; accept unused empty fields while preserving meaningful empty patterns. Resolve engine-default schemas, bound fallback probing, report missing tables, and include value samples.schema_inspect(schema_name="scratch")calls in one session, resolveschema_nameor the leading qualifier oftableto a configured warehouse's default schema when it names that warehouse and no such schema exists. Return a note, let explicitwarehousewin, and show the addressing form in not-found errors (4787af68e9).max_bytes(256 MiB default, 2 GiB hard cap). Accept native Mongo command documents and classify reads by command.c95ddd2633). Restore real DuckDB exports after test mocks to prevent suite-order failures.close()bypasses the query queue with a 2 s polite grace or 500 ms for an in-flight query. Decode rows as plain objects and BLOBs as Buffers; forward Mongocount/distinctcommandqueryfilters (32053eb019).3. Lint on execute
Observed: traces contained incorrect calculations and reserved aliases, while the analyst never called the separate check/lint tools (commit
7bd8be1c46).sql-result-lint.tsappends up to three advisory correctness findings tosql_execute/sql_materializeresults and recordslint_rules. It uses an allowlist, limits schema construction to referenced tables, and disables later session lints after a 250 ms overrun; synchronous core lint is not interruptible.ALTIMATE_SQL_EXECUTE_LINT=0disables it.The four rules
integer_division,limit_without_order_by,avg_of_avg, andreserved_word_aliasare allowlisted here but implemented in altimate-core-internal #773; TVF resolution is in #774. Those additions activate only when the corresponding core version ships.4.
runoutput and deadlinesObserved: nine answers contained intermediate commentary, a killed child exited successfully, and exploration continued until an external timeout with no answer. A later session stayed at reasoning effort high from its last visible reasoning to the 840 s abort, without an answer.
Save only the final model step to
--output, preserve the answer through a bareDONE, and write a harness note when no reply exists. Support JSON-mode answer files and one bounded finalization after budget exhaustion.--time-budget/ALTIMATE_RUN_TIME_BUDGET_Ssets a soft deadline at 80% and a hard abort at 100%, then allows one finalization generation bounded to 30 seconds; subagent time counts.At the soft deadline, abort the in-flight model request after any executing tool persists its result, retain emitted text/reasoning, and make the answer-now nudge the next turn without a provider-error retry. Request the provider's lowest reasoning effort for the rest of the run, including subagents and finalization, without changing the configured variant.
ALTIMATE_RUN_BUDGET_EFFORT_DOWNGRADE=0disables the downgrade, but keeps the interrupt (2fe68a47f6); the remainingnonevslowquestion is recorded below. The lowest effort is provider-specific: always-thinking Claude models (Opus 4.7 and later, Fable 5) rejectthinking.type: "disabled"with HTTP 400 ("use thinking.type.adaptive and output_config.effort"), which made every budget-exhausted final request fail and lose the best-effort reply; those models now keep adaptive thinking at effortlow(direct Anthropic, Vertex and Gateway:thinking: adaptive+effort: low; Bedrock:reasoningConfigadaptive/low with matching rawthinkingandoutput_config.effortfields, stale token budgets dropped), while Claude 4.6 and older keep disabled thinking (5641c87dc9). Live check on Bedrock Opus 5.5 with an 8 second budget: before, the finalizer request failed with the 400 and the run ended in error; after, the lowered-effort request succeeded and the run ended with a final reply.Exit 0 when finalization writes an answer, with the budget abort reported as a warning; silent finalization, hard failures and Ctrl-C remain non-zero. Signal exits become
128 + signal. Add Unix-ms trace fieldsgeneration_interrupted_at/effort_downgraded_at; runs without a time budget are unaffected. Update the help snapshot for--time-budget(f5ea25b9aa).5. Analyst method and inspectable runs
Observed: prompts conflicted with answer-only requests, and review of full traces exposed population, weighting and interpretation errors.
Require the requested final format, N concrete top-N rows, explicit population/grain/denominator, both readings of ambiguous clauses, and smaller-side cross-engine transfers. Add the built-in
data-questionskill and read-onlydata-explorer, expose embedded skills missing from partial installs, and reject reuse of foreign-session permissions. Trace descendant sessions, reasoning, effective prompts and tool schemas, retaining earlier evidence on resume.6. Search-result permissions
Observed: one session's
grep(path=/opt)surfaced another session's traces althoughreadof that directory was denied. This exposed a general isolation hole.Filter
grep/glob/lsresults through the read-permission rules: drop denied rows and issue one batched ask prompt for returned rows only. Apply the result cap after filtering; grep stops at 101 permitted results or 10,000 scanned. Tool contexts carry the ruleset andbatchforwards it (e987fe76dc). Evaluate worktree-relative paths exactly likeread; absolute rules may only tighten the result to deny or ask, never grant access (c95ddd2633).7. Review follow-ups
Commit
07249b8d47fixes review-bot findings in the two highest severity tiers, with regression coverage:data-explorerread-only permission ceiling after agent or global overrides, and bind resumed tasks to the agent that created them.READ ONLYtransaction and roll back; refuse Mongo$out/$mergeanywhere in a pipeline, including nested pipelines. Require approval for attached-catalog writes and refuse unboundeddistinct.Commits
a0d8f29962and3c168187fdfix the later review-bot findings in the same tiers:sql_materializevalidates the source statement as read-only before thesql_executeask and before the workspace redirect, so a write statement or Mongo write pipeline against a shadowed warehouse is rejected rather than redirected to the integration tool.Binarylosslessly (base64:<bytes>, subtype 4 as its canonical UUID string) instead of theBinary(<length>)label, so distinct same-length binaries no longer collide in scratch tables; schema inference reportsuuid/binary.readapproval fromgrep/glob/lsre-applies absolute read rules after the relative and wildcard fallbacks, so an absoluteaskrule prompts even with a trailing"*": "allow";Tool.Context.askaccepts an optionalruleset, honoured by the prompt, session-tools and legacy adapters (only ever built from the caller's own merged ruleset).grepbuilds candidates from the lexical requested path, so a relativereaddeny on a symlink alias applies while absolute rules still apply to the canonical target.modelParamsbranch had still forceddisabled).Lower-severity findings are triaged in the PR for follow-up.
How did you verify your code works?
Recorded verification by the Engineer runner outside the Codex sandbox; no tests or builds were run for this draft refresh:
32053eb019, opencodebun test test/cli/help test/altimate test/tool test/session test/provider test/release-validation test/cli/run: 11,471 pass, 669 skip, 1 fail. The sole failure wasPostgreSQL Driver E2E, which needs a live Postgres. Drivers: 444 pass, 0 fail. Typecheck clean. This broad run predates the final budget commit.2fe68a47f6,bun test test/cli/run test/session test/provider test/altimate/tools: 2,762 pass, 26 skip, 0 fail. Typecheck andgit diff --checkclean; markers balanced.07249b8d47, opencodebun test test/cli test/altimate test/tool test/session test/provider test/release-validation test/bus test/server test/agent test/e2e-smokeran outside the sandbox: 12,727 pass, 682 skip, 4 fail. The four failures also fail onmainor need a live PostgreSQL (experimental HttpApi,mcp HttpApi,PostgreSQL Driver E2E). The trace-viewer regression was fixed before commit; re-run: 1,993 pass, 0 new failures.07249b8d47, drivers: 463 pass, 0 fail. Typecheck andgit diff --checkclean; markers balanced.engineer reviewand Sonnet, preceded each commit batch. Residuals: an E2E trace assertion for the new budget fields, an OpenAI-native low-effort request assertion, and resolvingnonevslowas the lowest effort.Earlier file-by-file runs of
bun test <file>from each package directory: new opencode altimate tests 323 pass, 0 fail across 22 files; new driver tests 57 pass, 3 sandbox-only failures across four files. The table preserves those earlier results;local-engine.test.tssubsequently passed 34/34 outside the sandbox. Neither run is a claim about the entire repository suite.New test files covered by that earlier run; paths below are relative to
packages/<package>/test/:Also added under
packages/opencode/test/, outside that earlier file-by-file run:agent/analyst-data-explorer.test.ts,cli/cmd/run-output.test.ts,cli/run/budget-finalization.test.ts,cli/run/json-output.test.ts,cli/run/run-tracing.test.ts,cli/run/time-budget.test.ts,e2e-smoke/analyst-smoke.test.ts,skill/builtin-discovery.test.ts.pgrepreported "Cannot get process list" andpswas denied by the sandbox. The subsequent 34/34 pass outside the sandbox confirms the sandbox-only cause.bun test test/altimate/tools/datamate-list-integrations.test.ts: 1 pass, 2 fail, 2 errors, exit 1. Recorded as a pre-existing failure on main; main was not rerun here.9aa62046a2adds comment-onlyaltimate_changewrappers torun.ts,run-accounting.ts, andtermination.ts. The local check falsely passed whenupstream/devwas the v2 tree; verification used tagv1.18.34and a clone without the upstream remote, exercising CI's fallback. The prior v8 count was 324 starts / 324 ends across 19 touched files; current HEAD is confirmed balanced above.Screenshots / recordings
N/A.
Checklist
🤖 Generated with Claude Code
Note
High Risk
Changes core SQL/Mongo/DuckDB/SQLite execution paths (timeouts, subprocess kills, read-only PG, write-stage blocking) and grep result filtering—mistakes could leak data, drop results, or corrupt sessions.
Overview
Adds a built-in
data-questionOpenCode skill that documents inspect → plan → compute → verify → answer workflows for SQL/Mongo analysis, including cross-warehousesql_materializebridges and spill handling.Local database drivers move DuckDB and SQLite into a supervised Bun helper process (
LocalEngine) so queries can be killed on deadline, enforcemaxResultBytes, and avoid loading DuckDB in the host whenargvcontains--dir. DuckDB gains shared file engines, sandbox/secret isolation, spill cleanup, and optional persistent secrets outside scratch helpers.Cross-driver policy: shared
query-timeout(default 120s) andQueryTimeoutError, plus streaming result byte budgets on PostgreSQL (optionalREAD ONLYtransactions), MongoDB (maxTimeMS, cursor budgets, native command normalization, nested$out/$mergedetection, lossless binary/UUID serialization), and exports for Mongo helpers.Ripgrep caps retained line text at 2k chars and supports
filterPath/scanLimitso matches can be dropped before collection without unbounded scans.package.jsonpins@smithy/is-array-bufferand@smithy/util-buffer-fromto 4.2.2.Reviewed by Cursor Bugbot for commit 1f5e9cf. Bugbot is set up for automated code reviews on this repo. Configure here.