From 3cf45172c03b9ae63d1e132312aa83af7c736601 Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 19 Oct 2026 00:15:55 +0530 Subject: [PATCH 01/11] =?UTF-8?q?fix:=20Claude=20Code=20plugin=20path=20to?= =?UTF-8?q?=20workspaces=20=E2=80=94=20headless=20`run`=20hang,=20first-ru?= =?UTF-8?q?n=20skills,=20unsent=20memory?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gaps found walking the path from the Altimate Code plugin in Claude Code to a workspace, end to end against a real tenant: - `run` waited forever on a stdin it was handed but never sent anything on. Claude Code's Bash tool (and a Node `spawn` with default stdio) leaves stdin open, so every delegated task hung before it started. `readRunStdin` reads a file in full and any other stdin only if its first chunk arrives within 3s. - The first skill added to a linked workspace that had no skills missed the next `run`: no snapshot existed, so the turn did not wait for the sync and the skill tool could not load a skill the workspace prompt named. The per-turn gate now also waits when the empty-workspace record exists (`emptyRecordPath`, moved out of `skill-sync` so the gate need not load it). - A memory upload that failed in `run` (the first create can outlast the 15s request timeout) stayed on the machine with no word to the user. `run` now ends with a line saying so and pointing at `altimate-code workspace sync`. - `link` and `workspace` name the sign-in command (`altimate-code auth login -p altimate-backend`) instead of "run the TUI and sign in", and the non-TTY `link` error names `--workspace` / `--create`. - Docs: the Claude Code guide covers the plugin and using a workspace from Claude Code (sign in, link, `/altimate-code:altimate`). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011chv9Rmu7jQXokum1WNcs7 --- .../deterministic-validators-followups.md | 682 ------------------ .github/meta/harness-review-followups.md | 65 -- .../meta/night-run/MERGE-VALIDATION-MATRIX.md | 47 -- .../night-run/RELEASE-EVALUATION-BOARD.md | 45 -- .github/meta/night-run/UPGRADE-PATH-TEST.md | 34 - .../guides/using-with-claude-code.md | 58 +- .../opencode/src/altimate/cli/run-stdin.ts | 62 ++ .../altimate/workspace/empty-record-path.ts | 12 + .../src/altimate/workspace/memory-sync.ts | 21 +- .../src/altimate/workspace/skill-sync.ts | 15 +- packages/opencode/src/cli/cmd/link.ts | 16 +- packages/opencode/src/cli/cmd/run.ts | 24 +- packages/opencode/src/cli/cmd/workspace.ts | 4 +- packages/opencode/src/session/prompt.ts | 21 +- .../altimate/workspace/memory-sync.test.ts | 17 + .../opencode/test/cli/run/run-stdin.test.ts | 62 ++ packages/opencode/test/session/prompt.test.ts | 65 ++ 17 files changed, 348 insertions(+), 902 deletions(-) delete mode 100644 .github/meta/deterministic-validators-followups.md delete mode 100644 .github/meta/harness-review-followups.md delete mode 100644 .github/meta/night-run/MERGE-VALIDATION-MATRIX.md delete mode 100644 .github/meta/night-run/RELEASE-EVALUATION-BOARD.md delete mode 100644 .github/meta/night-run/UPGRADE-PATH-TEST.md create mode 100644 packages/opencode/src/altimate/cli/run-stdin.ts create mode 100644 packages/opencode/src/altimate/workspace/empty-record-path.ts create mode 100644 packages/opencode/test/cli/run/run-stdin.test.ts diff --git a/.github/meta/deterministic-validators-followups.md b/.github/meta/deterministic-validators-followups.md deleted file mode 100644 index c06572eea5..0000000000 --- a/.github/meta/deterministic-validators-followups.md +++ /dev/null @@ -1,682 +0,0 @@ -# Completion-gate validators — deferred review findings - -Findings from the review of the deterministic completion-gate validators that are -real but larger than the change they were raised against, plus the ones that were -declined on purpose. Each carries the rationale, so a later pass does not have to -re-derive it. - -Source: review feedback and the end-to-end evidence run on -`feat/deterministic-validators` (2026-08-29). - ---- - -## Deferred — real, but larger than a fix-in-place - -### 1. ~~Custom `model-paths` / `seed-paths` are not honoured~~ — RESOLVED - -`modelsModifiedSince` requires a `models` path segment, and both -`collectProducedNodeNames` and the authored-file scan use a hard-coded directory -list. A project that configures `model-paths: ['analytics']` in `dbt_project.yml` -is invisible to every path-based check in the lane. - -Direction is safe today — the validators under-fire rather than over-fire on such -a project — but the deliverable-names gate can report a name as absent when the -model exists under a custom path, which would block. - -Closed in the consensus pass. `resolveDbtSourcePaths(dbtRoot)` parses -`model-paths`, `seed-paths`, `snapshot-paths`, `analysis-paths`, `macro-paths`, -`test-paths` and `packages-install-path` (plus the pre-1.0 `source-paths` / -`data-paths` spellings), handling inline flow lists, block sequences and bare -scalars. `modelsModifiedSince`, `collectProducedNodeNames` and the authored-file -scan are all driven off it. - -One deliberate carve-out: when the scanned directory has no `dbt_project.yml` -there is nothing to honour, so `modelsModifiedSince` keeps its legacy "any -`models` ancestor" predicate. That keeps the helper usable on a directory that -is not itself a project root. - -### 2. Python models (`.py`) are outside the touched-model set - -`modelsModifiedSince` accepts `.sql` only, so a session that edits -`models/orders.py` produces an empty work list and `dbt-build-green` takes its -`nothing-to-gate` path. - -Why deferred: widening the extension is one line, but the consumers are not -extension-agnostic. `dbt-dialect-guard` and `dbt-incremental-config` would then -run SQL/Jinja regexes over Python source, where `#` is a comment and -`config(materialized=...)` is a `dbt.config()` call — different lexical rules -entirely. The correct shape is a per-consumer file-kind filter, which is a -refactor of the discovery API rather than an added extension. - -### 3. `run_results.json` is trusted as evidence an agent cannot forge - -Nothing stops a session writing a `run_results.json` full of `success` rows -instead of running dbt. Every filesystem-evidence gate in this lane shares that -property. - -Why deferred: closing it means recording dbt invocations from the tool layer and -signing them into the session record — a lane-wide trust model, not a validator -change. Partly mitigated already: build coverage now also reads the model DDL -under `/run/`, so a forgery has to fabricate two artifacts rather than -one. - -### 4. Post-build edit detection is mtime-based, not content-based - -`BUILD_FRESHNESS_TOLERANCE_MS` was raised to 60 s because a formatter or a -trailing-newline fix landing seconds after a green build was blocking sessions. -That trades a false positive for a blind spot: a substantive rewrite inside the -window is not caught. - -The right fix is a content comparison — hash each model at build time and compare -after — which needs a pre-build snapshot the gate does not currently take. Worth -doing when the lane gains a session-scoped artifact store. - -### 5. ~~Compound `{% if is_incremental() and … %}` conditions are not matched~~ — RESOLVED - -Closed in the review sweep. Guard-body extraction was rebuilt on the shared -nesting-aware helper (`extractJinjaIfBlocks` + `jinjaIfBranchHead`) rather than on -a looser regex, which is what this entry said the fix had to wait for. Compound -conditions and nested `{% if %}` / `{% else %}` inside a guard are now handled. -Covered by `review-sweep.test.ts`. - -### 6. ~~`analyses/` counts toward the produced-node inventory~~ — PARTLY RESOLVED - -An `analyses/foo.sql` satisfies a required model named `foo`, even though an -analysis is never materialised as a relation. The requested resource *type* is -also discarded during extraction, so a seed can satisfy a request for a model. - -Half closed in the consensus pass. `analyses/` and `tests/` no longer contribute -to the inventory from either source: the filesystem scan visits only the -configured model/seed/snapshot paths, and manifest nodes are filtered on -`resource_type`. An analysis is compiled but never materialised, so it was never -a relation that could satisfy a deliverable. - -Still open: the requested resource *type* is discarded during extraction, so a -seed can still satisfy a request for a model and vice versa. That needs the noun -carried from the task through to the comparison, changing the -`RequiredDeliverables` shape and the gate's messages. - -### 7. Five copies of the recursive project walker - -`modelsModifiedSince`, `collectProducedNodeNames`, `collectExecutedModelNames`, -`anyAuthoredFileSince` and `projectPrescribesGuards` each carry their own -recurse / skip-hidden / skip-`node_modules` / symlink / depth-cap loop. They have -already drifted (only two follow symlinks; only some skip `target`). - -Why deferred: a shared `walkProject(root, opts)` is a clean refactor but touches -every validator in the lane at once, and doing it in the same change as the -behavioural fixes would make both harder to review. Worth its own change. - ---- - -## Declined — the conservative behaviour is the intended one - -### `IDENTIFIER_RE` requires at least three characters - -Reviewers asked for identifiers of any length so a task requiring `id` is -honoured. Declined: two-character code spans in prose are overwhelmingly not -relation names, and every one that is wrongly accepted becomes a required model -that can never be satisfied. Under-extraction is a miss; over-extraction blocks a -correct session. - -### `hasGuard` accepts any `is_incremental()` occurrence - -Reviewers asked that it require an enclosing `{% if %}`. Declined: a model that -writes `{% set inc = is_incremental() %}{% if inc %}` is correct dbt, and -tightening this creates a new false positive to close a false negative. The -lenient direction is the safe one for a gate that blocks completion. - -### A fresh test-only artifact should hard-fail rather than skip coverage - -Reviewers asked that an artifact containing no model nodes block an edited model. -Declined as stated: `dbt build` followed by `dbt test` is a normal, correct -sequence and leaves exactly that artifact, so blocking on it fires on healthy -sessions. Addressed instead by reading the model DDL under `/run/`, which -a test invocation does not overwrite, and by recording -`verdict: "coverage-inconclusive"` when neither source can speak — so the case is -visible in telemetry rather than silently green. - -### Backslash string escapes in the SQL lexer - -A reviewer asked that `scrubSql` stop treating `\'` as an escaped quote, on the -grounds that none of the target warehouses use backslash as a string-escape -character. Declined: the premise is wrong. Snowflake, BigQuery and Redshift all -support backslash escape sequences in string literals; only DuckDB is -strictly `''`-only. Dropping the branch would mis-lex `'it\'s'` on three of the -four warehouses this lane targets, which is the more common shape than the -literal-trailing-backslash case the reviewer raised. - -### `unique_key` inherited from `dbt_project.yml` - -Full dbt config inheritance is not resolved. Rather than guess, the keyless-upsert -finding is suppressed for the whole project when `dbt_project.yml` mentions -`unique_key` at all. Deliberately blunt: it gives up a true positive in exchange -for never inventing an inconsistency that the merged config does not have. - ---- - -## Deferred — raised in the review sweep, still open - -### 8. `/run/` DDL proves execution, not success - -Build coverage falls back to the model DDL under `/run/` when -`run_results.json` has been overwritten by a later `dbt test`. dbt writes that -DDL *before* the warehouse executes the statement, so it is present for a model -that then failed. A session that runs `dbt build` (a model errors), then -`dbt test`, leaves a failed model with fresh DDL and no failing row in the -surviving artifact, and the gate reports green. - -Why deferred: the obvious narrowing — trust the DDL only when the fresh artifact -carries no model rows at all — breaks the very common `dbt run --select a` then -`dbt run --select b` session, where `a` is covered by DDL alone and the artifact -does carry model rows. That would block healthy sessions, which is the wrong -direction. The real fix is retaining per-invocation run-result history for the -session rather than reading whichever single artifact survived, which is the same -session-scoped artifact store that item 4 needs. - -Partially mitigated in the sweep: staleness is now measured against the DDL's own -mtime rather than the surviving artifact's, so an edit made between the build and -a later test is caught. - -Further mitigated in the consensus pass, and the reasoning above is confirmed -against real dbt 1.8.7: a `dbt build` that errors on a model still leaves that -model's DDL in `/run/`, and a following `dbt test` writes -`run_results.json` with `args.which: "test"` and zero rows. The two evidence -sources are then indistinguishable from a healthy `dbt run` + `dbt test`, so -this still cannot be made to block without firing on correct sessions. - -What did change is the label. Coverage resting on DDL alone now reports -`verdict: "build-unproven"` instead of `fresh-build`. That matters more than it -sounds: the confident verdict was contaminating the shadow telemetry the enable -decision is supposed to rest on, so red builds were being counted as green in -the measurement itself. The gate's pass/fail behaviour is unchanged. - -Closing it properly still needs per-invocation run-result history for the -session — the same session-scoped artifact store item 4 needs. Note the harness -makes this worse on its own: `dbt-tests-pass` spawns `altimate-dbt test` in the -project on every validation pass, so from the second pass onward the surviving -artifact is always a test artifact. - -### 9. Build coverage is keyed on the bare node name, not the package - -In a multi-package project where a dependency and the root project both define -`orders`, a successful `model.dependency.orders` row satisfies coverage for the -local `model.local.orders`. - -Why deferred: matching on the full unique ID means mapping each touched file to -its manifest node via `original_file_path`, which is a new manifest-backed -resolution step rather than a line edit. Much narrower after the sweep: installed -packages are now excluded from the touched-model set, so this needs a genuine -name collision between the root project and a dependency, both selected in the -same session. - -### 10. The dialect guard checks for a guard, not for the *right* guard - -`dbt-dialect-guard` suppresses a warehouse-specific call when it sits anywhere -inside a `{% if … target.type … %}` chain. It does not check that the branch the -call sits in is actually limited to a warehouse that provides the function, so -both of these pass while still breaking on at least one target: - -```jinja -{% if target.type != 'snowflake' %} {{ iff(a, b, c) }} {% endif %} -{% if target.type == 'snowflake' %} … {% else %} safe_cast(x as int) {% endif %} -``` - -Why deferred: closing it needs a mapping from each `DIALECT_FUNCTIONS` entry to -the `target.type` values that provide it, plus evaluation of each branch -condition against that set — `==`, `!=`, `in`, `not in`, and the implicit -complement an `{% else %}` arm carries. That is a feature with its own test -surface, and a half-implementation converts a false negative into a blocking -false positive on correct models. The validator's docstring states the weaker -property it actually checks, so the claim is not overstated in the meantime. - ---- - -## Consensus review (six reviewers, 2026-08-29) — closed in this pass - -Recorded here so a later pass does not re-derive them. Each has a regression -test in `test/altimate/validators/consensus-review.test.ts`. - -- **`dbt compile` artifacts certified builds.** `run_results.json` carries no - statement of which subcommand wrote it, and `dbt compile` emits a full set of - `status: "success"` model rows — verified against real dbt 1.8.7, including - for a model selecting from a non-existent relation. `readRunResults` now reads - `args.which`, and an artifact from a command that executes no model SQL is not - build evidence. An artifact with no `args.which` is still trusted, so the - change is backwards compatible; real dbt always stamps it. -- **`CONFIG_CALL_RE` truncated at the first `)`.** `pre_hook="{{ log_start(run_id) }}"` - ended the non-greedy capture early and silently dropped every argument after - it — reading a correctly-keyed merge model as an unkeyed upsert, and losing - `enabled=false` / `materialized='ephemeral'` exemptions. Replaced with a - paren-depth and quote aware scanner. -- **`dbt-nothing-built` passed on any unrelated edit.** It asked only whether - *anything* had been written under the project, never comparing against - `expectation.required`. A session told to create `fct_orders` cleared it by - touching `macros/helper.sql`. Evidence must now intersect the named - deliverables; the coarse bar is kept only for the opt-in with no named - deliverables. -- **`packages-install-path` was ignored.** The `dbt_packages` / `dbt_modules` - skip was matched on the bare directory name, so a project configuring the - install path sent dependency models through the two *pre-existing* subprocess - validators, and a locally authored directory of that name anywhere in the tree - was wrongly skipped. Now resolved from project config and matched on path. -- **Inactive Jinja exempted live models.** `{% if false %}{{ config(enabled=false) }}` - and the same inside `{% raw %}` read as real exemptions. Both regions are - blanked before config extraction. Limited to these two on purpose — a looser - condition would strip live config and push the gate towards blocking. -- **`insert_overwrite` / `microbatch` were told to add a guard.** Both converge - on re-run by construction, and the prescribed remediation would have changed - what the model does. Now exempt from the guard requirement. -- **`is_incremental()` inside a string literal counted as a guard.** The scan - tested the unmasked source while the file's own comment said otherwise. -- **`[^%]*` in `ownBranchMatches` / `jinjaIfBranchHead`.** A Jinja modulo in a - tag (`{% if loop.index % 2 == 0 %}`) stopped the match dead and lost an arm - from the depth counter. Aligned with `JINJA_IF_OPENER_SOURCE`. -- **Repository text was spliced into the retry prompt.** dbt error messages and - node names were interpolated verbatim into `reason`/`fixHint`, which dispatch - concatenates into a synthetic `role: "user"` turn — so a hostile repo could - place text at instruction position. All untrusted values now go through - `sanitizeForPrompt`. -- **Verdicts that hid a zero-verification pass.** An empty scope and an - exempt-only scope both reported `fresh-build`. Now `nothing-verified` and - `exempt-only`; DDL-only coverage is `build-unproven`; an edit inside the 60 s - grace window is reported in `edited_within_grace` and downgrades the verdict. -- **Unreadable models read as clean.** `dbt-dialect-guard` and - `dbt-incremental-config` now report `models_scanned`, `unreadable_models` and - `coverage_complete` rather than counting an unreadable file as verified. -- **`prompt.ts` claimed shadow mode spawns no subprocesses.** It does. Shadow - suppresses only the retry; every validator runs in full. Comment corrected — - the false claim was load-bearing for "enable in shadow first" advice. - -## Consensus review — open, NOT addressed in this pass - -Ordered by how much they should weigh on an enable decision. - -### 11. Node identity is a bare lowercase name, not a `unique_id` - -Supersedes and widens item 9. Run-result IDs and `/run/` paths are both -reduced to a bare name, discarding package *and* resource type, so -`model.dependency.orders` can satisfy coverage for a local `orders`, and a -snapshot's DDL can stand in for a model's. The honest fix is to resolve each -touched file to its manifest `unique_id` via `original_file_path` and match on -that — a manifest-backed resolution step the lane does not have yet. This is the -single largest remaining source of fabricated coverage. - -### 12. Effective dbt config is never resolved - -`dbt-incremental-config` only sees inline `config()`. A model made incremental -through `dbt_project.yml` or properties YAML is invisible, while any textual -`unique_key:` anywhere in `dbt_project.yml` suppresses the keyless-upsert -finding for every model in the project. dbt supports `unique_key` in SQL, -properties YAML and project config. Needs the effective config for the exact -node from a fresh manifest. - -### 13. `dbt-deliverable-names` mines natural language for a blocking contract - -Assessed by two reviewers as the validator most likely to block correct work. -Code-formatted column names in a Required/Deliverables section become required -models; rename wording can demand both names; negated bullets can demand the -prohibited artifact; `create a model with name fct_orders` is not recognised. -Both contract gates also stop at the first task document that parses any -contract, silently masking a later `REQUIREMENTS.md`. The direction of travel is -an explicit structured contract rather than prose mining, with ambiguous prose -yielding inconclusive instead of a blocking invented requirement. - -### 14. Dialect guard does not check which target owns the function - -Restates item 10, still open, now with a second failure mode: the scrubber does -not handle dollar-quoted strings, so valid text such as `$tag$… iff( …$tag$` -produces a blocking finding. - -### 15. Python models are still outside the touched set - -Item 2, unchanged. Now paired with the `model-paths` fix: discovery honours -custom paths but still accepts `.sql` only, so a `models/orders.py` session takes -the `nothing-to-gate` path. Needs a per-consumer file-kind filter, because the -SQL/Jinja regexes are wrong for Python source. - -### 16. Validator telemetry has no field allowlist - -`details` is attached to telemetry wholesale and includes absolute `dbt_root`, -`task_file` and `run_results_path` plus business model names, against a -telemetry contract that says file paths are not collected. Pre-existing for -`dbt_root`; this lane widens it. Wants an allowlist of bounded counters, -booleans, enum verdicts and durations, with paths dropped or hashed. - -### 17. No whole-check resource budget - -Recursive scanners follow directory symlinks without realpath containment or -cycle detection, several Jinja helpers are near-quadratic on pathological input, -and no deadline is passed through the registry. The 60 s timeout covers -`altimate-dbt` children only, not the in-process checks. - -### 18. Retry budget is shared, and the ceiling is wall-clock, not just waste - -`validatorRetryCount` is one session-scoped counter capped at 3, and the five -new validators register *before* `dbt-schema-verify` / `dbt-tests-pass`. See the -PR discussion for the analysis; not changed here because the counter lives in -`session/prompt.ts`, which this PR otherwise does not touch. The associated -number worth carrying: the two subprocess validators run one `altimate-dbt` -child per touched model at concurrency 4 with a 60 s per-child timeout, across -an initial dispatch plus up to three retries — roughly `480 × ceil(M/4)` seconds -worst case, about 3 h 20 m at 100 touched models. - ---- - -## Second review sweep (2026-08-31) — closed in this pass - -Each has a regression test in `test/altimate/validators/review-sweep-2.test.ts` -that fails against the pre-fix source. - -- **Seed and snapshot builds read as "nothing built".** Narrowing - `MODEL_EXECUTING_DBT_COMMANDS` to `{run, build}` was right for - `dbt-build-green` — a `dbt seed` says nothing about whether an edited model - compiles — but `dbt-nothing-built` reused the same predicate to answer a - different question. A successful `dbt seed` of exactly the required name then - read as no build at all, and the gate blocked a session that had delivered - what it was asked for. Split into `runResultsProducedNodes`. The two must not - be collapsed again: model *coverage* and *deliverable* evidence are different - claims. -- **`data/` was a default seed path.** dbt's default is `seeds/` alone; - `data-paths` is only the pre-1.0 spelling. Listing `data/` unconditionally - made `data/orders.csv` read as a produced, authored node in a project where - dbt will never load it, so both contract gates could accept a required - `orders` deliverable with nothing built. The legacy key is still honoured when - the project actually sets it. -- **A contradictory source config granted an exemption.** A model whose live - config states both `enabled=false` and `enabled=true` satisfied - `sourceExemptsFromRunResults`, which drops it out of scope entirely — so even - a fresh `error` row for it was filed as out-of-scope and the build gate - reported green having checked nothing. Each axis now requires that the source - does not also declare its opposite. -- **The dead-`if` strip blanked live `else` arms.** `stripInactiveJinja` blanked - a `{% if false %}` chain through its `{% endif %}`, taking real `config()` in - the `{% elif %}` / `{% else %}` arms with it — an ephemeral or disabled model - lost its exemption and the build gate demanded a `run_results` row dbt never - writes. Only the constant-false arm is blanked now, depth-counted so a nested - `if` inside it cannot steal the outer `else`. -- **The dialect-guard convention probe still carried `[^%]*`.** The modulo fix - applied to `ownBranchMatches` and `JINJA_IF_OPENER_SOURCE` never reached - `TARGET_TYPE_GUARD_PROBE_RE`, so `{% if n % 2 == 0 and target.type == … %}` - left the project's only guard unrecognised, `appliesTo` returned false, and - the validator silently switched itself off for the whole session. -- **The convention probe scanned hard-coded `models/` and `macros/`.** A project - on `model-paths: ['transform']` or `macro-paths: ['jinja']` found no - convention and disabled the lint, even though the touched-model scan already - honours custom paths. Driven off `resolveDbtSourcePaths` now, matching it. -- **The incremental predicate started at a subquery's `where`.** A clause - keyword anywhere in the arm outranked a leading `and`/`or`, so - `and ts > … (select max(ts) from {{ this }} where ok)` sliced from the *inner* - `where` and never examined the outer high-water-mark comparison. A guard body - that opens with a conjunction is now taken whole; the clause tier still - applies to arms that merely project a boolean, which is what motivated it. -- **`{% if not is_incremental() %}` was read as the incremental arm.** A clock in - that full-refresh branch was reported as blocking non-determinism on a correct - model. Negated arms are now skipped — see the still-open item below for why - they are not swapped for their complement. -- **Project-level YAML keys were matched at any indentation.** `target-path` and - every key `readDbtProjectPathList` reads also matched a same-named key nested - under `vars:`, sending the artifact search and every path-based scan at a - directory the project never configured — which blocks a genuinely green build. - Both are anchored at column 0 now. -- **A workspace-level required file failed the inverse gate.** With the dbt - project nested below the workspace, `dbt-deliverable-names` resolved - `reports/output.yml` from either root and `dbt-nothing-built` checked only - below `dbtRoot`, so a correct session passed one contract gate and was blocked - forever by the other. Both check both roots now. -- **Build staleness was dated from the model's DDL, not build completion.** - `Math.min(fresh.mtimeMs, ddlMtime)` always resolved to the DDL, because a - model with a status row was covered by that artifact's own invocation and dbt - writes per-model DDL as it walks the DAG. The 60 s tolerance therefore started - ticking at compile time, and a tidy-up edit seconds after a long green build - read as stale. `fresh.mtimeMs` is correct when a status row exists; DDL mtime - remains correct for the DDL-only case. -- **`update` / `modify` / `change` did not state a contract.** The most ordinary - phrasing for work on an existing relation ("Update the model `orders`") named - its deliverable literally and yielded nothing, so both contract gates went - inapplicable and a zero-write session finished clean. Added with the same - bounded inflections the other modification verbs use. -- **An unreadable `ALTIMATE_VALIDATORS_TASK_FILE` erased the contract.** A stale - or misspelled pin returned no task document at all rather than falling back to - discovery, and all three contract gates skipped in silence. A readable pin is - still exclusive. -- **Repository text reached the retry prompt unsanitised.** The consensus pass - routed `dbt-build-green`'s untrusted values through `sanitizeForPrompt` but - left the same exposure in three places: `dbt-nothing-built` interpolated - `expectation.taskFile` and the parsed deliverable list into `reason` and - `fixHint`, and `dbt-incremental-config` and `dbt-dialect-guard` interpolated - repository-derived model names. All build the same synthetic `role: "user"` - turn, so a name or filename carrying a newline breaks out of the sentence it - is quoted in and lands at instruction position. All now sanitized. -- **The inverse gate misdescribed an unrelated build.** A session that built - something other than the named deliverables was told it "produced no fresh - successful build artifact", which is simply false. It now branches on - `freshRun`. - -## Second review sweep — open, NOT addressed in this pass - -### 19. Both contract gates read the task document from the mutable worktree - -`dbt-deliverable-names` and `dbt-nothing-built` each call `readContract` / -`artifactExpectation` at completion time, against the live filesystem. A session -that deletes, renames or rewrites `TASK.md` so it no longer parses a contract -makes both gates return `appliesTo === false`, and can then terminate having -produced nothing — every remaining gate sees zero touched models. - -Why deferred: the honest fix is to capture the contract at session start, or to -read it from harness state the agent cannot write. Neither exists in this lane; -`ValidatorContext` carries `sessionStartMs` and nothing else. The available -half-measure — block when a task-document candidate path was written during the -session — cannot tell a deletion from a file that never existed, and fires on -the entirely normal case of a task that asks for the document itself to be -updated. That converts a false negative into a blocking false positive, which is -worse than the gap. Same missing infrastructure as items 4 and 8: a -session-scoped artifact store. - -### 20. The dialect guard's applicability is decided from the mutated filesystem - -`projectPrescribesGuards` runs at completion time. A session that removes the -project's only `target.type` guard and leaves an unguarded `iff()` observes no -remaining convention, so the validator makes itself inapplicable — silently -accepting exactly the portability regression it exists to catch. - -Why deferred: same as item 19. Applicability has to be determined from -pre-session project state, and there is nowhere to record it. - -### 21. Prose mining still over-extracts in two shapes - -Both are instances of item 13, restated with the specific shapes reviewers -found, so a later pass does not have to re-derive them: - -- `requirementHead` returns text from the start of the line, so code spans - *before* the requirement verb are collected. "Using the source `raw_orders`, - create the model `fct_orders`" demands `raw_orders` as a model, and no action - the agent can take satisfies it. Slicing from the verb instead trades this for - a passive-voice miss ("The `fct_orders` model must be created"), which is the - safer direction but is a behaviour change to the extractor, not a repair. -- A `## Required deliverables` section collects every code span under it, so - `- Model \`fct_orders\` with unique key \`order_id\`` makes `order_id` a - required model. The qualifier narrowing that fixed this for prose requirement - lines has no equivalent for section entries, which are structurally a list - rather than a sentence. - -The recorded direction of travel for item 13 is an explicit structured contract -with ambiguous prose yielding inconclusive, rather than a further round of -heuristics on top of these. Patching either shape in isolation keeps the -extractor on the treadmill this entry exists to get it off. - -### 22. `{% if not is_incremental() %}` arms are skipped, not complemented - -The negated form no longer produces a false positive on the full-refresh arm, -but the *real* incremental arm — the `{% else %}` — is still not inspected, so a -non-deterministic predicate there is missed. Identifying it means evaluating the -complement of an arbitrary Jinja condition across `elif` chains, which is the -same branch-semantics feature items 10 and 14 need, with the same property: a -half-implementation turns this miss into a blocking false positive on correct -models. - -### 23. `dbt-build-green` cannot see a model that was deleted - -`modelsModifiedSince` walks the CURRENT filesystem for `.sql` files with -`mtime >= sessionStartMs`. A file that existed at session start and was -deleted during the session cannot appear in that scan by construction — there -is no path to stat. With no fresh artifact either, `touchedPaths.length === 0 -&& !artifactIsFresh` takes the `nothing-to-gate` path, so a session that -deletes a model — leaving every downstream `ref()` to it broken — clears this -gate, `dbt-deliverable-names`, and `dbt-nothing-built` (which only checks -POSITIVE evidence of authored/built work) without a single one of them -inspecting the deletion. - -Why deferred: closing this needs to know the workspace's file listing AT -SESSION START, to diff against the current one. Two ways to get that, both -infrastructure this lane does not have: - -- A session-scoped snapshot written by the harness before the agent's first - turn — the same missing artifact store items 19 and 20 need, generalized - from "the task contract" to "the file tree". -- A VCS diff (`git diff --name-status --diff-filter=D` against a commit - bracketing the session start). This validator is deliberately built with - "No subprocess, no warehouse connection" as an architectural constraint (see - its own module doc), specifically so a completion check cannot itself become - a source of flakiness or a new external dependency in a path that gates - session termination. Adding a git subprocess call here is exactly the kind - of dependency that constraint rules out, and it degrades silently to - "no signal" the moment the workspace is not a git repository, or the - session's edits were never committed — the common case for an in-progress - session — leaving the same blind spot with an added dependency and no - reliable win. - -No half-measure was found that narrows this without either reintroducing a -subprocess dependency the validator's own design forbids, or trading this -false negative for a blocking false positive on a correct session (the same -pattern items 19–20 already ruled out for the same reason). Left open with the -reasoning recorded rather than patched. - ---- - -## REQUIRED precondition before enforcement beyond shadow mode - -**`ALTIMATE_VALIDATORS_ENABLED` must not be turned on beyond shadow mode -(`ALTIMATE_VALIDATORS_SHADOW`) until the duplicated heuristics below are -consolidated into a single shared, tested primitive per concern.** Default-off -and shadow are fine to ship as-is; enforcement is not, until this is done. - -### The evidence - -PR #1175 went through four consecutive review-fix rounds after the initial -consensus pass (3d2f6e96be → 631be58f44 → 677b00bb19 → fc64a3619d, plus the -false-positive fix that landed after 677b00bb19's round-4 findings). Each -round fixed real, adversarially-tested bugs — but the SAME underlying defect -kept reappearing in a sibling validator or a sibling code path within the same -file, because the fix for it existed only where it was first found rather than -in one shared place: - -- **Bare-name vs. full-identity matching.** Fixed for `dbt-build-green`'s - status lookup, then separately for its failure-classification path, then - found again (round 4, open) in `dbt-build-green`'s manifest-exemption - lookup — three instances of the identical bug (a dependency package's node - standing in for the root project's node of the same bare name) in one file. -- **Extension/source-dir filtering.** Fixed for `collectProducedNodeNames`, - then separately for `dbt-nothing-built`'s authored-work scan — two - independent implementations of "only `.sql`/`.py` under - models/snapshots, only `.csv` under seeds." -- **Package-directory exclusion.** Fixed for `dbt-dialect-guard`'s convention - probe, then separately for `collectProducedNodeNames`'s manifest loop, then - found again (round 4, open) for `dbt-build-green`'s manifest-exemption - lookup — the same "is this node under `dbt_packages/`" check, written three - times. -- **Conditional-config resolution.** `readConfigAxes` needed its own - unresolved-`{% if %}` guard; `dbt-incremental-config`'s idempotency-scope - parsing has an open, structurally identical gap (round 4: an inline - non-model code span silently marks a demand as model-scoped and skips - enforcement) because it re-implements code-span/verb parsing independently - of `extractRequiredDeliverables`. -- **File-vs-model modification tracking.** Added for model names - (`modificationModels`), then separately for file paths - (`modificationFiles`), and the false-positive fixed in this pass (a - modification file outside the scanned dbt source paths could never be - satisfied) was a direct consequence of the file-path variant being a - parallel, incomplete copy of the model variant rather than sharing one - evidence primitive. - -Round 4 surfaced 11 more findings after the false-positive fix; 2 were the -false positive itself (now fixed), the other 9 are each **another instance of -one of the five duplicated-heuristic categories above** (recorded, not fixed, -per the hard-stop decision below) — that recurrence, not any single bug, is -the actual finding. - -### Why this gates enforcement specifically - -Shadow mode only measures; a false positive there is a data point. Enabled -mode blocks session termination on a false positive — the failure mode that -erodes trust in the gate fastest, and the one class of bug (file-vs-model -modification tracking, above) that round 4 actually produced. Per-validator -duplication has now demonstrably reintroduced false positives once already; -without consolidation there is no reason to expect the next duplicated copy -(idempotency scoping, manifest exemptions, the two round-4 findings already -identified as open) won't do the same the next time someone edits one of the -five copies and not the others. - -### What "consolidated" means here - -One shared, unit-tested module per concern, imported by every validator that -needs it, replacing the current copies: -- a single node-identity resolver (full manifest `unique_id` first, bare-name - fallback only when unresolved) used everywhere a validator matches a - touched file to a run-result row or a manifest node; -- a single source-directory/extension classifier used everywhere a validator - decides whether a file counts as a model/seed/snapshot; -- a single package-directory exclusion check used everywhere a validator - walks the manifest or the filesystem for project-owned nodes; -- a single conditional-config resolver (config value plus "was this call - inside an unresolved `{% if %}`") used by every reader of `config()` state; -- a single modification-vs-creation evidence primitive that takes a required - name OR path and returns "needs session evidence" vs. "existence suffices," - used by both the model and file branches of `dbt-nothing-built`. - -### Code pointer - -See the comment beside the `ALTIMATE_VALIDATORS_ENABLED` read in -`packages/opencode/src/session/prompt.ts` — it points back here. - -### Round-4 findings deferred, not fixed (hard stop after the false-positive fix) - -Recorded so the consolidation work above has a concrete list to close against, -rather than re-deriving it: - -1. `dbt-incremental-config.ts` — an idempotency-scope demand line whose inline - code span names a term/function rather than a model is read as scoped and - silently drops enforcement instead of falling back to workspace-wide. -2. `validator-utils.ts` (`extractRequiredDeliverables`) — a requirement line - containing both a creation and a modification clause records only the - first verb, so the modification set can miss a real modification target - named later on the same line. -3. `validator-utils.ts` (`sanitizeTelemetryDetails`) — a relative path nested - inside an object under a path-bearing key is not redacted; only top-level - array/primitive entries under the key inherit the "this is a path field" - context. -4. `dbt-nothing-built.ts` — modification contracts are only read from the - FIRST task document that names deliverables, not merged across every - document the way `dbt-deliverable-names` already does (item resolved for - that file in the third sweep; the equivalent merge was never applied to - `dbt-nothing-built`'s `artifactExpectation`). -5. `dbt-nothing-built.ts` — `matchedDeliverables.length > 0` is satisfied by - ANY one matched item, so a task requesting updates to multiple models/files - passes after only one of them has session evidence. -6. `dbt-build-green.ts` — `exemptFromManifest` (ephemeral/disabled) is still - looked up by bare name; a dependency's ephemeral/disabled `orders` can - exempt the root project's distinct `orders` from the coverage assertion. - The status and failure-classification lookups were already moved to full - manifest identity in this pass; this is the third instance of the same - bare-name gap in the same file. -7. `dbt-incremental-config.ts` — a path-scoped idempotency demand - (`` Make the model `models/orders.sql` idempotent ``) stores the full path - as the scoped name, which never matches the bare model name looked up at - enforcement time, so the scope silently never applies. -8. `validator-utils.ts` (`resolveWithinRoot`) — resolves the real path of the - candidate's containing DIRECTORY (closing the round-3 directory-symlink - finding) but not the candidate FILE itself; a required path that is - directly a symlink to a file outside the root still passes. -9. `validator-utils.ts` (dialect-guard masking) — a BigQuery `#`-style line - comment containing a curated function name is not stripped before - matching, since `stripSqlComments` only recognises `--` and `/* */`. diff --git a/.github/meta/harness-review-followups.md b/.github/meta/harness-review-followups.md deleted file mode 100644 index b742a46e80..0000000000 --- a/.github/meta/harness-review-followups.md +++ /dev/null @@ -1,65 +0,0 @@ -# Harness reliability review — deferred follow-ups - -Deferred MED findings from the pre-PR release review (pre-PR adversarial review). -All 5 HIGH findings plus selected MED/LOW items were fixed on this branch; the -items below were explicitly deferred and are listed verbatim from the review. - -[MED] packages/opencode/src/tool/truncation.ts:66 — the plain-async truncation path hardcodes 2,000 lines/50KiB while the Effect wrapper honors `tool_output` configuration — MCP output through `prompt.ts` therefore ignores user caps despite the shared-core claim — consolidate the wrappers or pass the resolved configuration through both, with parity tests. - -[MED] packages/opencode/src/session/compaction.ts:663 (renderCarryAnchors) — carry-anchor trimming stops when one item remains — one oversized model-generated "Accomplished" item defeats `maxTokens` and can undo compaction — permit dropping or truncating the final item and assert the rendered result satisfies the cap. - -[MED] packages/opencode/src/cli/cmd/idle-done.ts:157 — every command not recognized as read-only is treated as verification — an exit-zero install, cleanup, deployment, or arbitrary unknown command can satisfy the "green verify" precondition and trigger a false completion challenge — require configured or positively classified verification evidence; unknown commands should be ineligible. - -[MED] packages/opencode/src/session/compaction.ts:70 — observation masks retain the first 80 characters of pruned output, while the ledger retains raw command/path/pattern text — credentials, authorization headers, query data, and signed URLs can survive pruning and be recopied into later synthetic prompts — retain only allowlisted metadata or hashes and apply shared secret redaction. - -[MED] packages/opencode/src/session/compaction.ts:572 (renderLedger) — ledger capping repeatedly joins and re-estimates the whole array while removing one line at a time, after collecting the full session history — this is quadratic in unique writes and adds latency at the critical compaction path — bound collection early and trim using accumulated token costs or a single cutoff search. - -The following items from a later review pass were also considered and deliberately -deferred (no behavior change on this branch): - -[LOW] packages/opencode/src/cli/cmd/run/run-mode.ts — the run entrypoint writes its process-scoped mode marker into the environment for the process lifetime, and child processes inherit it; a nested interactive server launched from such a session would arm run-mode mechanisms for genuinely interactive clients — clear or scope the marker in the interactive entrypoints (mirroring the existing child-env cleanup for the sibling non-interactive marker). - -[LOW] packages/opencode/src/session/compaction.ts — module-level per-session pin state and per-session tracker read maps grow without bound in a long-lived server process (the pin map has no production eviction path; per-session read tracking keeps one entry per unique path for the session's lifetime) — bound both with the same LRU pattern now used by the session-state stores. - -[LOW] packages/opencode/src/session/prompt.ts — the post-compaction pinned-task reminder re-derives its source by streaming the FULL session history from the database on every generation once a session has compacted — cache the resolved pin source per session or query only the needed boundary messages. - -[LOW] packages/opencode/src/session/termination.ts — the post-compaction three-option completion nudge (including the completion-token instruction) is injected in ALL modes; interactive users can see an occasional bare completion token line with no interactive function — mode-gate the nudge text or document the cosmetic change. - -The following items came from an external multi-reviewer pass over this branch and -were deliberately deferred (no behavior change on this branch): - -[MED] packages/opencode/src/cli/cmd/run.ts — a prompt retry after a client timeout or connection reset can hit the server AFTER it accepted the original POST, sending the same task again and creating a second user message / duplicate execution — the retry loop has no way to tell "not yet accepted" from "accepted, response lost." Needs a stable idempotency/message key the server honors, or a way to confirm the first attempt was never accepted before retrying. - -[MED] packages/opencode/src/session/compaction.ts (fitHead) — the summarization-request budget reserves a fixed 2,000 tokens for the summary prompt, but the actual assembled prompt (default template + carry anchors + pin-summary addition + first-person reframe, or a plugin-supplied override) can exceed that on an active session — size the reservation from the actual assembled prompt instead of a constant. - -[MED] packages/opencode/src/session/compaction.ts (buildLedger) — on a session's second or later auto-compaction, the ledger is built from the already-filtered/compacted message view, not the full session stream, so verified-write facts from before the first compaction silently drop out of later ledgers — build from the full stream and let selection filter afterward. - -[LOW] packages/opencode/src/session/prompt.ts (uncountedTail) — the proactive overflow estimate sums tool-result tokens on messages AFTER the last-finished assistant message, but a tool call and its result can live on that SAME message when the turn is still mid-flight — those results are excluded from the estimate, so compaction can fire a step later than it should on a heavy tool-output turn. - -[LOW] packages/opencode/src/tool/truncate-core.ts (preview, middle direction) — a degenerate `maxBytes: 1` config (no realistic caller sets this) can still allocate one byte to each of the head/tail halves and exceed the byte budget by a small margin; the equivalent `maxLines: 1` case was already fixed by degrading to tail-only — extend the same degrade to the byte-only case. - -[LOW] packages/opencode/src/session/{termination,nudge,tool-result-cap}.ts, packages/opencode/src/cli/cmd/{run-accounting,idle-done}.ts — these 5 new modules use `export namespace` for organization, which the repo's module-shape convention (`packages/opencode/AGENTS.md`) asks new code to avoid in favor of flat exports + a bottom-of-file self-reexport. ~69 pre-existing files in the package already use the same pattern, so this is consistent with existing debt rather than a regression; fold into a holistic namespace-to-flat-exports cleanup across the package rather than converting these 5 files in isolation. - -[LOW] packages/opencode/test/session/starvation.test.ts — the run-mode "armed" gate test re-implements the gate expression as a local helper instead of importing the real predicate from processor.ts, so a future change to the actual gate (added condition, reordered precedence, renamed exemption) would not be caught by this test — extract the gate into a shared, directly-testable predicate. - -The following items came from a further multi-reviewer pass over this branch and -were deliberately deferred (no behavior change on this branch): - -[MED] packages/opencode/src/session/llm.ts (addHistoricalToolStubs) — the stub-injection skip was narrowed to `toolChoice === "none" && no tools`, which closes the normal-turn regression, but the compaction summarizer still takes that path with a head that references historical tool calls. The skip rests on the claim that omitting both `tools` and `tool_choice` is universally accepted; the issue the stubs exist to fix concerned validation of referenced historical tool calls, which is orthogonal. Verify against the provider that originally needed the stubs before changing anything — reintroducing stubs here would advertise callable tools on a text-only call, so this is a provider-compatibility question, not a code cleanup. - -[MED] packages/opencode/src/session/starvation.ts (repeatSignature / consecutive-signature counter) — the repeat signature folds the failure message in but the counter accumulates for successful calls too, so three identical SUCCESSFUL calls (re-running the same verification command, polling a status probe) reach the threshold and produce a directive asserting that repeating the call cannot change the result, which is untrue for a polling or verification call. Restricting accumulation to calls that carry a failure message narrows the detector's semantics and its overlap with the identical-args doom-loop ladder; make that change with validation data rather than in review, and note the directive text is prompt-visible. - -[LOW] packages/opencode/src/cli/cmd/run-accounting.ts (termination) — `why_model_stopped` falls back to "stop" when no step-finish reason was ever recorded (an aborted run that never completed a step), so the run record attributes an ordinary stop to a session that never reported one. Distinguishing it needs a new value in the published record's enum, which is an output-contract change for downstream consumers — batch it with the next deliberate revision of the run record schema. - -[LOW] packages/opencode/src/session/prompt.ts (post-compaction pin reminder) and packages/opencode/test/cli/idle-done.test.ts — the idle-done unit tests now pin production event ordering (step-finish part before the step's snapshot patch part) in two dedicated cases, but the shared fixtures still emit the patch first; converge the remaining fixtures on production ordering when the file is next touched. - -The following items came from the fourth review pass over this branch and were -deliberately deferred (no behavior change on this branch): - -[MED] packages/opencode/src/session/processor.ts (dispatch tool-result cap) — the per-result hard cap is applied on the successful `tool-result` branch only. A `tool-error` still persists an unbounded error string, and an interrupted running tool keeps partial output in metadata that `message-v2.ts` later replays as a tool result, so a failed call with very large stderr can still overflow the next request. Applying the cap to error text and interrupted partial output changes what gets PERSISTED on the failure path, which is where diagnostics come from — size it against real failure payloads before truncating them. - -[MED] packages/opencode/src/cli/cmd/idle-done.ts (verify classification) — with no configured verify command, ANY non-read-only bash command is treated as a verification, so a mutating command that is not on the mutating-heads list (a build script that also writes generated sources, say) can register as the green verify it is not. The mutation watermark now advances for the in-place, redirection, and known-mutating-head forms, but the underlying "not read-only implies verification" inference still needs replacing with an explicit verify classifier. - -[LOW] packages/opencode/src/cli/cmd/run.ts (--max-turns) — `--max-turns 0` disables the limit entirely because the guard is a truthiness check, and negative or fractional values are accepted without validation. For a governance control an explicit zero should not mean unlimited; fix it together with the CLI validation pass that gives the other numeric options explicit range errors, so the behavior change is documented in one place. - -[LOW] packages/opencode/src/tool/bash.ts (child env) — `ALTIMATE_RUN_RESUMED` joins `ALTIMATE_RUN_MODE` as a marker that nested processes inherit from the bash tool's merged env. Its leak direction is benign (a nested run would use interactive pin selection), but it belongs in the same holistic decision about which run-scoped markers get stripped for child processes. diff --git a/.github/meta/night-run/MERGE-VALIDATION-MATRIX.md b/.github/meta/night-run/MERGE-VALIDATION-MATRIX.md deleted file mode 100644 index 989931d67e..0000000000 --- a/.github/meta/night-run/MERGE-VALIDATION-MATRIX.md +++ /dev/null @@ -1,47 +0,0 @@ -# Comprehensive merge validation — upstream v1.4.0→v1.17.9 bridge (PR #964) - -Method: not spot-checks. The merge surface (3,254 upstream commits) reduces to a BOUNDED, -enumerable set validated by three contracts, each covered exhaustively. - -## Surface (measured) -- 226 upstream-shared source files modified by the merge (exist in both v1.17.9 and HEAD) -- 293 fork-only source files -- 106 fork "altimate_change" behaviors present on main whose marker text wasn't found verbatim on HEAD - -## Coverage matrix - -| Contract | Question | Method | Result | -|---|---|---|---| -| C1: no dropped upstream code | did conflict-resolution revert upstream fixes? | 226 files reviewed by 11 subsystem agents; diff v1.17.9↔HEAD each | No ship-blocker. ~6 real MED/LOW refinement drops (below). High-value upstream fixes (Plan-Mode security, models-cache recovery, session-metadata migration identity, corrupt-message handler) present VERBATIM. | -| C2: no fork-feature loss | did fork customizations survive? | 106 candidate behaviors verified present-on-HEAD or dropped | Most false (path-restructure moved cli/cmd/tui→tui/src + reworded). Real drops: beginner-tips onboarding, internal-url inconsistency, minor branding. | -| C3: behavior | does the artifact work? | full suite + schema + upgrade + journey + real binary + CI | 10,555 tests pass (2 flaky, pass isolated); 19/19 schema tables parity; upgrade 16/16; journey 10 green; CI functional gates green. | - -## REAL findings (all MED/LOW — none block ship), fork-wanted, verified by lead - -| # | file | issue | sev | fix | -|---|------|-------|-----|-----| -| 1 | session/compaction.ts | tail-preserving compaction dead: consumers read `tail_start_id` (session.ts:775, message-v2.ts:1016) but no writer sets it on new compaction parts | MED | restore the writer that stamps `tail_start_id` when creating the compaction part | -| 2 | share/share-next.ts:152 | dropped `Effect.catchCause` on fullSync+flush → unhandled rejection on share-sync failure | MED | wrap fullSync/flush failures (log, don't throw) | -| 3 | session/llm.ts:214 | Copilot billing split-brain: `includeRawChunks` absent from streamText, but `copilotTotalNanoAiu`/`totalNanoAiu` consumers depend on it → Copilot billing metadata never populates | MED | pass `includeRawChunks: true` for the Copilot provider path | -| 4 | tui first-run (tips) | fork onboarding UX dropped: main had `BEGINNER_TIPS`/`isFirstTime`; HEAD has zero refs | MED | restore beginner-tips/first-time hint | -| 5 | util/filesystem.ts:131 | Windows path conversion lost: HEAD does only `realpathSync.native`; upstream also did `windowsPath()` (cygwin/gitbash/WSL) | LOW | restore windowsPath() normalization for win32 | -| 6 | session/retry.ts:71 | drops upstream's extra "retry 5xx even if SDK says non-retryable" guard | LOW | re-add the status-code 5xx retry guard | -| 7 | cli/cmd/mcp.ts:419 | CLI MCP installs default to opencode.json not altimate-code.json (both still discovered) | LOW | prefer altimate-code.json in CONFIG_FILENAMES | -| 8 | tui.ts:166 / runtime.ts:737 | internal worker url `opencode.internal` inconsistent with `altimate-code.internal` elsewhere (internal routing key, not user-visible) | LOW | unify the virtual-host string | -| — | footer/uninstall/upgrade/initialize.txt | unmarked fork branding + 1-2 "OpenCode config" strings | LOW | marker hygiene + branding fix | - -## INTENTIONAL divergences (verified NOT bugs — fork deliberately differs) -- task.ts background subagents — marker: "hides disabled background mode" -- providers Alibaba/Venice/Azure-workflow/GitLab-Duo — moved to packages/core/src/plugin/provider/* or not shipped -- run.ts interactive/replay/demo — fork ships its own run command (executive/analyst modes, max-turns, tracing) -- plugin/index.ts loader + omitted upstream provider plugins — fork ships its own provider set -- server.ts move-session — actually PRESENT via httpapi control-plane group (false positive) - -## FALSE positives (verified) -- agent.ts:524 safety-denial re-append — intended fail-safe (marker documents it) - -## Verdict -Comprehensive review of the ENTIRE merge surface found NO ship-blocker (no crash, data-loss, or -security regression). ~6 medium + a few low fork-wanted refinements to restore. The merge correctly -adopted upstream v1.17.9 (fixes present verbatim, schema parity, upgrade path clean) and preserved -the fork (registries, agents, tools, DE features intact). diff --git a/.github/meta/night-run/RELEASE-EVALUATION-BOARD.md b/.github/meta/night-run/RELEASE-EVALUATION-BOARD.md deleted file mode 100644 index 41013506cf..0000000000 --- a/.github/meta/night-run/RELEASE-EVALUATION-BOARD.md +++ /dev/null @@ -1,45 +0,0 @@ -# Release-Evaluation Board — final pre-release analysis (PR #964, v1.17.9 bridge) -7 independent perspectives (3 codex / 3 sonnet / 1 haiku) + Fable runtime probe + synthesis. -GOAL: dual preservation — don't lose FORK work, don't lose UPSTREAM work. - -## Scorecard - -| Perspective | Verdict | Findings | -|---|---|---| -| D Fork-feature preservation | **PASS** | All tools (101→102 superset), 9 agents, 13 warehouse drivers, 7 builtin skills, 8 commands, memory/tracing/telemetry/PII — present + WIRED. 0 new fork drops. | -| F Safety/security regression | **PASS** | All 5 shipped safety properties INTACT: DDL non-overridable, subagent deny-inheritance (#26597), sensitive-write #209 guard (edit.ts+write.ts), secret redaction, basic-auth username. No regression. | -| A Upstream-fix adoption (73 sampled) | 5 dropped | 1 HIGH-ish (processor providerExecuted), 4 MED/LOW upstream fixes reverted. | -| B Upgrade/format compat | 2 real | managed-TUI-config load dropped (MED), .json/.jsonc precedence inverted (LOW). Auth-username & thinking-default changes INTENTIONAL. DB/auth/session/cache formats back-compatible. | -| C Build/packaging/distribution | **1 HIGH ship-blocker** | release.yml pinned Bun 1.3.10 but build scripts require ^1.3.14 → tag releases fail. FIXED. | -| E Integration seams | 1 real | Azure/DigitalOcean/xAI upstream auth plugins in-tree but unwired in INTERNAL_PLUGINS (MED). Codex plugin = intentional fork keep. | -| G Branding | LOW polish | `.opencode/` paths in warehouse-list/skill-ops messages + `opencode.json` in provider dialogs should say altimate-code. Provider labels & auth-username default = intentional. | -| Fable runtime probe | **PASS** | Compiled binary: palette, agent-switch, shell-mode EXECUTE, /skills list, prompt round-trip all work — empirically confirms the resolved V2-premise review comments don't affect the shipped product. | - -## Actions taken (all upstream-fix restorations, marked + tested) -- FIXED (HIGH ship-blocker): release.yml Bun 1.3.10 → 1.3.14 (all 4 pins). -- FIXED (Fable): run.ts in-process auth header (local run 401'd when OPENCODE_SERVER_PASSWORD set). -- FIXED (delegated): provider.ts Cloudflare unified apiKey; transform.ts Devstral toLowerCase; project.ts session-migration time_updated preservation (3 sites); config/tui.ts managed-config load + paths.ts json/jsonc precedence; plugin/index.ts wire Azure/DigitalOcean/xAI auth plugins. - -## Flagged follow-up (NOT fixed — deliberate) -- processor.ts `providerExecuted` handling: shipped session doesn't use the provider-executed-tool flag (adapters capture it). MED — server-side provider tools (e.g. web search) not handled optimally. NOT fixed pre-release: the upstream change is entangled with GitLab-Duo approval code the fork doesn't ship; porting it blind risks core session processing. Needs a scoped follow-up. -- Branding LOW polish: optional, non-blocking. - -## Dual-preservation verdict -- FORK preservation: CLEAN (D + F pass; nothing we built is lost or weakened). -- UPSTREAM preservation: the gaps were HERE — 5 dropped fixes + 3 unwired auth plugins + managed-config, now restored. This is the class my earlier fork-focused passes missed; the board's upstream-adoption + seam lenses caught them. - -## Addendum — E (integration seams) full report -Enumerated the seam surface: 587 `@opencode-ai/*` import sites across 216 files in packages/opencode/src + 37 in packages/tui/src; 6 subsystem scopes audited. -- **Azure/DigitalOcean/xAI auth plugins unwired** (MED-HIGH) — FIXED (board commit b0ade919c7). -- **`openai/codex.ts` orphaned** (MED) — DOCUMENTED, not fixed. 0 consumers; fork deliberately uses its own `./codex` (BUILTIN). Harmless dead code; deleting an upstream file risks future-merge friction. Adopting its WebSocket/allowlist/dispose capabilities is a product decision, not a merge fix. -- **Dual-`Session`-module `getUsage` divergence** (MED-latent / LOW-active) — DOCUMENTED, not fixed. Live path is `processor.ts:326 → session/index.ts:830` (carries the OpenRouter cost fix); `session.ts:387` is the caller-less V2-migration copy. Not a live bug; session.ts is the not-shipped V2 surface. -- **OVERTURNED false-positive HIGH**: an earlier subagent claimed `fromPlugin` permission-gate bypass (ctx.ask became Effect → `await` no-ops). E traced the `legacyToInit`→bridged-`legacyCtx` layer (tool-zod-compat.ts:221) that makes ask Promise-based before the plugin runs — the gate runs correctly. Safety surface confirmed clean. -- Server / cli / effect / config / storage seam scopes: clean. Signature-skew class (`Adaptor.target()`) already fixed; 192 tracked `upstream_fix` markers + green typecheck catch compile-level skew. - -## Final board disposition -- Ship-blocker: 1 (release.yml) — FIXED. -- Real dropped-upstream fixes/features: 6 + 3 auth plugins + run-auth — FIXED. -- Security (pre-existing): sensitive-write guard neutralization — FIXED. -- provider-executed metadata — FIXED (execution was already correct); settlement-telemetry field = scoped follow-up. -- Latent/dead-code in the not-shipped V2 surface (codex orphan, getUsage copy) — DOCUMENTED, deliberately not chased. -- Fork preservation + safety surface: PASS (nothing lost or weakened). diff --git a/.github/meta/night-run/UPGRADE-PATH-TEST.md b/.github/meta/night-run/UPGRADE-PATH-TEST.md deleted file mode 100644 index b290d15dd8..0000000000 --- a/.github/meta/night-run/UPGRADE-PATH-TEST.md +++ /dev/null @@ -1,34 +0,0 @@ -# Upgrade-path test: v0.8.10 → upstream/merge-v1.17.9 (PR #964) - -Method: built BOTH binaries from source (v0.8.10 tag + branch). Isolated via XDG_* -temp dirs (v0.8.10 reads data dir from xdg-basedir, NOT OPENCODE_TEST_HOME — that -only overrides `home`). Booted v0.8.10 to create an authentic old-schema DB -(10 drizzle migrations; `session` but no `session_message`/`project_directory`; -session lacks metadata/cost/tokens_*/agent/model). Injected sentinel project + -session rows + a v0.8.10 config.json + auth.json. Booted the NEW binary against -the SAME isolated dir = the upgrade moment. - -## Result: 16/16 assertions PASS + idempotent across 3 boots -Schema migrated forward: session.{metadata,cost,tokens_input,tokens_output,agent, -model} added; project_directory + session_message tables created; drizzle journal -advanced 10→11; NO "duplicate column" crash; TUI rendered clean. -Data preserved: sentinel project + session survived; session title byte-identical -(sha1 match); 0 rows lost. Config + auth.json carried over. integrity_check = ok -before and after. - -## Minor finding — FIXED (commit 27c490515a) + verified -`__drizzle_migrations` grows by 1 row per launch: the `20260511173437_session-metadata` -entry is re-inserted every boot (markDrizzleEntriesApplied uses INSERT OR IGNORE but -the table has no UNIQUE constraint, so it never dedupes). DDL is guard-checked so it -never re-runs → no crash, no data change, ~50 bytes/launch. FIXED: markDrizzleEntriesApplied now INSERT...WHERE NOT EXISTS(name) + a guarded -dedupeDrizzleJournal self-heal. Re-verified end-to-end: fixed binary boots 3x -against a v0.8.10 DB with journal rows STABLE at 11 (was 11->12->13), no dup -names, integrity ok. Regression test test/storage/db-journal-idempotency.test.ts. - -## Safety note -v0.8.10 honors OPENCODE_DISABLE_CHANNEL_DB and reads its data dir from XDG. An early -probe that set that flag without XDG isolation briefly opened the real -~/.local/share/altimate-code/opencode.db (1194 sessions) READ-only-ish — verified -intact (integrity ok, 1194 sessions preserved; v0.8.10 migrations are guarded/idempotent). -Correct isolation for old-binary tests = XDG_{DATA,CONFIG,STATE,CACHE}_HOME, not -OPENCODE_TEST_HOME. diff --git a/docs/docs/data-engineering/guides/using-with-claude-code.md b/docs/docs/data-engineering/guides/using-with-claude-code.md index 48533ce8b6..4239b5af90 100644 --- a/docs/docs/data-engineering/guides/using-with-claude-code.md +++ b/docs/docs/data-engineering/guides/using-with-claude-code.md @@ -1,6 +1,6 @@ --- title: "Using Altimate with Claude Code" -description: "Run Altimate's data engineering tools alongside Claude Code via the /altimate command or MCP server integration." +description: "Hand data engineering work from Claude Code to Altimate Code with the Altimate Code plugin, and use an Altimate workspace's skills, knowledge and memory from Claude Code." --- # Using altimate with Claude Code @@ -80,9 +80,29 @@ altimate: ## Setup -1. Install altimate globally: `npm install -g altimate-code` -2. Configure warehouse connections in your project -3. Run `/configure-claude` inside altimate to set up the integration: +### The Altimate Code plugin (recommended) + +1. Install Altimate Code (Node.js 20 or later): `npm install -g altimate-code` +2. Install the plugin from Anthropic's plugin directory: + + ```bash + claude plugin install altimate-code + ``` + + Or add Altimate's own marketplace and install from it: + + ```bash + claude plugin marketplace add AltimateAI/altimate-claude-plugin + claude plugin install altimate-code@altimate-claude-plugin + ``` + +3. Start a new Claude Code session in your project. Ask for dbt or warehouse work as usual, or start a request with `/altimate-code:altimate` to hand it over yourself. (Claude Code names plugin commands `:`; the bare `/altimate` below comes from `/configure-claude`.) + +The plugin runs `altimate-code run` in your project for each task, with the model provider Altimate Code is set up with. If you have not set one up, run `altimate-code` once and choose a provider, or sign in to Altimate (below). + +### The `/altimate` command, without the plugin + +Run `/configure-claude` inside altimate to write the command: ```bash altimate @@ -92,6 +112,36 @@ altimate This creates `~/.claude/commands/altimate.md`. You can now use `/altimate` in any Claude Code session. +## Use an Altimate workspace from Claude Code + +Claude Code reaches a workspace through Altimate Code: every task the plugin hands over runs in your project, so it gets the workspace the project is linked to — its skills, its knowledge, your team's shared memory and its integrations. Nothing in Claude Code itself needs configuring. + +1. **Sign in to Altimate.** In a terminal: + + ```bash + altimate-code auth login -p altimate-backend + ``` + + Choose **Altimate LLM Gateway**. It opens your browser; sign in and return to the terminal. (Inside the TUI, `/connect` does the same.) + +2. **Link the project.** In the project folder: + + ```bash + altimate-code link + ``` + + Pick the workspace, or create one. Linking needs an interactive terminal; from a script or an agent, use `altimate-code link --workspace ` or `altimate-code link --create`. + +3. **Check it.** `altimate-code workspace status` shows the linked workspace and when its skills last synced. + +4. **Ask Claude.** Questions about your team's conventions, best practices or knowledge are answered from the workspace when they reach Altimate Code. Start them with `/altimate-code:altimate` to be sure they do: + + ``` + /altimate-code:altimate what are our naming conventions for staging models? + ``` + +What you teach Altimate Code during these tasks is saved to the workspace's memory and shared with your team in the CLI, the IDE and Studio. If an upload fails, the task ends with a line saying so; `altimate-code workspace sync` sends what is left. Skills added to the workspace sync into `.altimate-code/skill/_workspace/` at the start of the next task. + ## When to use which | Task | Use Claude Code | Use altimate | diff --git a/packages/opencode/src/altimate/cli/run-stdin.ts b/packages/opencode/src/altimate/cli/run-stdin.ts new file mode 100644 index 0000000000..a366e1ab4e --- /dev/null +++ b/packages/opencode/src/altimate/cli/run-stdin.ts @@ -0,0 +1,62 @@ +import { fstatSync } from "fs" + +/** How long `run` waits for the first byte on a stdin that is not a TTY or a file. */ +export const STDIN_WAIT_MS = 3000 + +export interface StdinSource { + /** True when fd 0 is a regular file (`run < prompt.txt`), which always reaches EOF. */ + isFile(): boolean + readAll(): Promise + stream(): ReadableStream +} + +export const bunStdin: StdinSource = { + isFile() { + try { + return fstatSync(0).isFile() + } catch { + return false + } + }, + readAll: () => Bun.stdin.text(), + stream: () => Bun.stdin.stream(), +} + +/** + * Read text piped into `altimate-code run`, without waiting on a stdin that never sends anything. + * + * Agents that shell out to `run` (Claude Code's Bash tool, a Node `spawn` with default stdio) hand + * it a pipe or socket they never write to or close. Reading that to EOF blocks forever, so a + * delegated task hangs before it starts. A file is read in full; any other stdin is read only if + * its first chunk arrives within `waitMs`, and then to EOF. + */ +export async function readRunStdin( + source: StdinSource = bunStdin, + waitMs: number = STDIN_WAIT_MS, +): Promise<{ text: string; timedOut: boolean }> { + if (source.isFile()) return { text: await source.readAll(), timedOut: false } + const reader = source.stream().getReader() + let timer: ReturnType | undefined + try { + const first = await Promise.race([ + reader.read(), + new Promise<"timeout">((resolve) => { + timer = setTimeout(() => resolve("timeout"), waitMs) + }), + ]) + if (first === "timeout") return { text: "", timedOut: true } + if (first.done) return { text: "", timedOut: false } + const decoder = new TextDecoder() + let text = decoder.decode(first.value, { stream: true }) + while (true) { + const next = await reader.read() + if (next.done) break + text += decoder.decode(next.value, { stream: true }) + } + return { text: text + decoder.decode(), timedOut: false } + } finally { + if (timer) clearTimeout(timer) + // Releases the stream so an idle stdin does not keep the process alive after the run. + reader.cancel().catch(() => {}) + } +} diff --git a/packages/opencode/src/altimate/workspace/empty-record-path.ts b/packages/opencode/src/altimate/workspace/empty-record-path.ts new file mode 100644 index 0000000000..b38f8e6833 --- /dev/null +++ b/packages/opencode/src/altimate/workspace/empty-record-path.ts @@ -0,0 +1,12 @@ +import path from "path" +import { createHash } from "node:crypto" +import { Global } from "@/global" + +/** Where a sync records that this project's workspace has no skills. Keyed by the resolved + * directory, so the TUI thread and the prompt worker agree on one file. Under the app's state + * directory, so nothing is written into the user's repository. Kept out of `skill-sync` so the + * per-turn gate in `session/prompt` can check it without loading that module. */ +export function emptyRecordPath(canon: string): string { + const id = createHash("sha256").update(canon).digest("hex").slice(0, 32) + return path.join(Global.Path.state, "altimate-workspace-empty", id) +} diff --git a/packages/opencode/src/altimate/workspace/memory-sync.ts b/packages/opencode/src/altimate/workspace/memory-sync.ts index 6302d8bdc7..14c790c075 100644 --- a/packages/opencode/src/altimate/workspace/memory-sync.ts +++ b/packages/opencode/src/altimate/workspace/memory-sync.ts @@ -837,6 +837,18 @@ function serialize(scope: "global" | "project", blockId: string, op: () => Pr * `skill-sync.flushPendingSyncs` holds it for a cold skill sync. */ const mirrorsInFlight = syncState.mirrorsInFlight +/** Blocks whose mirror failed in this process, by `/`. Nothing retries them in a one-shot `run` + * (the TUI and `serve` poll; `run` does not), so its exit reports them instead of letting the block stay on + * this machine unannounced. Process-global for the same two-module-graph reason as `syncState`. */ +const failedMirrors: Set = ((globalThis as unknown as Record | undefined>)[ + Symbol.for("altimate.memory-sync.failed-mirrors") +] ??= new Set()) + +/** How many blocks this process saved locally but could not send to the workspace. */ +export function failedMirrorCount(): number { + return failedMirrors.size +} + /** Hold a task in `mirrorsInFlight` for its lifetime. */ async function tracked(task: Promise): Promise { mirrorsInFlight.add(task) @@ -884,7 +896,14 @@ export async function mirrorBlock(block: MemoryBlock, directory?: string): Promi const binding = await currentBinding(directory) if (!binding) return if (!(await memoryEnabled(binding))) return - await push(block, binding, undefined, directory) + const key = `${block.scope}/${block.id}` + try { + await push(block, binding, undefined, directory) + failedMirrors.delete(key) + } catch (err) { + failedMirrors.add(key) + throw err + } }) return tracked(task) } diff --git a/packages/opencode/src/altimate/workspace/skill-sync.ts b/packages/opencode/src/altimate/workspace/skill-sync.ts index e7af76124a..7fbf6093a9 100644 --- a/packages/opencode/src/altimate/workspace/skill-sync.ts +++ b/packages/opencode/src/altimate/workspace/skill-sync.ts @@ -38,10 +38,9 @@ // detection is per-skill ``updated_at`` and the only integrity check available // is byte length. import fs from "fs/promises" -import { createHash } from "node:crypto" import { statSync } from "node:fs" import path from "path" -import { Global } from "@/global" +import { emptyRecordPath } from "./empty-record-path" import { Flag as CoreFlag } from "@opencode-ai/core/flag/flag" import { Log } from "@/altimate/util/log" import { AltimateApi } from "@/altimate/api/client" @@ -655,14 +654,10 @@ export async function recentlySynced(directory: string): Promise { /** Which account each project's snapshot was last fetched for. */ const syncedFor = store.syncedFor -/** Where the record lives that a project's workspace had no custom skills at the last sync - * (account key + workspace id). On disk rather than on `globalThis`, which is per thread: an - * unlink in the TUI thread has to clear what the prompt worker's sync recorded, and the other way - * round. Under the app's state directory, so nothing is written into the user's repository. */ -function emptyRecordPath(canon: string): string { - const id = createHash("sha256").update(canon).digest("hex").slice(0, 32) - return path.join(Global.Path.state, "altimate-workspace-empty", id) -} +/* The record that a project's workspace had no custom skills at the last sync (account key + + * workspace id) lives at `emptyRecordPath`. On disk rather than on `globalThis`, which is per + * thread: an unlink in the TUI thread has to clear what the prompt worker's sync recorded, and the + * other way round. */ /** The record as written: `\n`. The write id tells one sync's record from another * sync's record for the same key, so a sync only ever withdraws the record it wrote itself. */ diff --git a/packages/opencode/src/cli/cmd/link.ts b/packages/opencode/src/cli/cmd/link.ts index dbfaa92241..42346bc8da 100644 --- a/packages/opencode/src/cli/cmd/link.ts +++ b/packages/opencode/src/cli/cmd/link.ts @@ -54,6 +54,10 @@ import { stripBidiControls, } from "@/altimate/workspace/workspace-name" +/** The browser sign-in from a plain shell: the Altimate LLM Gateway method of the `altimate-backend` + * provider saves the credentials every workspace command reads. */ +export const SIGN_IN_COMMAND = "altimate-code auth login -p altimate-backend" + const CREATE_NEW_SENTINEL = "__create_new__" const SET_UP_IN_BROWSER_SENTINEL = "__browser_handoff__" @@ -238,8 +242,10 @@ export const LinkCommand = cmd({ if (!process.stdin.isTTY) { UI.error( "`altimate-code link` needs an interactive terminal (stdin must be a TTY). " + - "Run it directly in a shell, or use the TUI palette command " + - '"Link this project to a workspace".', + "Run it directly in a shell, use the TUI palette command " + + '"Link this project to a workspace", or link without prompts: ' + + "`altimate-code link --workspace ` for an existing workspace, " + + "`altimate-code link --create` for a new one.", ) process.exitCode = 1 return @@ -247,7 +253,7 @@ export const LinkCommand = cmd({ if (!(await AltimateApi.isConfigured())) { UI.error( - "Not signed in to Altimate. Run the TUI (altimate-code) and sign in first, then re-run `altimate-code link`.", + `Not signed in to Altimate. Sign in with \`${SIGN_IN_COMMAND}\` (it opens your browser), then re-run \`altimate-code link\`.`, ) process.exitCode = 1 return @@ -1051,7 +1057,9 @@ export async function linkHeadless( return } if (!(await deps.isConfigured())) { - deps.printError("Not signed in to Altimate. Run altimate-code, sign in, then re-run `altimate-code link`.") + deps.printError( + `Not signed in to Altimate. Sign in with \`${SIGN_IN_COMMAND}\` (it opens your browser), then re-run \`altimate-code link\`.`, + ) process.exitCode = EXIT_USAGE return } diff --git a/packages/opencode/src/cli/cmd/run.ts b/packages/opencode/src/cli/cmd/run.ts index 6766583bad..738596106a 100644 --- a/packages/opencode/src/cli/cmd/run.ts +++ b/packages/opencode/src/cli/cmd/run.ts @@ -552,8 +552,18 @@ export const RunCommand = cmd({ // Earlier revision used `!process.stdin?.isTTY`, which turned the crash // into a stall: undefined stdin satisfied the guard and we then awaited // Bun.stdin.text() on a stream that would never EOF. Skip the read - // entirely when there is no stdin to read from. - if (process.stdin && !process.stdin.isTTY) message += "\n" + (await Bun.stdin.text()) + // entirely when there is no stdin to read from. An agent's shell tool can + // also hand `run` a stdin it never writes to or closes, so the read is + // bounded: see `readRunStdin`. + if (process.stdin && !process.stdin.isTTY) { + const { readRunStdin, STDIN_WAIT_MS } = await import("../../altimate/cli/run-stdin") + const piped = await readRunStdin() + if (piped.timedOut) + process.stderr.write( + `No input on stdin after ${STDIN_WAIT_MS / 1000}s; continuing without it. Redirect stdin from /dev/null to skip the wait.${EOL}`, + ) + message += "\n" + piped.text + } // altimate_change end if (message.trim().length === 0 && !args.command) { @@ -2098,6 +2108,16 @@ You are speaking to a non-technical business executive. Follow these rules stric .then((m) => m.flushPendingMirrors()) .catch(() => {}), ]) + // A mirror that failed is not retried by this process, and nothing else + // tells the user the memory stayed on this machine. + const unsent = await import("../../altimate/workspace/memory-sync") + .then((m) => m.failedMirrorCount()) + .catch(() => 0) + if (unsent > 0) + process.stderr.write( + `${unsent === 1 ? "1 memory was" : `${unsent} memories were`} saved here but could not be sent to the workspace. ` + + `Run \`altimate-code workspace sync\` to send ${unsent === 1 ? "it" : "them"}.${EOL}`, + ) } // altimate_change end diff --git a/packages/opencode/src/cli/cmd/workspace.ts b/packages/opencode/src/cli/cmd/workspace.ts index 240ecc9700..a6c5c7007d 100644 --- a/packages/opencode/src/cli/cmd/workspace.ts +++ b/packages/opencode/src/cli/cmd/workspace.ts @@ -15,7 +15,7 @@ import * as prompts from "@clack/prompts" import { cmd } from "./cmd" import { bootstrap } from "../bootstrap" import { UI } from "../ui" -import { stripControlChars } from "./link" +import { SIGN_IN_COMMAND, stripControlChars } from "./link" import { AltimateApi } from "@/altimate/api/client" import * as Manage from "@/altimate/workspace/manage" import { resolveBindingOutcome, type BindingOutcome, type CachedBinding } from "@/altimate/workspace/state" @@ -162,7 +162,7 @@ function failure(deps: WorkspaceDeps, json: boolean, code: number, message: stri } /** Checked first in every subcommand, so each fails the same way with the same exit code. */ -const NOT_SIGNED_IN = "Not signed in to Altimate. Run altimate-code, sign in, then try again." +const NOT_SIGNED_IN = `Not signed in to Altimate. Sign in with \`${SIGN_IN_COMMAND}\` (it opens your browser), then try again.` const NOT_LINKED_LINES = ["This project is not linked to a workspace.", "Link it with `altimate-code link`."] /** diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index 207c761aca..fef3f093bc 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -9,6 +9,8 @@ import { SessionID, MessageID, PartID } from "./schema" // altimate_change start — module-scope echo transport predicate import { MessageV2, isEchoTransportError } from "./message-v2" // altimate_change end +// altimate_change — a linked workspace with no skills yet still waits for the skill sync; see `prompt` +import { emptyRecordPath } from "../altimate/workspace/empty-record-path" import { Log } from "../util/log" import { SessionRevert } from "./revert" import { Session } from "." @@ -418,11 +420,14 @@ export namespace SessionPrompt { } else { // Workspaces are on by default, so this branch runs on every user's turn. Awaiting it puts // event-loop ticks (and up to WORKSPACE_SKILL_WAIT_MS) before `createUserMessage` — the - // reordering described above. Only a project that already has a workspace snapshot can have - // skills to land on this turn, so only that case waits; everyone else starts the sync in the - // background and keeps the synchronous path. The background sync does not refresh the skill - // registry when it lands, so its skills reach the next turn (which finds the snapshot and - // refreshes before it starts), never part-way through this one. + // reordering described above. Only a project a sync has already seen linked can have skills + // to land on this turn, so only that case waits: one with a workspace snapshot, or one whose + // workspace had no skills at the last sync. Without the second, the first skill added to an + // empty workspace missed the next `run` entirely — each is a fresh process, so the skill was + // named in the workspace prompt but the skill tool could not load it. Everyone + // else starts the sync in the background and keeps the synchronous path. The background sync + // does not refresh the skill registry when it lands, so its skills reach the next turn (which + // finds the snapshot and refreshes before it starts), never part-way through this one. const workspaceSkillTurn = async (background: boolean) => { try { const skillSync = await import("../altimate/workspace/skill-sync") @@ -491,7 +496,11 @@ export namespace SessionPrompt { log.warn("workspace skill sync failed", { err: String(err) }) } } - if (existsSync(path.join(Instance.directory, ".altimate-code", "skill", "_workspace"))) await workspaceSkillTurn(false) + if ( + existsSync(path.join(Instance.directory, ".altimate-code", "skill", "_workspace")) || + existsSync(emptyRecordPath(path.resolve(Instance.directory))) + ) + await workspaceSkillTurn(false) // Registered now, synchronously, so a `run` that ends at once still flushes it on exit. else PendingTurns.track(workspaceSkillTurn(true)) } diff --git a/packages/opencode/test/altimate/workspace/memory-sync.test.ts b/packages/opencode/test/altimate/workspace/memory-sync.test.ts index b32598d174..3a00654d56 100644 --- a/packages/opencode/test/altimate/workspace/memory-sync.test.ts +++ b/packages/opencode/test/altimate/workspace/memory-sync.test.ts @@ -45,6 +45,7 @@ const { isEnabled, memoryEnabledCached, mirrorBlock, + failedMirrorCount, flushPendingMirrors, overlayBlocks, refresh, @@ -81,6 +82,8 @@ let createResult: any = [{ id: "mem-new" }] let workspaces: any[] = [{ id: 42, name: "acme", memory_enabled: true }] /** When set, GET /list fails -- used to prove a failed lookup never creates. */ let listFails = false +/** When set, POST /datamates/memory/ fails, as a create that times out does. */ +let createFails = false function stubFetch() { globalThis.fetch = (async (input: any, init?: any) => { @@ -90,6 +93,9 @@ function stubFetch() { if (listFails && url.includes("/datamates/memory/list")) { return new Response(JSON.stringify({ detail: "boom" }), { status: 500 }) } + if (createFails && method === "POST" && url.includes("/datamates/memory/")) { + return new Response(JSON.stringify({ detail: "boom" }), { status: 500 }) + } const payload = (() => { // Server-side binding lookup, used when no local cache entry exists. if (url.includes("/datamate-project-bindings/by-")) { @@ -164,6 +170,7 @@ beforeEach(() => { captured = [] listResponse = [] listFails = false + createFails = false createResult = [{ id: "mem-new" }] workspaces = [{ id: 42, name: "acme", memory_enabled: true }] serverBinding = null @@ -435,6 +442,16 @@ describe("blockTitle", () => { // ── write path ────────────────────────────────────────────────────────────── describe("mirrorBlock", () => { + test("a failed mirror is counted until the block is sent, so `run` can say it stayed local", async () => { + const before = failedMirrorCount() + createFails = true + await expect(mirrorBlock(block({ id: "unsent" }))).rejects.toThrow() + expect(failedMirrorCount()).toBe(before + 1) + createFails = false + await mirrorBlock(block({ id: "unsent" })) + expect(failedMirrorCount()).toBe(before) + }) + test("flushPendingMirrors waits for a mirror a short-lived process would abandon (#1332)", async () => { // `MemoryStore.write` fires the mirror and forgets it; a one-shot `run` exits // when the turn ends, routinely before the upload lands. The flush holds the diff --git a/packages/opencode/test/cli/run/run-stdin.test.ts b/packages/opencode/test/cli/run/run-stdin.test.ts new file mode 100644 index 0000000000..cd8ebc9e09 --- /dev/null +++ b/packages/opencode/test/cli/run/run-stdin.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, test } from "bun:test" +import { readRunStdin, type StdinSource } from "../../../src/altimate/cli/run-stdin" + +const encoder = new TextEncoder() + +/** A stdin whose stream yields `chunks`, each after `delayMs`, then ends unless `open` is set. */ +function source(opts: { chunks?: string[]; delayMs?: number; open?: boolean; file?: string }): StdinSource & { + cancelled: () => boolean +} { + let cancelled = false + return { + cancelled: () => cancelled, + isFile: () => opts.file !== undefined, + readAll: async () => opts.file ?? "", + stream: () => + new ReadableStream({ + async start(controller) { + for (const chunk of opts.chunks ?? []) { + await Bun.sleep(opts.delayMs ?? 0) + if (cancelled) return + controller.enqueue(encoder.encode(chunk)) + } + if (!opts.open) controller.close() + }, + cancel() { + cancelled = true + }, + }), + } +} + +describe("readRunStdin", () => { + test("reads a file in full without the wait", async () => { + const result = await readRunStdin(source({ file: "from a file\n" }), 10) + expect(result).toEqual({ text: "from a file\n", timedOut: false }) + }) + + test("reads piped text to EOF", async () => { + const result = await readRunStdin(source({ chunks: ["hello ", "world\n"] }), 1000) + expect(result).toEqual({ text: "hello world\n", timedOut: false }) + }) + + test("keeps reading after the first chunk even past the wait", async () => { + const result = await readRunStdin(source({ chunks: ["a", "b", "c"], delayMs: 30 }), 50) + expect(result).toEqual({ text: "abc", timedOut: false }) + }) + + test("an empty stdin that closes returns at once", async () => { + const result = await readRunStdin(source({ chunks: [] }), 1000) + expect(result).toEqual({ text: "", timedOut: false }) + }) + + test("a stdin left open with no data gives up after the wait and releases the stream", async () => { + const stdin = source({ chunks: [], open: true }) + const started = Date.now() + const result = await readRunStdin(stdin, 50) + expect(result).toEqual({ text: "", timedOut: true }) + expect(Date.now() - started).toBeLessThan(1000) + await Bun.sleep(0) + expect(stdin.cancelled()).toBe(true) + }) +}) diff --git a/packages/opencode/test/session/prompt.test.ts b/packages/opencode/test/session/prompt.test.ts index f250e5f098..b69066c2ce 100644 --- a/packages/opencode/test/session/prompt.test.ts +++ b/packages/opencode/test/session/prompt.test.ts @@ -1885,6 +1885,71 @@ it.instance( ), 30_000, ) + +// A linked workspace that had no skills at the last sync leaves no snapshot, only the empty record. The +// first skill added to it must reach the next `run`, which is a fresh process, so that turn waits for the +// sync like a project with a snapshot does, before the turn's message is written. +let emptyRecordFile: string | undefined +it.instance( + "a turn waits for the workspace sync when the linked workspace had no skills at the last sync", + () => + withWorkspacesOn( + Effect.gen(function* () { + const { llm } = yield* useServerConfig(providerCfg) + const { prompt, chat } = yield* boot() + const skillSync = yield* Effect.promise(() => import("../../src/altimate/workspace/skill-sync")) + let landedAt: number | undefined + let messageAt: number | undefined + // `prompt` touches the session right after it creates the user message, so this marks when the + // turn's message was written. + const realTouch = LegacySession.touch + const touch = spyOn(LegacySession, "touch").mockImplementation(((sessionID: SessionID) => { + messageAt ??= performance.now() + return realTouch(sessionID) + }) as typeof LegacySession.touch) + const recent = spyOn(skillSync, "recentlySynced").mockResolvedValue(false) + const sync = spyOn(skillSync, "syncSkills").mockImplementation( + () => + new Promise((resolve) => + setTimeout(() => { + landedAt = performance.now() + resolve({ changed: false, skipped: [] } as unknown as Awaited>) + }, 500), + ), + ) + yield* Effect.gen(function* () { + yield* llm.text("done") + yield* prompt.prompt({ + sessionID: chat.id, agent: "build", model: promptRef, + parts: [{ type: "text", text: "Hello." }], + }) + expect(sync).toHaveBeenCalled() + expect(landedAt).toBeDefined() + expect(messageAt).toBeDefined() + expect(landedAt!).toBeLessThan(messageAt!) + }).pipe( + Effect.ensuring( + Effect.sync(() => { + recent.mockRestore() + sync.mockRestore() + touch.mockRestore() + if (emptyRecordFile) require("fs").rmSync(emptyRecordFile, { force: true }) + }), + ), + ) + }), + ), + { + init: (directory: string) => + Effect.promise(async () => { + const { emptyRecordPath } = await import("../../src/altimate/workspace/empty-record-path") + emptyRecordFile = emptyRecordPath(path.resolve(directory)) + require("fs").mkdirSync(path.dirname(emptyRecordFile), { recursive: true }) + require("fs").writeFileSync(emptyRecordFile, "key\nwrite-id") + }), + }, + 30_000, +) // altimate_change end it.instance.todo( "running task tool preserves metadata after tool-call transition", From 885854dc634561680f1cc3f5d4924b814a807b76 Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 19 Oct 2026 02:19:06 +0530 Subject: [PATCH 02/11] feat: `workspace list`, and a sign-in command that works without a terminal Found running the Claude Code path end to end: from Claude Code's `!` prompt, neither printed setup command worked. `auth login -p altimate-backend` waits at its method picker, and `link` refuses because stdin is not a TTY. Claude also had no way to show the user which workspaces exist. - `altimate-code workspace list [--json]` prints the workspaces the account can link to (id and name), for `link --workspace`. Exits 2 when not signed in, 1 when the list cannot be loaded. - `SIGN_IN_COMMAND` adds `--method "Altimate LLM Gateway"`, which skips the picker, so every "not signed in" message prints a command that works from any shell. - The non-TTY `link` error points at `workspace list`. - Docs: the Claude Code guide and the CLI reference. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011chv9Rmu7jQXokum1WNcs7 --- .../guides/using-with-claude-code.md | 6 +-- docs/docs/usage/cli.md | 1 + packages/opencode/src/cli/cmd/link.ts | 7 ++-- packages/opencode/src/cli/cmd/workspace.ts | 41 +++++++++++++++++-- .../test/cli/workspace-command.test.ts | 21 ++++++++++ 5 files changed, 67 insertions(+), 9 deletions(-) diff --git a/docs/docs/data-engineering/guides/using-with-claude-code.md b/docs/docs/data-engineering/guides/using-with-claude-code.md index 4239b5af90..8526972996 100644 --- a/docs/docs/data-engineering/guides/using-with-claude-code.md +++ b/docs/docs/data-engineering/guides/using-with-claude-code.md @@ -119,10 +119,10 @@ Claude Code reaches a workspace through Altimate Code: every task the plugin han 1. **Sign in to Altimate.** In a terminal: ```bash - altimate-code auth login -p altimate-backend + altimate-code auth login -p altimate-backend --method "Altimate LLM Gateway" ``` - Choose **Altimate LLM Gateway**. It opens your browser; sign in and return to the terminal. (Inside the TUI, `/connect` does the same.) + It opens your browser; sign in and return to the terminal. (Inside the TUI, `/connect` does the same.) The command also works from Claude Code: type it after `!` in the prompt. 2. **Link the project.** In the project folder: @@ -130,7 +130,7 @@ Claude Code reaches a workspace through Altimate Code: every task the plugin han altimate-code link ``` - Pick the workspace, or create one. Linking needs an interactive terminal; from a script or an agent, use `altimate-code link --workspace ` or `altimate-code link --create`. + Pick the workspace, or create one. The picker needs an interactive terminal, so it does not run from Claude Code's `!` prompt or a script. There, list the workspaces with `altimate-code workspace list` and link with `altimate-code link --workspace `, or create one with `altimate-code link --create`. Claude can run these for you: ask it to link the project to a workspace. 3. **Check it.** `altimate-code workspace status` shows the linked workspace and when its skills last synced. diff --git a/docs/docs/usage/cli.md b/docs/docs/usage/cli.md index 8cfce97e50..b18b1a7f52 100644 --- a/docs/docs/usage/cli.md +++ b/docs/docs/usage/cli.md @@ -57,6 +57,7 @@ With workspaces on: - `altimate-code link` links the current project to a workspace (or creates one), then shows what the workspace provides (its skills, knowledge, integrations and whether memory is on) and a few prompts to start with. The sidebar then names the workspace and shows how many memories are not yet synced and when skills last synced. - From a shell, script or CI job, without the TUI: - `altimate-code link --workspace ` links to an existing workspace (an id is matched before a name), and `altimate-code link --create [name]` creates one (named after the repo by default) and links to it. Neither prompts. If the project is already linked to a different workspace they refuse unless `--yes` is passed. Running `--create` again when the project is already linked to a workspace of that name creates nothing, so it is safe in a devcontainer's setup command; if another workspace already has that name it refuses and names its id, unless `--allow-duplicate` is passed. Linking to the workspace the project is already linked to confirms that link on this machine, which a fresh clone needs before `workspace sync` sends its memory. + - `altimate-code workspace list [--json]` lists the workspaces your account can link to, with their ids, for `link --workspace`. - `altimate-code workspace status [--json]` shows the linked workspace and, when known, how many saved memories have not reached it and when skills last synced. - `altimate-code workspace refresh` pulls the workspace's skills (memory is loaded by the next session started in the project); `altimate-code workspace sync` sends memory the workspace has not received; `altimate-code workspace unlink [--yes]` detaches the project (`--yes` is required without a terminal). - These commands accept `--directory`. Exit codes: `0` done, `1` failed (including when the workspace service could not be reached), `2` a request to change (not signed in, a re-link or unlink without `--yes` and no terminal to confirm on, or a `sync` to a link not yet confirmed on this machine), `3` the project is not linked. With `--json`, `ok` is `true` exactly when the exit code is `0`. diff --git a/packages/opencode/src/cli/cmd/link.ts b/packages/opencode/src/cli/cmd/link.ts index 42346bc8da..9d402fd070 100644 --- a/packages/opencode/src/cli/cmd/link.ts +++ b/packages/opencode/src/cli/cmd/link.ts @@ -55,8 +55,9 @@ import { } from "@/altimate/workspace/workspace-name" /** The browser sign-in from a plain shell: the Altimate LLM Gateway method of the `altimate-backend` - * provider saves the credentials every workspace command reads. */ -export const SIGN_IN_COMMAND = "altimate-code auth login -p altimate-backend" + * provider saves the credentials every workspace command reads. `--method` skips the method picker, + * so the command also works where stdin is not a terminal (an agent's shell, Claude Code's `!`). */ +export const SIGN_IN_COMMAND = 'altimate-code auth login -p altimate-backend --method "Altimate LLM Gateway"' const CREATE_NEW_SENTINEL = "__create_new__" const SET_UP_IN_BROWSER_SENTINEL = "__browser_handoff__" @@ -244,7 +245,7 @@ export const LinkCommand = cmd({ "`altimate-code link` needs an interactive terminal (stdin must be a TTY). " + "Run it directly in a shell, use the TUI palette command " + '"Link this project to a workspace", or link without prompts: ' + - "`altimate-code link --workspace ` for an existing workspace, " + + "`altimate-code link --workspace ` for an existing workspace (`altimate-code workspace list` shows them), " + "`altimate-code link --create` for a new one.", ) process.exitCode = 1 diff --git a/packages/opencode/src/cli/cmd/workspace.ts b/packages/opencode/src/cli/cmd/workspace.ts index a6c5c7007d..e7882f1a0c 100644 --- a/packages/opencode/src/cli/cmd/workspace.ts +++ b/packages/opencode/src/cli/cmd/workspace.ts @@ -1,6 +1,6 @@ // altimate_change - new file // -// `altimate-code workspace status|refresh|sync|unlink`: the `/workspace` menu's actions for a shell, +// `altimate-code workspace list|status|refresh|sync|unlink`: the `/workspace` menu's actions for a shell, // so a setup script, a devcontainer, CI or another agent can manage a project's workspace without // the TUI. Thin wrappers over `altimate/workspace/manage.ts` — the same code the menu and the // `serve` routes run — with plain-text output, `--json` for scripts, and exit codes a caller can @@ -17,6 +17,7 @@ import { bootstrap } from "../bootstrap" import { UI } from "../ui" import { SIGN_IN_COMMAND, stripControlChars } from "./link" import { AltimateApi } from "@/altimate/api/client" +import { WorkspaceApi, type DatamateRef } from "@/altimate/workspace/api-client" import * as Manage from "@/altimate/workspace/manage" import { resolveBindingOutcome, type BindingOutcome, type CachedBinding } from "@/altimate/workspace/state" @@ -120,6 +121,7 @@ export function describeRefresh(report: Manage.RefreshReport): { lines: string[] * guards can be checked without a service, a terminal or a project. */ export interface WorkspaceDeps { isConfigured(): Promise + list(): Promise resolve(directory: string): Promise status(directory: string, binding: CachedBinding): Promise refresh(directory: string): Promise @@ -135,6 +137,7 @@ export interface WorkspaceDeps { const defaultDeps: WorkspaceDeps = { isConfigured: () => AltimateApi.isConfigured().catch(() => false), + list: () => WorkspaceApi.listDatamates(), resolve: (directory) => resolveBindingOutcome(directory), status: (directory, binding) => Manage.status(directory, { poll: true, binding }), refresh: (directory) => Manage.refresh(directory), @@ -187,6 +190,26 @@ async function linkOf( } } +/** The workspaces this account can link to, so a caller without a terminal for the `link` picker can + * choose one and pass it to `link --workspace`. */ +export async function runList(json: boolean, deps: WorkspaceDeps = defaultDeps): Promise { + if (!(await deps.isConfigured())) return failure(deps, json, EXIT.USAGE, NOT_SIGNED_IN) + let list: DatamateRef[] + try { + list = await deps.list() + } catch (err) { + return failure(deps, json, EXIT.FAILED, `Could not load workspaces: ${messageOf(err)}`) + } + const workspaces = list.map((w) => ({ id: w.id, name: w.name })) + const lines = workspaces.length + ? [ + ...workspaces.map((w) => `${w.id} ${stripControlChars(w.name)}`), + "Link this project to one with `altimate-code link --workspace `.", + ] + : ["No workspaces yet. Create one for this project with `altimate-code link --create`."] + return report(deps, json, EXIT.OK, { workspaces }, lines) +} + export async function runStatus(directory: string, json: boolean, deps: WorkspaceDeps = defaultDeps): Promise { if (!(await deps.isConfigured())) return failure(deps, json, EXIT.USAGE, NOT_SIGNED_IN) try { @@ -303,6 +326,16 @@ async function inProject(directory: string, json: boolean, run: () => Promise yargs.option("directory", DIRECTORY).option("json", JSON_FLAG), + handler: async (args) => { + const directory = args.directory ?? process.cwd() + await inProject(directory, args.json, () => runList(args.json)) + }, +}) + const StatusCommand = cmd({ command: "status", describe: "show which workspace this project is linked to, and what has not synced", @@ -349,8 +382,10 @@ const UnlinkCommand = cmd({ export const WorkspaceCommand = cmd({ command: "workspace", - describe: "manage this project's Altimate workspace (status, refresh, sync, unlink)", + describe: "manage this project's Altimate workspace (list, status, refresh, sync, unlink)", builder: (yargs) => - yargs.command(StatusCommand).command(RefreshCommand).command(SyncCommand).command(UnlinkCommand).demandCommand(), + yargs + .command(ListCommand) + .command(StatusCommand).command(RefreshCommand).command(SyncCommand).command(UnlinkCommand).demandCommand(), async handler() {}, }) diff --git a/packages/opencode/test/cli/workspace-command.test.ts b/packages/opencode/test/cli/workspace-command.test.ts index fc0ac55432..e316b8dbf1 100644 --- a/packages/opencode/test/cli/workspace-command.test.ts +++ b/packages/opencode/test/cli/workspace-command.test.ts @@ -6,6 +6,7 @@ import { describeRefresh, describeStatus, describeSync, + runList, runRefresh, runStatus, runSync, @@ -131,6 +132,7 @@ function fakeDeps(over: Partial = {}) { const json: any[] = [] const deps: WorkspaceDeps = { isConfigured: async () => true, + list: async () => (calls.push("list"), [{ id: 35, name: "Growth" }, { id: 9, name: "Fin\u001b]8;;x\u0007ance" }]), resolve: async () => ({ status: "bound", binding }), status: async () => (calls.push("status"), statusReport), refresh: async () => (calls.push("refresh"), { skillsChanged: false, skillsSkipped: [], errors: [] } as any), @@ -149,6 +151,7 @@ function fakeDeps(over: Partial = {}) { describe("workspace subcommands", () => { test("not signed in: every subcommand exits 2 and touches nothing", async () => { for (const run of [ + (d: WorkspaceDeps) => runList(false, d), (d: WorkspaceDeps) => runStatus("/p", false, d), (d: WorkspaceDeps) => runRefresh("/p", false, d), (d: WorkspaceDeps) => runSync("/p", false, d), @@ -160,6 +163,24 @@ describe("workspace subcommands", () => { } }) + test("list prints each workspace's id and name, and how to link to one", async () => { + const f = fakeDeps() + expect(await runList(false, f.deps)).toBe(EXIT.OK) + expect(f.out).toEqual(["35 Growth", "9 Fin]8;;xance", "Link this project to one with `altimate-code link --workspace `."]) + const j = fakeDeps() + expect(await runList(true, j.deps)).toBe(EXIT.OK) + expect(j.json[0]).toEqual({ ok: true, workspaces: [{ id: 35, name: "Growth" }, { id: 9, name: "Fin\u001b]8;;x\u0007ance" }] }) + }) + + test("list with no workspaces points at --create; a failed request exits 1", async () => { + const empty = fakeDeps({ list: async () => [] }) + expect(await runList(false, empty.deps)).toBe(EXIT.OK) + expect(empty.out.join("\n")).toContain("link --create") + const down = fakeDeps({ list: async () => Promise.reject(new Error("503")) }) + expect(await runList(true, down.deps)).toBe(EXIT.FAILED) + expect(down.json[0]).toMatchObject({ ok: false, error: "Could not load workspaces: 503" }) + }) + test("a service that cannot be reached is a failure (1), never 'not linked' (3)", async () => { for (const run of [ (d: WorkspaceDeps) => runStatus("/p", true, d), From dfa62c11e6a8bc429ff3919771e0c05dc42563d3 Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 19 Oct 2026 02:37:09 +0530 Subject: [PATCH 03/11] fix: report unsent memory from `workspace status`'s count, not a failure set Review follow-ups. The in-process `failedMirrors` set missed uploads that were deferred, declined or still pending when the exit flush gave up, kept blocks deleted since, and keyed by block id alone. `run` now asks, only when it mirrored something, for the same pending count `workspace status` shows (a local index read), and prints it, or says the count is unknown. - `workspace list` runs without opening a project. - stdin tests: the first chunk arrives at once, so timing margins are wide; the empty-stdin test asserts it returns at once. - Docs: re-linking to a different workspace needs `--yes`. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011chv9Rmu7jQXokum1WNcs7 --- .../guides/using-with-claude-code.md | 2 +- .../src/altimate/cli/unsent-memory.ts | 27 ++++++++++++++++ .../src/altimate/workspace/memory-sync.ts | 31 ++++++++----------- packages/opencode/src/cli/cmd/run.ts | 16 ++++------ packages/opencode/src/cli/cmd/workspace.ts | 6 ++-- .../test/altimate/cli/unsent-memory.test.ts | 22 +++++++++++++ .../altimate/workspace/memory-sync.test.ts | 9 ++---- .../opencode/test/cli/run/run-stdin.test.ts | 13 +++++--- 8 files changed, 83 insertions(+), 43 deletions(-) create mode 100644 packages/opencode/src/altimate/cli/unsent-memory.ts create mode 100644 packages/opencode/test/altimate/cli/unsent-memory.test.ts diff --git a/docs/docs/data-engineering/guides/using-with-claude-code.md b/docs/docs/data-engineering/guides/using-with-claude-code.md index 8526972996..08032f933b 100644 --- a/docs/docs/data-engineering/guides/using-with-claude-code.md +++ b/docs/docs/data-engineering/guides/using-with-claude-code.md @@ -130,7 +130,7 @@ Claude Code reaches a workspace through Altimate Code: every task the plugin han altimate-code link ``` - Pick the workspace, or create one. The picker needs an interactive terminal, so it does not run from Claude Code's `!` prompt or a script. There, list the workspaces with `altimate-code workspace list` and link with `altimate-code link --workspace `, or create one with `altimate-code link --create`. Claude can run these for you: ask it to link the project to a workspace. + Pick the workspace, or create one. The picker needs an interactive terminal, so it does not run from Claude Code's `!` prompt or a script. There, list the workspaces with `altimate-code workspace list` and link with `altimate-code link --workspace `, or create one with `altimate-code link --create`. Moving a linked project to a different workspace also needs `--yes`. Claude can run these for you: ask it to link the project to a workspace. 3. **Check it.** `altimate-code workspace status` shows the linked workspace and when its skills last synced. diff --git a/packages/opencode/src/altimate/cli/unsent-memory.ts b/packages/opencode/src/altimate/cli/unsent-memory.ts new file mode 100644 index 0000000000..1c6dbefef2 --- /dev/null +++ b/packages/opencode/src/altimate/cli/unsent-memory.ts @@ -0,0 +1,27 @@ +// altimate_change - new file +import * as Manage from "@/altimate/workspace/manage" +import { mirrorWasRequested } from "@/altimate/workspace/memory-sync" + +/** + * What `run` prints at exit about memory saved here that the workspace has not received, or null when + * there is nothing to say. `unsynced` is `workspace status`'s count: an upload that failed, was deferred + * or declined, or was still in flight when the exit flush gave up is pending there; a block deleted + * since is not. + */ +export function unsentMemoryNotice(unsynced: number | null): string | null { + if (unsynced === null) + return "Could not confirm that the memory saved here reached the workspace. Run `altimate-code workspace status` to check." + if (unsynced === 0) return null + return ( + `${unsynced === 1 ? "1 memory saved here is" : `${unsynced} memories saved here are`} not in the workspace yet. ` + + `Run \`altimate-code workspace sync\` to send ${unsynced === 1 ? "it" : "them"}.` + ) +} + +/** Counted only when this process saved memory for a linked workspace, so other runs pay nothing. */ +export async function unsentMemoryNoticeAfterRun(directory: string): Promise { + if (!mirrorWasRequested()) return null + const report = await Manage.status(directory) + if (!report.binding || !report.memory) return null + return unsentMemoryNotice(report.memory.unsynced) +} diff --git a/packages/opencode/src/altimate/workspace/memory-sync.ts b/packages/opencode/src/altimate/workspace/memory-sync.ts index 14c790c075..c9a727ae1b 100644 --- a/packages/opencode/src/altimate/workspace/memory-sync.ts +++ b/packages/opencode/src/altimate/workspace/memory-sync.ts @@ -837,16 +837,16 @@ function serialize(scope: "global" | "project", blockId: string, op: () => Pr * `skill-sync.flushPendingSyncs` holds it for a cold skill sync. */ const mirrorsInFlight = syncState.mirrorsInFlight -/** Blocks whose mirror failed in this process, by `/`. Nothing retries them in a one-shot `run` - * (the TUI and `serve` poll; `run` does not), so its exit reports them instead of letting the block stay on - * this machine unannounced. Process-global for the same two-module-graph reason as `syncState`. */ -const failedMirrors: Set = ((globalThis as unknown as Record | undefined>)[ - Symbol.for("altimate.memory-sync.failed-mirrors") -] ??= new Set()) - -/** How many blocks this process saved locally but could not send to the workspace. */ -export function failedMirrorCount(): number { - return failedMirrors.size +/** Whether this process tried to mirror a block to a linked workspace. A one-shot `run` checks it at exit + * and, only then, counts the memory still not in the workspace (`Manage.status`): nothing in `run` retries + * an upload, so without the check a block left behind stays on this machine unannounced. Process-global for + * the same two-module-graph reason as `syncState`. */ +const mirrorRequests: { count: number } = ((globalThis as unknown as Record)[ + Symbol.for("altimate.memory-sync.mirror-requests") +] ??= { count: 0 }) + +export function mirrorWasRequested(): boolean { + return mirrorRequests.count > 0 } /** Hold a task in `mirrorsInFlight` for its lifetime. */ @@ -895,15 +895,10 @@ export async function mirrorBlock(block: MemoryBlock, directory?: string): Promi // binding governs only whether we upload at all. const binding = await currentBinding(directory) if (!binding) return + // Counted before the memory-setting check: a check that fails also leaves the block unsent. + mirrorRequests.count++ if (!(await memoryEnabled(binding))) return - const key = `${block.scope}/${block.id}` - try { - await push(block, binding, undefined, directory) - failedMirrors.delete(key) - } catch (err) { - failedMirrors.add(key) - throw err - } + await push(block, binding, undefined, directory) }) return tracked(task) } diff --git a/packages/opencode/src/cli/cmd/run.ts b/packages/opencode/src/cli/cmd/run.ts index 738596106a..c9eb989a76 100644 --- a/packages/opencode/src/cli/cmd/run.ts +++ b/packages/opencode/src/cli/cmd/run.ts @@ -2108,16 +2108,12 @@ You are speaking to a non-technical business executive. Follow these rules stric .then((m) => m.flushPendingMirrors()) .catch(() => {}), ]) - // A mirror that failed is not retried by this process, and nothing else - // tells the user the memory stayed on this machine. - const unsent = await import("../../altimate/workspace/memory-sync") - .then((m) => m.failedMirrorCount()) - .catch(() => 0) - if (unsent > 0) - process.stderr.write( - `${unsent === 1 ? "1 memory was" : `${unsent} memories were`} saved here but could not be sent to the workspace. ` + - `Run \`altimate-code workspace sync\` to send ${unsent === 1 ? "it" : "them"}.${EOL}`, - ) + // This process does not retry an upload, and nothing else tells the user + // that memory stayed on this machine. + const notice = await import("../../altimate/cli/unsent-memory") + .then((m) => m.unsentMemoryNoticeAfterRun(process.cwd())) + .catch(() => null) + if (notice) process.stderr.write(notice + EOL) } // altimate_change end diff --git a/packages/opencode/src/cli/cmd/workspace.ts b/packages/opencode/src/cli/cmd/workspace.ts index e7882f1a0c..a4ffa99704 100644 --- a/packages/opencode/src/cli/cmd/workspace.ts +++ b/packages/opencode/src/cli/cmd/workspace.ts @@ -326,13 +326,13 @@ async function inProject(directory: string, json: boolean, run: () => Promise yargs.option("directory", DIRECTORY).option("json", JSON_FLAG), + builder: (yargs) => yargs.option("json", JSON_FLAG), handler: async (args) => { - const directory = args.directory ?? process.cwd() - await inProject(directory, args.json, () => runList(args.json)) + process.exitCode = await runList(args.json) }, }) diff --git a/packages/opencode/test/altimate/cli/unsent-memory.test.ts b/packages/opencode/test/altimate/cli/unsent-memory.test.ts new file mode 100644 index 0000000000..28dbd662f5 --- /dev/null +++ b/packages/opencode/test/altimate/cli/unsent-memory.test.ts @@ -0,0 +1,22 @@ +// altimate_change - new file +import { describe, expect, test } from "bun:test" +import { unsentMemoryNotice } from "../../../src/altimate/cli/unsent-memory" + +describe("unsentMemoryNotice", () => { + test("says nothing when every memory reached the workspace", () => { + expect(unsentMemoryNotice(0)).toBeNull() + }) + + test("counts what is pending and points at workspace sync", () => { + expect(unsentMemoryNotice(1)).toBe( + "1 memory saved here is not in the workspace yet. Run `altimate-code workspace sync` to send it.", + ) + expect(unsentMemoryNotice(3)).toBe( + "3 memories saved here are not in the workspace yet. Run `altimate-code workspace sync` to send them.", + ) + }) + + test("an unknown count is reported as unknown, never as zero", () => { + expect(unsentMemoryNotice(null)).toContain("workspace status") + }) +}) diff --git a/packages/opencode/test/altimate/workspace/memory-sync.test.ts b/packages/opencode/test/altimate/workspace/memory-sync.test.ts index 3a00654d56..8b5b52d272 100644 --- a/packages/opencode/test/altimate/workspace/memory-sync.test.ts +++ b/packages/opencode/test/altimate/workspace/memory-sync.test.ts @@ -45,7 +45,7 @@ const { isEnabled, memoryEnabledCached, mirrorBlock, - failedMirrorCount, + mirrorWasRequested, flushPendingMirrors, overlayBlocks, refresh, @@ -442,14 +442,11 @@ describe("blockTitle", () => { // ── write path ────────────────────────────────────────────────────────────── describe("mirrorBlock", () => { - test("a failed mirror is counted until the block is sent, so `run` can say it stayed local", async () => { - const before = failedMirrorCount() + test("a mirror to a linked workspace is recorded, failed or not, so `run`'s exit checks what stayed local", async () => { createFails = true await expect(mirrorBlock(block({ id: "unsent" }))).rejects.toThrow() - expect(failedMirrorCount()).toBe(before + 1) createFails = false - await mirrorBlock(block({ id: "unsent" })) - expect(failedMirrorCount()).toBe(before) + expect(mirrorWasRequested()).toBe(true) }) test("flushPendingMirrors waits for a mirror a short-lived process would abandon (#1332)", async () => { diff --git a/packages/opencode/test/cli/run/run-stdin.test.ts b/packages/opencode/test/cli/run/run-stdin.test.ts index cd8ebc9e09..e0427eb002 100644 --- a/packages/opencode/test/cli/run/run-stdin.test.ts +++ b/packages/opencode/test/cli/run/run-stdin.test.ts @@ -3,7 +3,7 @@ import { readRunStdin, type StdinSource } from "../../../src/altimate/cli/run-st const encoder = new TextEncoder() -/** A stdin whose stream yields `chunks`, each after `delayMs`, then ends unless `open` is set. */ +/** A stdin whose stream yields `chunks` (the first at once, each later one after `delayMs`), then ends unless `open` is set. */ function source(opts: { chunks?: string[]; delayMs?: number; open?: boolean; file?: string }): StdinSource & { cancelled: () => boolean } { @@ -15,8 +15,8 @@ function source(opts: { chunks?: string[]; delayMs?: number; open?: boolean; fil stream: () => new ReadableStream({ async start(controller) { - for (const chunk of opts.chunks ?? []) { - await Bun.sleep(opts.delayMs ?? 0) + for (const [i, chunk] of (opts.chunks ?? []).entries()) { + if (i > 0) await Bun.sleep(opts.delayMs ?? 0) if (cancelled) return controller.enqueue(encoder.encode(chunk)) } @@ -41,13 +41,16 @@ describe("readRunStdin", () => { }) test("keeps reading after the first chunk even past the wait", async () => { - const result = await readRunStdin(source({ chunks: ["a", "b", "c"], delayMs: 30 }), 50) + // The first chunk is immediate, so only the later ones (160ms in all) outlast the 50ms wait. + const result = await readRunStdin(source({ chunks: ["a", "b", "c"], delayMs: 80 }), 50) expect(result).toEqual({ text: "abc", timedOut: false }) }) test("an empty stdin that closes returns at once", async () => { - const result = await readRunStdin(source({ chunks: [] }), 1000) + const started = Date.now() + const result = await readRunStdin(source({ chunks: [] }), 5000) expect(result).toEqual({ text: "", timedOut: false }) + expect(Date.now() - started).toBeLessThan(1000) }) test("a stdin left open with no data gives up after the wait and releases the stream", async () => { From 63f733465cfa24019bec441a790c80351dcc4070 Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 19 Oct 2026 03:13:08 +0530 Subject: [PATCH 04/11] chore: restore `.github/meta` notes deleted by mistake in this branch Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011chv9Rmu7jQXokum1WNcs7 --- .../deterministic-validators-followups.md | 682 ++++++++++++++++++ .github/meta/harness-review-followups.md | 65 ++ .../meta/night-run/MERGE-VALIDATION-MATRIX.md | 47 ++ .../night-run/RELEASE-EVALUATION-BOARD.md | 45 ++ .github/meta/night-run/UPGRADE-PATH-TEST.md | 34 + 5 files changed, 873 insertions(+) create mode 100644 .github/meta/deterministic-validators-followups.md create mode 100644 .github/meta/harness-review-followups.md create mode 100644 .github/meta/night-run/MERGE-VALIDATION-MATRIX.md create mode 100644 .github/meta/night-run/RELEASE-EVALUATION-BOARD.md create mode 100644 .github/meta/night-run/UPGRADE-PATH-TEST.md diff --git a/.github/meta/deterministic-validators-followups.md b/.github/meta/deterministic-validators-followups.md new file mode 100644 index 0000000000..c06572eea5 --- /dev/null +++ b/.github/meta/deterministic-validators-followups.md @@ -0,0 +1,682 @@ +# Completion-gate validators — deferred review findings + +Findings from the review of the deterministic completion-gate validators that are +real but larger than the change they were raised against, plus the ones that were +declined on purpose. Each carries the rationale, so a later pass does not have to +re-derive it. + +Source: review feedback and the end-to-end evidence run on +`feat/deterministic-validators` (2026-08-29). + +--- + +## Deferred — real, but larger than a fix-in-place + +### 1. ~~Custom `model-paths` / `seed-paths` are not honoured~~ — RESOLVED + +`modelsModifiedSince` requires a `models` path segment, and both +`collectProducedNodeNames` and the authored-file scan use a hard-coded directory +list. A project that configures `model-paths: ['analytics']` in `dbt_project.yml` +is invisible to every path-based check in the lane. + +Direction is safe today — the validators under-fire rather than over-fire on such +a project — but the deliverable-names gate can report a name as absent when the +model exists under a custom path, which would block. + +Closed in the consensus pass. `resolveDbtSourcePaths(dbtRoot)` parses +`model-paths`, `seed-paths`, `snapshot-paths`, `analysis-paths`, `macro-paths`, +`test-paths` and `packages-install-path` (plus the pre-1.0 `source-paths` / +`data-paths` spellings), handling inline flow lists, block sequences and bare +scalars. `modelsModifiedSince`, `collectProducedNodeNames` and the authored-file +scan are all driven off it. + +One deliberate carve-out: when the scanned directory has no `dbt_project.yml` +there is nothing to honour, so `modelsModifiedSince` keeps its legacy "any +`models` ancestor" predicate. That keeps the helper usable on a directory that +is not itself a project root. + +### 2. Python models (`.py`) are outside the touched-model set + +`modelsModifiedSince` accepts `.sql` only, so a session that edits +`models/orders.py` produces an empty work list and `dbt-build-green` takes its +`nothing-to-gate` path. + +Why deferred: widening the extension is one line, but the consumers are not +extension-agnostic. `dbt-dialect-guard` and `dbt-incremental-config` would then +run SQL/Jinja regexes over Python source, where `#` is a comment and +`config(materialized=...)` is a `dbt.config()` call — different lexical rules +entirely. The correct shape is a per-consumer file-kind filter, which is a +refactor of the discovery API rather than an added extension. + +### 3. `run_results.json` is trusted as evidence an agent cannot forge + +Nothing stops a session writing a `run_results.json` full of `success` rows +instead of running dbt. Every filesystem-evidence gate in this lane shares that +property. + +Why deferred: closing it means recording dbt invocations from the tool layer and +signing them into the session record — a lane-wide trust model, not a validator +change. Partly mitigated already: build coverage now also reads the model DDL +under `/run/`, so a forgery has to fabricate two artifacts rather than +one. + +### 4. Post-build edit detection is mtime-based, not content-based + +`BUILD_FRESHNESS_TOLERANCE_MS` was raised to 60 s because a formatter or a +trailing-newline fix landing seconds after a green build was blocking sessions. +That trades a false positive for a blind spot: a substantive rewrite inside the +window is not caught. + +The right fix is a content comparison — hash each model at build time and compare +after — which needs a pre-build snapshot the gate does not currently take. Worth +doing when the lane gains a session-scoped artifact store. + +### 5. ~~Compound `{% if is_incremental() and … %}` conditions are not matched~~ — RESOLVED + +Closed in the review sweep. Guard-body extraction was rebuilt on the shared +nesting-aware helper (`extractJinjaIfBlocks` + `jinjaIfBranchHead`) rather than on +a looser regex, which is what this entry said the fix had to wait for. Compound +conditions and nested `{% if %}` / `{% else %}` inside a guard are now handled. +Covered by `review-sweep.test.ts`. + +### 6. ~~`analyses/` counts toward the produced-node inventory~~ — PARTLY RESOLVED + +An `analyses/foo.sql` satisfies a required model named `foo`, even though an +analysis is never materialised as a relation. The requested resource *type* is +also discarded during extraction, so a seed can satisfy a request for a model. + +Half closed in the consensus pass. `analyses/` and `tests/` no longer contribute +to the inventory from either source: the filesystem scan visits only the +configured model/seed/snapshot paths, and manifest nodes are filtered on +`resource_type`. An analysis is compiled but never materialised, so it was never +a relation that could satisfy a deliverable. + +Still open: the requested resource *type* is discarded during extraction, so a +seed can still satisfy a request for a model and vice versa. That needs the noun +carried from the task through to the comparison, changing the +`RequiredDeliverables` shape and the gate's messages. + +### 7. Five copies of the recursive project walker + +`modelsModifiedSince`, `collectProducedNodeNames`, `collectExecutedModelNames`, +`anyAuthoredFileSince` and `projectPrescribesGuards` each carry their own +recurse / skip-hidden / skip-`node_modules` / symlink / depth-cap loop. They have +already drifted (only two follow symlinks; only some skip `target`). + +Why deferred: a shared `walkProject(root, opts)` is a clean refactor but touches +every validator in the lane at once, and doing it in the same change as the +behavioural fixes would make both harder to review. Worth its own change. + +--- + +## Declined — the conservative behaviour is the intended one + +### `IDENTIFIER_RE` requires at least three characters + +Reviewers asked for identifiers of any length so a task requiring `id` is +honoured. Declined: two-character code spans in prose are overwhelmingly not +relation names, and every one that is wrongly accepted becomes a required model +that can never be satisfied. Under-extraction is a miss; over-extraction blocks a +correct session. + +### `hasGuard` accepts any `is_incremental()` occurrence + +Reviewers asked that it require an enclosing `{% if %}`. Declined: a model that +writes `{% set inc = is_incremental() %}{% if inc %}` is correct dbt, and +tightening this creates a new false positive to close a false negative. The +lenient direction is the safe one for a gate that blocks completion. + +### A fresh test-only artifact should hard-fail rather than skip coverage + +Reviewers asked that an artifact containing no model nodes block an edited model. +Declined as stated: `dbt build` followed by `dbt test` is a normal, correct +sequence and leaves exactly that artifact, so blocking on it fires on healthy +sessions. Addressed instead by reading the model DDL under `/run/`, which +a test invocation does not overwrite, and by recording +`verdict: "coverage-inconclusive"` when neither source can speak — so the case is +visible in telemetry rather than silently green. + +### Backslash string escapes in the SQL lexer + +A reviewer asked that `scrubSql` stop treating `\'` as an escaped quote, on the +grounds that none of the target warehouses use backslash as a string-escape +character. Declined: the premise is wrong. Snowflake, BigQuery and Redshift all +support backslash escape sequences in string literals; only DuckDB is +strictly `''`-only. Dropping the branch would mis-lex `'it\'s'` on three of the +four warehouses this lane targets, which is the more common shape than the +literal-trailing-backslash case the reviewer raised. + +### `unique_key` inherited from `dbt_project.yml` + +Full dbt config inheritance is not resolved. Rather than guess, the keyless-upsert +finding is suppressed for the whole project when `dbt_project.yml` mentions +`unique_key` at all. Deliberately blunt: it gives up a true positive in exchange +for never inventing an inconsistency that the merged config does not have. + +--- + +## Deferred — raised in the review sweep, still open + +### 8. `/run/` DDL proves execution, not success + +Build coverage falls back to the model DDL under `/run/` when +`run_results.json` has been overwritten by a later `dbt test`. dbt writes that +DDL *before* the warehouse executes the statement, so it is present for a model +that then failed. A session that runs `dbt build` (a model errors), then +`dbt test`, leaves a failed model with fresh DDL and no failing row in the +surviving artifact, and the gate reports green. + +Why deferred: the obvious narrowing — trust the DDL only when the fresh artifact +carries no model rows at all — breaks the very common `dbt run --select a` then +`dbt run --select b` session, where `a` is covered by DDL alone and the artifact +does carry model rows. That would block healthy sessions, which is the wrong +direction. The real fix is retaining per-invocation run-result history for the +session rather than reading whichever single artifact survived, which is the same +session-scoped artifact store that item 4 needs. + +Partially mitigated in the sweep: staleness is now measured against the DDL's own +mtime rather than the surviving artifact's, so an edit made between the build and +a later test is caught. + +Further mitigated in the consensus pass, and the reasoning above is confirmed +against real dbt 1.8.7: a `dbt build` that errors on a model still leaves that +model's DDL in `/run/`, and a following `dbt test` writes +`run_results.json` with `args.which: "test"` and zero rows. The two evidence +sources are then indistinguishable from a healthy `dbt run` + `dbt test`, so +this still cannot be made to block without firing on correct sessions. + +What did change is the label. Coverage resting on DDL alone now reports +`verdict: "build-unproven"` instead of `fresh-build`. That matters more than it +sounds: the confident verdict was contaminating the shadow telemetry the enable +decision is supposed to rest on, so red builds were being counted as green in +the measurement itself. The gate's pass/fail behaviour is unchanged. + +Closing it properly still needs per-invocation run-result history for the +session — the same session-scoped artifact store item 4 needs. Note the harness +makes this worse on its own: `dbt-tests-pass` spawns `altimate-dbt test` in the +project on every validation pass, so from the second pass onward the surviving +artifact is always a test artifact. + +### 9. Build coverage is keyed on the bare node name, not the package + +In a multi-package project where a dependency and the root project both define +`orders`, a successful `model.dependency.orders` row satisfies coverage for the +local `model.local.orders`. + +Why deferred: matching on the full unique ID means mapping each touched file to +its manifest node via `original_file_path`, which is a new manifest-backed +resolution step rather than a line edit. Much narrower after the sweep: installed +packages are now excluded from the touched-model set, so this needs a genuine +name collision between the root project and a dependency, both selected in the +same session. + +### 10. The dialect guard checks for a guard, not for the *right* guard + +`dbt-dialect-guard` suppresses a warehouse-specific call when it sits anywhere +inside a `{% if … target.type … %}` chain. It does not check that the branch the +call sits in is actually limited to a warehouse that provides the function, so +both of these pass while still breaking on at least one target: + +```jinja +{% if target.type != 'snowflake' %} {{ iff(a, b, c) }} {% endif %} +{% if target.type == 'snowflake' %} … {% else %} safe_cast(x as int) {% endif %} +``` + +Why deferred: closing it needs a mapping from each `DIALECT_FUNCTIONS` entry to +the `target.type` values that provide it, plus evaluation of each branch +condition against that set — `==`, `!=`, `in`, `not in`, and the implicit +complement an `{% else %}` arm carries. That is a feature with its own test +surface, and a half-implementation converts a false negative into a blocking +false positive on correct models. The validator's docstring states the weaker +property it actually checks, so the claim is not overstated in the meantime. + +--- + +## Consensus review (six reviewers, 2026-08-29) — closed in this pass + +Recorded here so a later pass does not re-derive them. Each has a regression +test in `test/altimate/validators/consensus-review.test.ts`. + +- **`dbt compile` artifacts certified builds.** `run_results.json` carries no + statement of which subcommand wrote it, and `dbt compile` emits a full set of + `status: "success"` model rows — verified against real dbt 1.8.7, including + for a model selecting from a non-existent relation. `readRunResults` now reads + `args.which`, and an artifact from a command that executes no model SQL is not + build evidence. An artifact with no `args.which` is still trusted, so the + change is backwards compatible; real dbt always stamps it. +- **`CONFIG_CALL_RE` truncated at the first `)`.** `pre_hook="{{ log_start(run_id) }}"` + ended the non-greedy capture early and silently dropped every argument after + it — reading a correctly-keyed merge model as an unkeyed upsert, and losing + `enabled=false` / `materialized='ephemeral'` exemptions. Replaced with a + paren-depth and quote aware scanner. +- **`dbt-nothing-built` passed on any unrelated edit.** It asked only whether + *anything* had been written under the project, never comparing against + `expectation.required`. A session told to create `fct_orders` cleared it by + touching `macros/helper.sql`. Evidence must now intersect the named + deliverables; the coarse bar is kept only for the opt-in with no named + deliverables. +- **`packages-install-path` was ignored.** The `dbt_packages` / `dbt_modules` + skip was matched on the bare directory name, so a project configuring the + install path sent dependency models through the two *pre-existing* subprocess + validators, and a locally authored directory of that name anywhere in the tree + was wrongly skipped. Now resolved from project config and matched on path. +- **Inactive Jinja exempted live models.** `{% if false %}{{ config(enabled=false) }}` + and the same inside `{% raw %}` read as real exemptions. Both regions are + blanked before config extraction. Limited to these two on purpose — a looser + condition would strip live config and push the gate towards blocking. +- **`insert_overwrite` / `microbatch` were told to add a guard.** Both converge + on re-run by construction, and the prescribed remediation would have changed + what the model does. Now exempt from the guard requirement. +- **`is_incremental()` inside a string literal counted as a guard.** The scan + tested the unmasked source while the file's own comment said otherwise. +- **`[^%]*` in `ownBranchMatches` / `jinjaIfBranchHead`.** A Jinja modulo in a + tag (`{% if loop.index % 2 == 0 %}`) stopped the match dead and lost an arm + from the depth counter. Aligned with `JINJA_IF_OPENER_SOURCE`. +- **Repository text was spliced into the retry prompt.** dbt error messages and + node names were interpolated verbatim into `reason`/`fixHint`, which dispatch + concatenates into a synthetic `role: "user"` turn — so a hostile repo could + place text at instruction position. All untrusted values now go through + `sanitizeForPrompt`. +- **Verdicts that hid a zero-verification pass.** An empty scope and an + exempt-only scope both reported `fresh-build`. Now `nothing-verified` and + `exempt-only`; DDL-only coverage is `build-unproven`; an edit inside the 60 s + grace window is reported in `edited_within_grace` and downgrades the verdict. +- **Unreadable models read as clean.** `dbt-dialect-guard` and + `dbt-incremental-config` now report `models_scanned`, `unreadable_models` and + `coverage_complete` rather than counting an unreadable file as verified. +- **`prompt.ts` claimed shadow mode spawns no subprocesses.** It does. Shadow + suppresses only the retry; every validator runs in full. Comment corrected — + the false claim was load-bearing for "enable in shadow first" advice. + +## Consensus review — open, NOT addressed in this pass + +Ordered by how much they should weigh on an enable decision. + +### 11. Node identity is a bare lowercase name, not a `unique_id` + +Supersedes and widens item 9. Run-result IDs and `/run/` paths are both +reduced to a bare name, discarding package *and* resource type, so +`model.dependency.orders` can satisfy coverage for a local `orders`, and a +snapshot's DDL can stand in for a model's. The honest fix is to resolve each +touched file to its manifest `unique_id` via `original_file_path` and match on +that — a manifest-backed resolution step the lane does not have yet. This is the +single largest remaining source of fabricated coverage. + +### 12. Effective dbt config is never resolved + +`dbt-incremental-config` only sees inline `config()`. A model made incremental +through `dbt_project.yml` or properties YAML is invisible, while any textual +`unique_key:` anywhere in `dbt_project.yml` suppresses the keyless-upsert +finding for every model in the project. dbt supports `unique_key` in SQL, +properties YAML and project config. Needs the effective config for the exact +node from a fresh manifest. + +### 13. `dbt-deliverable-names` mines natural language for a blocking contract + +Assessed by two reviewers as the validator most likely to block correct work. +Code-formatted column names in a Required/Deliverables section become required +models; rename wording can demand both names; negated bullets can demand the +prohibited artifact; `create a model with name fct_orders` is not recognised. +Both contract gates also stop at the first task document that parses any +contract, silently masking a later `REQUIREMENTS.md`. The direction of travel is +an explicit structured contract rather than prose mining, with ambiguous prose +yielding inconclusive instead of a blocking invented requirement. + +### 14. Dialect guard does not check which target owns the function + +Restates item 10, still open, now with a second failure mode: the scrubber does +not handle dollar-quoted strings, so valid text such as `$tag$… iff( …$tag$` +produces a blocking finding. + +### 15. Python models are still outside the touched set + +Item 2, unchanged. Now paired with the `model-paths` fix: discovery honours +custom paths but still accepts `.sql` only, so a `models/orders.py` session takes +the `nothing-to-gate` path. Needs a per-consumer file-kind filter, because the +SQL/Jinja regexes are wrong for Python source. + +### 16. Validator telemetry has no field allowlist + +`details` is attached to telemetry wholesale and includes absolute `dbt_root`, +`task_file` and `run_results_path` plus business model names, against a +telemetry contract that says file paths are not collected. Pre-existing for +`dbt_root`; this lane widens it. Wants an allowlist of bounded counters, +booleans, enum verdicts and durations, with paths dropped or hashed. + +### 17. No whole-check resource budget + +Recursive scanners follow directory symlinks without realpath containment or +cycle detection, several Jinja helpers are near-quadratic on pathological input, +and no deadline is passed through the registry. The 60 s timeout covers +`altimate-dbt` children only, not the in-process checks. + +### 18. Retry budget is shared, and the ceiling is wall-clock, not just waste + +`validatorRetryCount` is one session-scoped counter capped at 3, and the five +new validators register *before* `dbt-schema-verify` / `dbt-tests-pass`. See the +PR discussion for the analysis; not changed here because the counter lives in +`session/prompt.ts`, which this PR otherwise does not touch. The associated +number worth carrying: the two subprocess validators run one `altimate-dbt` +child per touched model at concurrency 4 with a 60 s per-child timeout, across +an initial dispatch plus up to three retries — roughly `480 × ceil(M/4)` seconds +worst case, about 3 h 20 m at 100 touched models. + +--- + +## Second review sweep (2026-08-31) — closed in this pass + +Each has a regression test in `test/altimate/validators/review-sweep-2.test.ts` +that fails against the pre-fix source. + +- **Seed and snapshot builds read as "nothing built".** Narrowing + `MODEL_EXECUTING_DBT_COMMANDS` to `{run, build}` was right for + `dbt-build-green` — a `dbt seed` says nothing about whether an edited model + compiles — but `dbt-nothing-built` reused the same predicate to answer a + different question. A successful `dbt seed` of exactly the required name then + read as no build at all, and the gate blocked a session that had delivered + what it was asked for. Split into `runResultsProducedNodes`. The two must not + be collapsed again: model *coverage* and *deliverable* evidence are different + claims. +- **`data/` was a default seed path.** dbt's default is `seeds/` alone; + `data-paths` is only the pre-1.0 spelling. Listing `data/` unconditionally + made `data/orders.csv` read as a produced, authored node in a project where + dbt will never load it, so both contract gates could accept a required + `orders` deliverable with nothing built. The legacy key is still honoured when + the project actually sets it. +- **A contradictory source config granted an exemption.** A model whose live + config states both `enabled=false` and `enabled=true` satisfied + `sourceExemptsFromRunResults`, which drops it out of scope entirely — so even + a fresh `error` row for it was filed as out-of-scope and the build gate + reported green having checked nothing. Each axis now requires that the source + does not also declare its opposite. +- **The dead-`if` strip blanked live `else` arms.** `stripInactiveJinja` blanked + a `{% if false %}` chain through its `{% endif %}`, taking real `config()` in + the `{% elif %}` / `{% else %}` arms with it — an ephemeral or disabled model + lost its exemption and the build gate demanded a `run_results` row dbt never + writes. Only the constant-false arm is blanked now, depth-counted so a nested + `if` inside it cannot steal the outer `else`. +- **The dialect-guard convention probe still carried `[^%]*`.** The modulo fix + applied to `ownBranchMatches` and `JINJA_IF_OPENER_SOURCE` never reached + `TARGET_TYPE_GUARD_PROBE_RE`, so `{% if n % 2 == 0 and target.type == … %}` + left the project's only guard unrecognised, `appliesTo` returned false, and + the validator silently switched itself off for the whole session. +- **The convention probe scanned hard-coded `models/` and `macros/`.** A project + on `model-paths: ['transform']` or `macro-paths: ['jinja']` found no + convention and disabled the lint, even though the touched-model scan already + honours custom paths. Driven off `resolveDbtSourcePaths` now, matching it. +- **The incremental predicate started at a subquery's `where`.** A clause + keyword anywhere in the arm outranked a leading `and`/`or`, so + `and ts > … (select max(ts) from {{ this }} where ok)` sliced from the *inner* + `where` and never examined the outer high-water-mark comparison. A guard body + that opens with a conjunction is now taken whole; the clause tier still + applies to arms that merely project a boolean, which is what motivated it. +- **`{% if not is_incremental() %}` was read as the incremental arm.** A clock in + that full-refresh branch was reported as blocking non-determinism on a correct + model. Negated arms are now skipped — see the still-open item below for why + they are not swapped for their complement. +- **Project-level YAML keys were matched at any indentation.** `target-path` and + every key `readDbtProjectPathList` reads also matched a same-named key nested + under `vars:`, sending the artifact search and every path-based scan at a + directory the project never configured — which blocks a genuinely green build. + Both are anchored at column 0 now. +- **A workspace-level required file failed the inverse gate.** With the dbt + project nested below the workspace, `dbt-deliverable-names` resolved + `reports/output.yml` from either root and `dbt-nothing-built` checked only + below `dbtRoot`, so a correct session passed one contract gate and was blocked + forever by the other. Both check both roots now. +- **Build staleness was dated from the model's DDL, not build completion.** + `Math.min(fresh.mtimeMs, ddlMtime)` always resolved to the DDL, because a + model with a status row was covered by that artifact's own invocation and dbt + writes per-model DDL as it walks the DAG. The 60 s tolerance therefore started + ticking at compile time, and a tidy-up edit seconds after a long green build + read as stale. `fresh.mtimeMs` is correct when a status row exists; DDL mtime + remains correct for the DDL-only case. +- **`update` / `modify` / `change` did not state a contract.** The most ordinary + phrasing for work on an existing relation ("Update the model `orders`") named + its deliverable literally and yielded nothing, so both contract gates went + inapplicable and a zero-write session finished clean. Added with the same + bounded inflections the other modification verbs use. +- **An unreadable `ALTIMATE_VALIDATORS_TASK_FILE` erased the contract.** A stale + or misspelled pin returned no task document at all rather than falling back to + discovery, and all three contract gates skipped in silence. A readable pin is + still exclusive. +- **Repository text reached the retry prompt unsanitised.** The consensus pass + routed `dbt-build-green`'s untrusted values through `sanitizeForPrompt` but + left the same exposure in three places: `dbt-nothing-built` interpolated + `expectation.taskFile` and the parsed deliverable list into `reason` and + `fixHint`, and `dbt-incremental-config` and `dbt-dialect-guard` interpolated + repository-derived model names. All build the same synthetic `role: "user"` + turn, so a name or filename carrying a newline breaks out of the sentence it + is quoted in and lands at instruction position. All now sanitized. +- **The inverse gate misdescribed an unrelated build.** A session that built + something other than the named deliverables was told it "produced no fresh + successful build artifact", which is simply false. It now branches on + `freshRun`. + +## Second review sweep — open, NOT addressed in this pass + +### 19. Both contract gates read the task document from the mutable worktree + +`dbt-deliverable-names` and `dbt-nothing-built` each call `readContract` / +`artifactExpectation` at completion time, against the live filesystem. A session +that deletes, renames or rewrites `TASK.md` so it no longer parses a contract +makes both gates return `appliesTo === false`, and can then terminate having +produced nothing — every remaining gate sees zero touched models. + +Why deferred: the honest fix is to capture the contract at session start, or to +read it from harness state the agent cannot write. Neither exists in this lane; +`ValidatorContext` carries `sessionStartMs` and nothing else. The available +half-measure — block when a task-document candidate path was written during the +session — cannot tell a deletion from a file that never existed, and fires on +the entirely normal case of a task that asks for the document itself to be +updated. That converts a false negative into a blocking false positive, which is +worse than the gap. Same missing infrastructure as items 4 and 8: a +session-scoped artifact store. + +### 20. The dialect guard's applicability is decided from the mutated filesystem + +`projectPrescribesGuards` runs at completion time. A session that removes the +project's only `target.type` guard and leaves an unguarded `iff()` observes no +remaining convention, so the validator makes itself inapplicable — silently +accepting exactly the portability regression it exists to catch. + +Why deferred: same as item 19. Applicability has to be determined from +pre-session project state, and there is nowhere to record it. + +### 21. Prose mining still over-extracts in two shapes + +Both are instances of item 13, restated with the specific shapes reviewers +found, so a later pass does not have to re-derive them: + +- `requirementHead` returns text from the start of the line, so code spans + *before* the requirement verb are collected. "Using the source `raw_orders`, + create the model `fct_orders`" demands `raw_orders` as a model, and no action + the agent can take satisfies it. Slicing from the verb instead trades this for + a passive-voice miss ("The `fct_orders` model must be created"), which is the + safer direction but is a behaviour change to the extractor, not a repair. +- A `## Required deliverables` section collects every code span under it, so + `- Model \`fct_orders\` with unique key \`order_id\`` makes `order_id` a + required model. The qualifier narrowing that fixed this for prose requirement + lines has no equivalent for section entries, which are structurally a list + rather than a sentence. + +The recorded direction of travel for item 13 is an explicit structured contract +with ambiguous prose yielding inconclusive, rather than a further round of +heuristics on top of these. Patching either shape in isolation keeps the +extractor on the treadmill this entry exists to get it off. + +### 22. `{% if not is_incremental() %}` arms are skipped, not complemented + +The negated form no longer produces a false positive on the full-refresh arm, +but the *real* incremental arm — the `{% else %}` — is still not inspected, so a +non-deterministic predicate there is missed. Identifying it means evaluating the +complement of an arbitrary Jinja condition across `elif` chains, which is the +same branch-semantics feature items 10 and 14 need, with the same property: a +half-implementation turns this miss into a blocking false positive on correct +models. + +### 23. `dbt-build-green` cannot see a model that was deleted + +`modelsModifiedSince` walks the CURRENT filesystem for `.sql` files with +`mtime >= sessionStartMs`. A file that existed at session start and was +deleted during the session cannot appear in that scan by construction — there +is no path to stat. With no fresh artifact either, `touchedPaths.length === 0 +&& !artifactIsFresh` takes the `nothing-to-gate` path, so a session that +deletes a model — leaving every downstream `ref()` to it broken — clears this +gate, `dbt-deliverable-names`, and `dbt-nothing-built` (which only checks +POSITIVE evidence of authored/built work) without a single one of them +inspecting the deletion. + +Why deferred: closing this needs to know the workspace's file listing AT +SESSION START, to diff against the current one. Two ways to get that, both +infrastructure this lane does not have: + +- A session-scoped snapshot written by the harness before the agent's first + turn — the same missing artifact store items 19 and 20 need, generalized + from "the task contract" to "the file tree". +- A VCS diff (`git diff --name-status --diff-filter=D` against a commit + bracketing the session start). This validator is deliberately built with + "No subprocess, no warehouse connection" as an architectural constraint (see + its own module doc), specifically so a completion check cannot itself become + a source of flakiness or a new external dependency in a path that gates + session termination. Adding a git subprocess call here is exactly the kind + of dependency that constraint rules out, and it degrades silently to + "no signal" the moment the workspace is not a git repository, or the + session's edits were never committed — the common case for an in-progress + session — leaving the same blind spot with an added dependency and no + reliable win. + +No half-measure was found that narrows this without either reintroducing a +subprocess dependency the validator's own design forbids, or trading this +false negative for a blocking false positive on a correct session (the same +pattern items 19–20 already ruled out for the same reason). Left open with the +reasoning recorded rather than patched. + +--- + +## REQUIRED precondition before enforcement beyond shadow mode + +**`ALTIMATE_VALIDATORS_ENABLED` must not be turned on beyond shadow mode +(`ALTIMATE_VALIDATORS_SHADOW`) until the duplicated heuristics below are +consolidated into a single shared, tested primitive per concern.** Default-off +and shadow are fine to ship as-is; enforcement is not, until this is done. + +### The evidence + +PR #1175 went through four consecutive review-fix rounds after the initial +consensus pass (3d2f6e96be → 631be58f44 → 677b00bb19 → fc64a3619d, plus the +false-positive fix that landed after 677b00bb19's round-4 findings). Each +round fixed real, adversarially-tested bugs — but the SAME underlying defect +kept reappearing in a sibling validator or a sibling code path within the same +file, because the fix for it existed only where it was first found rather than +in one shared place: + +- **Bare-name vs. full-identity matching.** Fixed for `dbt-build-green`'s + status lookup, then separately for its failure-classification path, then + found again (round 4, open) in `dbt-build-green`'s manifest-exemption + lookup — three instances of the identical bug (a dependency package's node + standing in for the root project's node of the same bare name) in one file. +- **Extension/source-dir filtering.** Fixed for `collectProducedNodeNames`, + then separately for `dbt-nothing-built`'s authored-work scan — two + independent implementations of "only `.sql`/`.py` under + models/snapshots, only `.csv` under seeds." +- **Package-directory exclusion.** Fixed for `dbt-dialect-guard`'s convention + probe, then separately for `collectProducedNodeNames`'s manifest loop, then + found again (round 4, open) for `dbt-build-green`'s manifest-exemption + lookup — the same "is this node under `dbt_packages/`" check, written three + times. +- **Conditional-config resolution.** `readConfigAxes` needed its own + unresolved-`{% if %}` guard; `dbt-incremental-config`'s idempotency-scope + parsing has an open, structurally identical gap (round 4: an inline + non-model code span silently marks a demand as model-scoped and skips + enforcement) because it re-implements code-span/verb parsing independently + of `extractRequiredDeliverables`. +- **File-vs-model modification tracking.** Added for model names + (`modificationModels`), then separately for file paths + (`modificationFiles`), and the false-positive fixed in this pass (a + modification file outside the scanned dbt source paths could never be + satisfied) was a direct consequence of the file-path variant being a + parallel, incomplete copy of the model variant rather than sharing one + evidence primitive. + +Round 4 surfaced 11 more findings after the false-positive fix; 2 were the +false positive itself (now fixed), the other 9 are each **another instance of +one of the five duplicated-heuristic categories above** (recorded, not fixed, +per the hard-stop decision below) — that recurrence, not any single bug, is +the actual finding. + +### Why this gates enforcement specifically + +Shadow mode only measures; a false positive there is a data point. Enabled +mode blocks session termination on a false positive — the failure mode that +erodes trust in the gate fastest, and the one class of bug (file-vs-model +modification tracking, above) that round 4 actually produced. Per-validator +duplication has now demonstrably reintroduced false positives once already; +without consolidation there is no reason to expect the next duplicated copy +(idempotency scoping, manifest exemptions, the two round-4 findings already +identified as open) won't do the same the next time someone edits one of the +five copies and not the others. + +### What "consolidated" means here + +One shared, unit-tested module per concern, imported by every validator that +needs it, replacing the current copies: +- a single node-identity resolver (full manifest `unique_id` first, bare-name + fallback only when unresolved) used everywhere a validator matches a + touched file to a run-result row or a manifest node; +- a single source-directory/extension classifier used everywhere a validator + decides whether a file counts as a model/seed/snapshot; +- a single package-directory exclusion check used everywhere a validator + walks the manifest or the filesystem for project-owned nodes; +- a single conditional-config resolver (config value plus "was this call + inside an unresolved `{% if %}`") used by every reader of `config()` state; +- a single modification-vs-creation evidence primitive that takes a required + name OR path and returns "needs session evidence" vs. "existence suffices," + used by both the model and file branches of `dbt-nothing-built`. + +### Code pointer + +See the comment beside the `ALTIMATE_VALIDATORS_ENABLED` read in +`packages/opencode/src/session/prompt.ts` — it points back here. + +### Round-4 findings deferred, not fixed (hard stop after the false-positive fix) + +Recorded so the consolidation work above has a concrete list to close against, +rather than re-deriving it: + +1. `dbt-incremental-config.ts` — an idempotency-scope demand line whose inline + code span names a term/function rather than a model is read as scoped and + silently drops enforcement instead of falling back to workspace-wide. +2. `validator-utils.ts` (`extractRequiredDeliverables`) — a requirement line + containing both a creation and a modification clause records only the + first verb, so the modification set can miss a real modification target + named later on the same line. +3. `validator-utils.ts` (`sanitizeTelemetryDetails`) — a relative path nested + inside an object under a path-bearing key is not redacted; only top-level + array/primitive entries under the key inherit the "this is a path field" + context. +4. `dbt-nothing-built.ts` — modification contracts are only read from the + FIRST task document that names deliverables, not merged across every + document the way `dbt-deliverable-names` already does (item resolved for + that file in the third sweep; the equivalent merge was never applied to + `dbt-nothing-built`'s `artifactExpectation`). +5. `dbt-nothing-built.ts` — `matchedDeliverables.length > 0` is satisfied by + ANY one matched item, so a task requesting updates to multiple models/files + passes after only one of them has session evidence. +6. `dbt-build-green.ts` — `exemptFromManifest` (ephemeral/disabled) is still + looked up by bare name; a dependency's ephemeral/disabled `orders` can + exempt the root project's distinct `orders` from the coverage assertion. + The status and failure-classification lookups were already moved to full + manifest identity in this pass; this is the third instance of the same + bare-name gap in the same file. +7. `dbt-incremental-config.ts` — a path-scoped idempotency demand + (`` Make the model `models/orders.sql` idempotent ``) stores the full path + as the scoped name, which never matches the bare model name looked up at + enforcement time, so the scope silently never applies. +8. `validator-utils.ts` (`resolveWithinRoot`) — resolves the real path of the + candidate's containing DIRECTORY (closing the round-3 directory-symlink + finding) but not the candidate FILE itself; a required path that is + directly a symlink to a file outside the root still passes. +9. `validator-utils.ts` (dialect-guard masking) — a BigQuery `#`-style line + comment containing a curated function name is not stripped before + matching, since `stripSqlComments` only recognises `--` and `/* */`. diff --git a/.github/meta/harness-review-followups.md b/.github/meta/harness-review-followups.md new file mode 100644 index 0000000000..b742a46e80 --- /dev/null +++ b/.github/meta/harness-review-followups.md @@ -0,0 +1,65 @@ +# Harness reliability review — deferred follow-ups + +Deferred MED findings from the pre-PR release review (pre-PR adversarial review). +All 5 HIGH findings plus selected MED/LOW items were fixed on this branch; the +items below were explicitly deferred and are listed verbatim from the review. + +[MED] packages/opencode/src/tool/truncation.ts:66 — the plain-async truncation path hardcodes 2,000 lines/50KiB while the Effect wrapper honors `tool_output` configuration — MCP output through `prompt.ts` therefore ignores user caps despite the shared-core claim — consolidate the wrappers or pass the resolved configuration through both, with parity tests. + +[MED] packages/opencode/src/session/compaction.ts:663 (renderCarryAnchors) — carry-anchor trimming stops when one item remains — one oversized model-generated "Accomplished" item defeats `maxTokens` and can undo compaction — permit dropping or truncating the final item and assert the rendered result satisfies the cap. + +[MED] packages/opencode/src/cli/cmd/idle-done.ts:157 — every command not recognized as read-only is treated as verification — an exit-zero install, cleanup, deployment, or arbitrary unknown command can satisfy the "green verify" precondition and trigger a false completion challenge — require configured or positively classified verification evidence; unknown commands should be ineligible. + +[MED] packages/opencode/src/session/compaction.ts:70 — observation masks retain the first 80 characters of pruned output, while the ledger retains raw command/path/pattern text — credentials, authorization headers, query data, and signed URLs can survive pruning and be recopied into later synthetic prompts — retain only allowlisted metadata or hashes and apply shared secret redaction. + +[MED] packages/opencode/src/session/compaction.ts:572 (renderLedger) — ledger capping repeatedly joins and re-estimates the whole array while removing one line at a time, after collecting the full session history — this is quadratic in unique writes and adds latency at the critical compaction path — bound collection early and trim using accumulated token costs or a single cutoff search. + +The following items from a later review pass were also considered and deliberately +deferred (no behavior change on this branch): + +[LOW] packages/opencode/src/cli/cmd/run/run-mode.ts — the run entrypoint writes its process-scoped mode marker into the environment for the process lifetime, and child processes inherit it; a nested interactive server launched from such a session would arm run-mode mechanisms for genuinely interactive clients — clear or scope the marker in the interactive entrypoints (mirroring the existing child-env cleanup for the sibling non-interactive marker). + +[LOW] packages/opencode/src/session/compaction.ts — module-level per-session pin state and per-session tracker read maps grow without bound in a long-lived server process (the pin map has no production eviction path; per-session read tracking keeps one entry per unique path for the session's lifetime) — bound both with the same LRU pattern now used by the session-state stores. + +[LOW] packages/opencode/src/session/prompt.ts — the post-compaction pinned-task reminder re-derives its source by streaming the FULL session history from the database on every generation once a session has compacted — cache the resolved pin source per session or query only the needed boundary messages. + +[LOW] packages/opencode/src/session/termination.ts — the post-compaction three-option completion nudge (including the completion-token instruction) is injected in ALL modes; interactive users can see an occasional bare completion token line with no interactive function — mode-gate the nudge text or document the cosmetic change. + +The following items came from an external multi-reviewer pass over this branch and +were deliberately deferred (no behavior change on this branch): + +[MED] packages/opencode/src/cli/cmd/run.ts — a prompt retry after a client timeout or connection reset can hit the server AFTER it accepted the original POST, sending the same task again and creating a second user message / duplicate execution — the retry loop has no way to tell "not yet accepted" from "accepted, response lost." Needs a stable idempotency/message key the server honors, or a way to confirm the first attempt was never accepted before retrying. + +[MED] packages/opencode/src/session/compaction.ts (fitHead) — the summarization-request budget reserves a fixed 2,000 tokens for the summary prompt, but the actual assembled prompt (default template + carry anchors + pin-summary addition + first-person reframe, or a plugin-supplied override) can exceed that on an active session — size the reservation from the actual assembled prompt instead of a constant. + +[MED] packages/opencode/src/session/compaction.ts (buildLedger) — on a session's second or later auto-compaction, the ledger is built from the already-filtered/compacted message view, not the full session stream, so verified-write facts from before the first compaction silently drop out of later ledgers — build from the full stream and let selection filter afterward. + +[LOW] packages/opencode/src/session/prompt.ts (uncountedTail) — the proactive overflow estimate sums tool-result tokens on messages AFTER the last-finished assistant message, but a tool call and its result can live on that SAME message when the turn is still mid-flight — those results are excluded from the estimate, so compaction can fire a step later than it should on a heavy tool-output turn. + +[LOW] packages/opencode/src/tool/truncate-core.ts (preview, middle direction) — a degenerate `maxBytes: 1` config (no realistic caller sets this) can still allocate one byte to each of the head/tail halves and exceed the byte budget by a small margin; the equivalent `maxLines: 1` case was already fixed by degrading to tail-only — extend the same degrade to the byte-only case. + +[LOW] packages/opencode/src/session/{termination,nudge,tool-result-cap}.ts, packages/opencode/src/cli/cmd/{run-accounting,idle-done}.ts — these 5 new modules use `export namespace` for organization, which the repo's module-shape convention (`packages/opencode/AGENTS.md`) asks new code to avoid in favor of flat exports + a bottom-of-file self-reexport. ~69 pre-existing files in the package already use the same pattern, so this is consistent with existing debt rather than a regression; fold into a holistic namespace-to-flat-exports cleanup across the package rather than converting these 5 files in isolation. + +[LOW] packages/opencode/test/session/starvation.test.ts — the run-mode "armed" gate test re-implements the gate expression as a local helper instead of importing the real predicate from processor.ts, so a future change to the actual gate (added condition, reordered precedence, renamed exemption) would not be caught by this test — extract the gate into a shared, directly-testable predicate. + +The following items came from a further multi-reviewer pass over this branch and +were deliberately deferred (no behavior change on this branch): + +[MED] packages/opencode/src/session/llm.ts (addHistoricalToolStubs) — the stub-injection skip was narrowed to `toolChoice === "none" && no tools`, which closes the normal-turn regression, but the compaction summarizer still takes that path with a head that references historical tool calls. The skip rests on the claim that omitting both `tools` and `tool_choice` is universally accepted; the issue the stubs exist to fix concerned validation of referenced historical tool calls, which is orthogonal. Verify against the provider that originally needed the stubs before changing anything — reintroducing stubs here would advertise callable tools on a text-only call, so this is a provider-compatibility question, not a code cleanup. + +[MED] packages/opencode/src/session/starvation.ts (repeatSignature / consecutive-signature counter) — the repeat signature folds the failure message in but the counter accumulates for successful calls too, so three identical SUCCESSFUL calls (re-running the same verification command, polling a status probe) reach the threshold and produce a directive asserting that repeating the call cannot change the result, which is untrue for a polling or verification call. Restricting accumulation to calls that carry a failure message narrows the detector's semantics and its overlap with the identical-args doom-loop ladder; make that change with validation data rather than in review, and note the directive text is prompt-visible. + +[LOW] packages/opencode/src/cli/cmd/run-accounting.ts (termination) — `why_model_stopped` falls back to "stop" when no step-finish reason was ever recorded (an aborted run that never completed a step), so the run record attributes an ordinary stop to a session that never reported one. Distinguishing it needs a new value in the published record's enum, which is an output-contract change for downstream consumers — batch it with the next deliberate revision of the run record schema. + +[LOW] packages/opencode/src/session/prompt.ts (post-compaction pin reminder) and packages/opencode/test/cli/idle-done.test.ts — the idle-done unit tests now pin production event ordering (step-finish part before the step's snapshot patch part) in two dedicated cases, but the shared fixtures still emit the patch first; converge the remaining fixtures on production ordering when the file is next touched. + +The following items came from the fourth review pass over this branch and were +deliberately deferred (no behavior change on this branch): + +[MED] packages/opencode/src/session/processor.ts (dispatch tool-result cap) — the per-result hard cap is applied on the successful `tool-result` branch only. A `tool-error` still persists an unbounded error string, and an interrupted running tool keeps partial output in metadata that `message-v2.ts` later replays as a tool result, so a failed call with very large stderr can still overflow the next request. Applying the cap to error text and interrupted partial output changes what gets PERSISTED on the failure path, which is where diagnostics come from — size it against real failure payloads before truncating them. + +[MED] packages/opencode/src/cli/cmd/idle-done.ts (verify classification) — with no configured verify command, ANY non-read-only bash command is treated as a verification, so a mutating command that is not on the mutating-heads list (a build script that also writes generated sources, say) can register as the green verify it is not. The mutation watermark now advances for the in-place, redirection, and known-mutating-head forms, but the underlying "not read-only implies verification" inference still needs replacing with an explicit verify classifier. + +[LOW] packages/opencode/src/cli/cmd/run.ts (--max-turns) — `--max-turns 0` disables the limit entirely because the guard is a truthiness check, and negative or fractional values are accepted without validation. For a governance control an explicit zero should not mean unlimited; fix it together with the CLI validation pass that gives the other numeric options explicit range errors, so the behavior change is documented in one place. + +[LOW] packages/opencode/src/tool/bash.ts (child env) — `ALTIMATE_RUN_RESUMED` joins `ALTIMATE_RUN_MODE` as a marker that nested processes inherit from the bash tool's merged env. Its leak direction is benign (a nested run would use interactive pin selection), but it belongs in the same holistic decision about which run-scoped markers get stripped for child processes. diff --git a/.github/meta/night-run/MERGE-VALIDATION-MATRIX.md b/.github/meta/night-run/MERGE-VALIDATION-MATRIX.md new file mode 100644 index 0000000000..989931d67e --- /dev/null +++ b/.github/meta/night-run/MERGE-VALIDATION-MATRIX.md @@ -0,0 +1,47 @@ +# Comprehensive merge validation — upstream v1.4.0→v1.17.9 bridge (PR #964) + +Method: not spot-checks. The merge surface (3,254 upstream commits) reduces to a BOUNDED, +enumerable set validated by three contracts, each covered exhaustively. + +## Surface (measured) +- 226 upstream-shared source files modified by the merge (exist in both v1.17.9 and HEAD) +- 293 fork-only source files +- 106 fork "altimate_change" behaviors present on main whose marker text wasn't found verbatim on HEAD + +## Coverage matrix + +| Contract | Question | Method | Result | +|---|---|---|---| +| C1: no dropped upstream code | did conflict-resolution revert upstream fixes? | 226 files reviewed by 11 subsystem agents; diff v1.17.9↔HEAD each | No ship-blocker. ~6 real MED/LOW refinement drops (below). High-value upstream fixes (Plan-Mode security, models-cache recovery, session-metadata migration identity, corrupt-message handler) present VERBATIM. | +| C2: no fork-feature loss | did fork customizations survive? | 106 candidate behaviors verified present-on-HEAD or dropped | Most false (path-restructure moved cli/cmd/tui→tui/src + reworded). Real drops: beginner-tips onboarding, internal-url inconsistency, minor branding. | +| C3: behavior | does the artifact work? | full suite + schema + upgrade + journey + real binary + CI | 10,555 tests pass (2 flaky, pass isolated); 19/19 schema tables parity; upgrade 16/16; journey 10 green; CI functional gates green. | + +## REAL findings (all MED/LOW — none block ship), fork-wanted, verified by lead + +| # | file | issue | sev | fix | +|---|------|-------|-----|-----| +| 1 | session/compaction.ts | tail-preserving compaction dead: consumers read `tail_start_id` (session.ts:775, message-v2.ts:1016) but no writer sets it on new compaction parts | MED | restore the writer that stamps `tail_start_id` when creating the compaction part | +| 2 | share/share-next.ts:152 | dropped `Effect.catchCause` on fullSync+flush → unhandled rejection on share-sync failure | MED | wrap fullSync/flush failures (log, don't throw) | +| 3 | session/llm.ts:214 | Copilot billing split-brain: `includeRawChunks` absent from streamText, but `copilotTotalNanoAiu`/`totalNanoAiu` consumers depend on it → Copilot billing metadata never populates | MED | pass `includeRawChunks: true` for the Copilot provider path | +| 4 | tui first-run (tips) | fork onboarding UX dropped: main had `BEGINNER_TIPS`/`isFirstTime`; HEAD has zero refs | MED | restore beginner-tips/first-time hint | +| 5 | util/filesystem.ts:131 | Windows path conversion lost: HEAD does only `realpathSync.native`; upstream also did `windowsPath()` (cygwin/gitbash/WSL) | LOW | restore windowsPath() normalization for win32 | +| 6 | session/retry.ts:71 | drops upstream's extra "retry 5xx even if SDK says non-retryable" guard | LOW | re-add the status-code 5xx retry guard | +| 7 | cli/cmd/mcp.ts:419 | CLI MCP installs default to opencode.json not altimate-code.json (both still discovered) | LOW | prefer altimate-code.json in CONFIG_FILENAMES | +| 8 | tui.ts:166 / runtime.ts:737 | internal worker url `opencode.internal` inconsistent with `altimate-code.internal` elsewhere (internal routing key, not user-visible) | LOW | unify the virtual-host string | +| — | footer/uninstall/upgrade/initialize.txt | unmarked fork branding + 1-2 "OpenCode config" strings | LOW | marker hygiene + branding fix | + +## INTENTIONAL divergences (verified NOT bugs — fork deliberately differs) +- task.ts background subagents — marker: "hides disabled background mode" +- providers Alibaba/Venice/Azure-workflow/GitLab-Duo — moved to packages/core/src/plugin/provider/* or not shipped +- run.ts interactive/replay/demo — fork ships its own run command (executive/analyst modes, max-turns, tracing) +- plugin/index.ts loader + omitted upstream provider plugins — fork ships its own provider set +- server.ts move-session — actually PRESENT via httpapi control-plane group (false positive) + +## FALSE positives (verified) +- agent.ts:524 safety-denial re-append — intended fail-safe (marker documents it) + +## Verdict +Comprehensive review of the ENTIRE merge surface found NO ship-blocker (no crash, data-loss, or +security regression). ~6 medium + a few low fork-wanted refinements to restore. The merge correctly +adopted upstream v1.17.9 (fixes present verbatim, schema parity, upgrade path clean) and preserved +the fork (registries, agents, tools, DE features intact). diff --git a/.github/meta/night-run/RELEASE-EVALUATION-BOARD.md b/.github/meta/night-run/RELEASE-EVALUATION-BOARD.md new file mode 100644 index 0000000000..41013506cf --- /dev/null +++ b/.github/meta/night-run/RELEASE-EVALUATION-BOARD.md @@ -0,0 +1,45 @@ +# Release-Evaluation Board — final pre-release analysis (PR #964, v1.17.9 bridge) +7 independent perspectives (3 codex / 3 sonnet / 1 haiku) + Fable runtime probe + synthesis. +GOAL: dual preservation — don't lose FORK work, don't lose UPSTREAM work. + +## Scorecard + +| Perspective | Verdict | Findings | +|---|---|---| +| D Fork-feature preservation | **PASS** | All tools (101→102 superset), 9 agents, 13 warehouse drivers, 7 builtin skills, 8 commands, memory/tracing/telemetry/PII — present + WIRED. 0 new fork drops. | +| F Safety/security regression | **PASS** | All 5 shipped safety properties INTACT: DDL non-overridable, subagent deny-inheritance (#26597), sensitive-write #209 guard (edit.ts+write.ts), secret redaction, basic-auth username. No regression. | +| A Upstream-fix adoption (73 sampled) | 5 dropped | 1 HIGH-ish (processor providerExecuted), 4 MED/LOW upstream fixes reverted. | +| B Upgrade/format compat | 2 real | managed-TUI-config load dropped (MED), .json/.jsonc precedence inverted (LOW). Auth-username & thinking-default changes INTENTIONAL. DB/auth/session/cache formats back-compatible. | +| C Build/packaging/distribution | **1 HIGH ship-blocker** | release.yml pinned Bun 1.3.10 but build scripts require ^1.3.14 → tag releases fail. FIXED. | +| E Integration seams | 1 real | Azure/DigitalOcean/xAI upstream auth plugins in-tree but unwired in INTERNAL_PLUGINS (MED). Codex plugin = intentional fork keep. | +| G Branding | LOW polish | `.opencode/` paths in warehouse-list/skill-ops messages + `opencode.json` in provider dialogs should say altimate-code. Provider labels & auth-username default = intentional. | +| Fable runtime probe | **PASS** | Compiled binary: palette, agent-switch, shell-mode EXECUTE, /skills list, prompt round-trip all work — empirically confirms the resolved V2-premise review comments don't affect the shipped product. | + +## Actions taken (all upstream-fix restorations, marked + tested) +- FIXED (HIGH ship-blocker): release.yml Bun 1.3.10 → 1.3.14 (all 4 pins). +- FIXED (Fable): run.ts in-process auth header (local run 401'd when OPENCODE_SERVER_PASSWORD set). +- FIXED (delegated): provider.ts Cloudflare unified apiKey; transform.ts Devstral toLowerCase; project.ts session-migration time_updated preservation (3 sites); config/tui.ts managed-config load + paths.ts json/jsonc precedence; plugin/index.ts wire Azure/DigitalOcean/xAI auth plugins. + +## Flagged follow-up (NOT fixed — deliberate) +- processor.ts `providerExecuted` handling: shipped session doesn't use the provider-executed-tool flag (adapters capture it). MED — server-side provider tools (e.g. web search) not handled optimally. NOT fixed pre-release: the upstream change is entangled with GitLab-Duo approval code the fork doesn't ship; porting it blind risks core session processing. Needs a scoped follow-up. +- Branding LOW polish: optional, non-blocking. + +## Dual-preservation verdict +- FORK preservation: CLEAN (D + F pass; nothing we built is lost or weakened). +- UPSTREAM preservation: the gaps were HERE — 5 dropped fixes + 3 unwired auth plugins + managed-config, now restored. This is the class my earlier fork-focused passes missed; the board's upstream-adoption + seam lenses caught them. + +## Addendum — E (integration seams) full report +Enumerated the seam surface: 587 `@opencode-ai/*` import sites across 216 files in packages/opencode/src + 37 in packages/tui/src; 6 subsystem scopes audited. +- **Azure/DigitalOcean/xAI auth plugins unwired** (MED-HIGH) — FIXED (board commit b0ade919c7). +- **`openai/codex.ts` orphaned** (MED) — DOCUMENTED, not fixed. 0 consumers; fork deliberately uses its own `./codex` (BUILTIN). Harmless dead code; deleting an upstream file risks future-merge friction. Adopting its WebSocket/allowlist/dispose capabilities is a product decision, not a merge fix. +- **Dual-`Session`-module `getUsage` divergence** (MED-latent / LOW-active) — DOCUMENTED, not fixed. Live path is `processor.ts:326 → session/index.ts:830` (carries the OpenRouter cost fix); `session.ts:387` is the caller-less V2-migration copy. Not a live bug; session.ts is the not-shipped V2 surface. +- **OVERTURNED false-positive HIGH**: an earlier subagent claimed `fromPlugin` permission-gate bypass (ctx.ask became Effect → `await` no-ops). E traced the `legacyToInit`→bridged-`legacyCtx` layer (tool-zod-compat.ts:221) that makes ask Promise-based before the plugin runs — the gate runs correctly. Safety surface confirmed clean. +- Server / cli / effect / config / storage seam scopes: clean. Signature-skew class (`Adaptor.target()`) already fixed; 192 tracked `upstream_fix` markers + green typecheck catch compile-level skew. + +## Final board disposition +- Ship-blocker: 1 (release.yml) — FIXED. +- Real dropped-upstream fixes/features: 6 + 3 auth plugins + run-auth — FIXED. +- Security (pre-existing): sensitive-write guard neutralization — FIXED. +- provider-executed metadata — FIXED (execution was already correct); settlement-telemetry field = scoped follow-up. +- Latent/dead-code in the not-shipped V2 surface (codex orphan, getUsage copy) — DOCUMENTED, deliberately not chased. +- Fork preservation + safety surface: PASS (nothing lost or weakened). diff --git a/.github/meta/night-run/UPGRADE-PATH-TEST.md b/.github/meta/night-run/UPGRADE-PATH-TEST.md new file mode 100644 index 0000000000..b290d15dd8 --- /dev/null +++ b/.github/meta/night-run/UPGRADE-PATH-TEST.md @@ -0,0 +1,34 @@ +# Upgrade-path test: v0.8.10 → upstream/merge-v1.17.9 (PR #964) + +Method: built BOTH binaries from source (v0.8.10 tag + branch). Isolated via XDG_* +temp dirs (v0.8.10 reads data dir from xdg-basedir, NOT OPENCODE_TEST_HOME — that +only overrides `home`). Booted v0.8.10 to create an authentic old-schema DB +(10 drizzle migrations; `session` but no `session_message`/`project_directory`; +session lacks metadata/cost/tokens_*/agent/model). Injected sentinel project + +session rows + a v0.8.10 config.json + auth.json. Booted the NEW binary against +the SAME isolated dir = the upgrade moment. + +## Result: 16/16 assertions PASS + idempotent across 3 boots +Schema migrated forward: session.{metadata,cost,tokens_input,tokens_output,agent, +model} added; project_directory + session_message tables created; drizzle journal +advanced 10→11; NO "duplicate column" crash; TUI rendered clean. +Data preserved: sentinel project + session survived; session title byte-identical +(sha1 match); 0 rows lost. Config + auth.json carried over. integrity_check = ok +before and after. + +## Minor finding — FIXED (commit 27c490515a) + verified +`__drizzle_migrations` grows by 1 row per launch: the `20260511173437_session-metadata` +entry is re-inserted every boot (markDrizzleEntriesApplied uses INSERT OR IGNORE but +the table has no UNIQUE constraint, so it never dedupes). DDL is guard-checked so it +never re-runs → no crash, no data change, ~50 bytes/launch. FIXED: markDrizzleEntriesApplied now INSERT...WHERE NOT EXISTS(name) + a guarded +dedupeDrizzleJournal self-heal. Re-verified end-to-end: fixed binary boots 3x +against a v0.8.10 DB with journal rows STABLE at 11 (was 11->12->13), no dup +names, integrity ok. Regression test test/storage/db-journal-idempotency.test.ts. + +## Safety note +v0.8.10 honors OPENCODE_DISABLE_CHANNEL_DB and reads its data dir from XDG. An early +probe that set that flag without XDG isolation briefly opened the real +~/.local/share/altimate-code/opencode.db (1194 sessions) READ-only-ish — verified +intact (integrity ok, 1194 sessions preserved; v0.8.10 migrations are guarded/idempotent). +Correct isolation for old-binary tests = XDG_{DATA,CONFIG,STATE,CACHE}_HOME, not +OPENCODE_TEST_HOME. From 6551112f78375779aba5a747d61cb3e8c0d378e1 Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 19 Oct 2026 05:20:39 +0530 Subject: [PATCH 05/11] fix: bound the stdin wait only for sockets; time-limit the unsent-memory check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consensus review follow-ups. - `readRunStdin` applies the 3s first-byte wait only when stdin is a socket, which is what an agent's shell tool or a Node `spawn` hands a child. A shell pipeline is a FIFO and is read to EOF as before, so `dbt build | altimate-code run …` keeps its input however long the producer takes. - The exit check for unsent memory is capped at 5s, reports an unreadable local count as unknown, and is injectable and tested (gating, unknown, timeout, unlinked). - Notice wording: "N memories on this machine are not in the workspace yet", since the count is the project's, not only this run's. - A test ties `SIGN_IN_COMMAND`'s `--method` to a label the `altimate-backend` provider actually has. - New-file markers; the empty-record comment folded into its docblock. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011chv9Rmu7jQXokum1WNcs7 --- .../opencode/src/altimate/cli/run-stdin.ts | 22 ++++++-- .../src/altimate/cli/unsent-memory.ts | 44 ++++++++++++---- .../altimate/workspace/empty-record-path.ts | 1 + .../src/altimate/workspace/skill-sync.ts | 10 ++-- packages/opencode/src/cli/cmd/run.ts | 4 +- .../test/altimate/cli/unsent-memory.test.ts | 52 +++++++++++++++++-- .../opencode/test/cli/run/run-stdin.test.ts | 12 +++-- .../test/cli/workspace-command.test.ts | 14 ++++- 8 files changed, 130 insertions(+), 29 deletions(-) diff --git a/packages/opencode/src/altimate/cli/run-stdin.ts b/packages/opencode/src/altimate/cli/run-stdin.ts index a366e1ab4e..076a09ec28 100644 --- a/packages/opencode/src/altimate/cli/run-stdin.ts +++ b/packages/opencode/src/altimate/cli/run-stdin.ts @@ -1,11 +1,15 @@ +// altimate_change - new file import { fstatSync } from "fs" -/** How long `run` waits for the first byte on a stdin that is not a TTY or a file. */ +/** How long `run` waits for the first byte on a socket stdin. */ export const STDIN_WAIT_MS = 3000 export interface StdinSource { /** True when fd 0 is a regular file (`run < prompt.txt`), which always reaches EOF. */ isFile(): boolean + /** True when fd 0 is a socket: what an agent's shell tool or a Node `spawn` hands a child. A shell + * pipeline (`cmd | altimate-code run`) is a FIFO instead. */ + isSocket(): boolean readAll(): Promise stream(): ReadableStream } @@ -18,6 +22,13 @@ export const bunStdin: StdinSource = { return false } }, + isSocket() { + try { + return fstatSync(0).isSocket() + } catch { + return false + } + }, readAll: () => Bun.stdin.text(), stream: () => Bun.stdin.stream(), } @@ -26,15 +37,16 @@ export const bunStdin: StdinSource = { * Read text piped into `altimate-code run`, without waiting on a stdin that never sends anything. * * Agents that shell out to `run` (Claude Code's Bash tool, a Node `spawn` with default stdio) hand - * it a pipe or socket they never write to or close. Reading that to EOF blocks forever, so a - * delegated task hangs before it starts. A file is read in full; any other stdin is read only if - * its first chunk arrives within `waitMs`, and then to EOF. + * it a socket they never write to or close. Reading that to EOF blocks forever, so a delegated task + * hangs before it starts. A socket is read only if its first chunk arrives within `waitMs`, and then + * to EOF. Anything else (a file, or a shell pipeline whose producer may take minutes, like + * `dbt build | altimate-code run`) is read to EOF as before. */ export async function readRunStdin( source: StdinSource = bunStdin, waitMs: number = STDIN_WAIT_MS, ): Promise<{ text: string; timedOut: boolean }> { - if (source.isFile()) return { text: await source.readAll(), timedOut: false } + if (source.isFile() || !source.isSocket()) return { text: await source.readAll(), timedOut: false } const reader = source.stream().getReader() let timer: ReturnType | undefined try { diff --git a/packages/opencode/src/altimate/cli/unsent-memory.ts b/packages/opencode/src/altimate/cli/unsent-memory.ts index 1c6dbefef2..ebbce7cc4e 100644 --- a/packages/opencode/src/altimate/cli/unsent-memory.ts +++ b/packages/opencode/src/altimate/cli/unsent-memory.ts @@ -2,26 +2,52 @@ import * as Manage from "@/altimate/workspace/manage" import { mirrorWasRequested } from "@/altimate/workspace/memory-sync" +/** How long `run`'s exit waits for the count. Without a local link row, `Manage.status` asks the service. */ +export const UNSENT_CHECK_MS = 5000 + /** - * What `run` prints at exit about memory saved here that the workspace has not received, or null when - * there is nothing to say. `unsynced` is `workspace status`'s count: an upload that failed, was deferred - * or declined, or was still in flight when the exit flush gave up is pending there; a block deleted - * since is not. + * What `run` prints at exit about memory on this machine that the workspace has not received, or null + * when there is nothing to say. `unsynced` is `workspace status`'s count for the project, not only this + * run's: an upload that failed, was deferred or declined, or was still in flight when the exit flush gave + * up is pending there; a block deleted since is not. */ export function unsentMemoryNotice(unsynced: number | null): string | null { if (unsynced === null) return "Could not confirm that the memory saved here reached the workspace. Run `altimate-code workspace status` to check." if (unsynced === 0) return null return ( - `${unsynced === 1 ? "1 memory saved here is" : `${unsynced} memories saved here are`} not in the workspace yet. ` + + `${unsynced === 1 ? "1 memory on this machine is" : `${unsynced} memories on this machine are`} not in the workspace yet. ` + `Run \`altimate-code workspace sync\` to send ${unsynced === 1 ? "it" : "them"}.` ) } -/** Counted only when this process saved memory for a linked workspace, so other runs pay nothing. */ -export async function unsentMemoryNoticeAfterRun(directory: string): Promise { - if (!mirrorWasRequested()) return null - const report = await Manage.status(directory) +export interface UnsentMemoryDeps { + requested(): boolean + status(directory: string): Promise + timeoutMs: number +} + +const defaultDeps: UnsentMemoryDeps = { + requested: mirrorWasRequested, + status: (directory) => Manage.status(directory), + timeoutMs: UNSENT_CHECK_MS, +} + +/** Counted only when this process saved memory for a linked workspace, so other runs pay nothing. A count + * that cannot be read, or not in time, is reported as unknown. */ +export async function unsentMemoryNoticeAfterRun( + directory: string, + deps: UnsentMemoryDeps = defaultDeps, +): Promise { + if (!deps.requested()) return null + let timer: ReturnType | undefined + const report = await Promise.race([ + deps.status(directory).catch(() => null), + new Promise((resolve) => { + timer = setTimeout(() => resolve(null), deps.timeoutMs) + }), + ]).finally(() => clearTimeout(timer)) + if (!report || report.memoryUnreadable) return unsentMemoryNotice(null) if (!report.binding || !report.memory) return null return unsentMemoryNotice(report.memory.unsynced) } diff --git a/packages/opencode/src/altimate/workspace/empty-record-path.ts b/packages/opencode/src/altimate/workspace/empty-record-path.ts index b38f8e6833..7ec41cfb4f 100644 --- a/packages/opencode/src/altimate/workspace/empty-record-path.ts +++ b/packages/opencode/src/altimate/workspace/empty-record-path.ts @@ -1,3 +1,4 @@ +// altimate_change - new file import path from "path" import { createHash } from "node:crypto" import { Global } from "@/global" diff --git a/packages/opencode/src/altimate/workspace/skill-sync.ts b/packages/opencode/src/altimate/workspace/skill-sync.ts index 7fbf6093a9..ba7da39fac 100644 --- a/packages/opencode/src/altimate/workspace/skill-sync.ts +++ b/packages/opencode/src/altimate/workspace/skill-sync.ts @@ -654,12 +654,10 @@ export async function recentlySynced(directory: string): Promise { /** Which account each project's snapshot was last fetched for. */ const syncedFor = store.syncedFor -/* The record that a project's workspace had no custom skills at the last sync (account key + - * workspace id) lives at `emptyRecordPath`. On disk rather than on `globalThis`, which is per - * thread: an unlink in the TUI thread has to clear what the prompt worker's sync recorded, and the - * other way round. */ - -/** The record as written: `\n`. The write id tells one sync's record from another +/** The record as written: `\n`, at `emptyRecordPath` (./empty-record-path), saying the + * project's workspace had no custom skills at the last sync (account key + workspace id). On disk rather + * than on `globalThis`, which is per thread: an unlink in the TUI thread has to clear what the prompt + * worker's sync recorded, and the other way round. The write id tells one sync's record from another * sync's record for the same key, so a sync only ever withdraws the record it wrote itself. */ async function readEmptyRecordRaw(canon: string): Promise { try { diff --git a/packages/opencode/src/cli/cmd/run.ts b/packages/opencode/src/cli/cmd/run.ts index c9eb989a76..34bb7dda7a 100644 --- a/packages/opencode/src/cli/cmd/run.ts +++ b/packages/opencode/src/cli/cmd/run.ts @@ -553,8 +553,8 @@ export const RunCommand = cmd({ // into a stall: undefined stdin satisfied the guard and we then awaited // Bun.stdin.text() on a stream that would never EOF. Skip the read // entirely when there is no stdin to read from. An agent's shell tool can - // also hand `run` a stdin it never writes to or closes, so the read is - // bounded: see `readRunStdin`. + // also hand `run` a socket stdin it never writes to or closes, so that + // read is bounded: see `readRunStdin`. if (process.stdin && !process.stdin.isTTY) { const { readRunStdin, STDIN_WAIT_MS } = await import("../../altimate/cli/run-stdin") const piped = await readRunStdin() diff --git a/packages/opencode/test/altimate/cli/unsent-memory.test.ts b/packages/opencode/test/altimate/cli/unsent-memory.test.ts index 28dbd662f5..6d8d584034 100644 --- a/packages/opencode/test/altimate/cli/unsent-memory.test.ts +++ b/packages/opencode/test/altimate/cli/unsent-memory.test.ts @@ -1,6 +1,22 @@ // altimate_change - new file import { describe, expect, test } from "bun:test" -import { unsentMemoryNotice } from "../../../src/altimate/cli/unsent-memory" +import { unsentMemoryNotice, unsentMemoryNoticeAfterRun, type UnsentMemoryDeps } from "../../../src/altimate/cli/unsent-memory" + +const binding = { datamateId: 46, datamateName: "Growth" } as any + +function deps(over: Partial = {}): UnsentMemoryDeps & { calls: number } { + const d = { + calls: 0, + requested: () => true, + timeoutMs: 1000, + status: async () => { + d.calls++ + return { binding, memory: { local: 3, unsynced: 2 }, skillsEnabled: true, skillsSyncedAt: null } as any + }, + ...over, + } + return d +} describe("unsentMemoryNotice", () => { test("says nothing when every memory reached the workspace", () => { @@ -9,10 +25,10 @@ describe("unsentMemoryNotice", () => { test("counts what is pending and points at workspace sync", () => { expect(unsentMemoryNotice(1)).toBe( - "1 memory saved here is not in the workspace yet. Run `altimate-code workspace sync` to send it.", + "1 memory on this machine is not in the workspace yet. Run `altimate-code workspace sync` to send it.", ) expect(unsentMemoryNotice(3)).toBe( - "3 memories saved here are not in the workspace yet. Run `altimate-code workspace sync` to send them.", + "3 memories on this machine are not in the workspace yet. Run `altimate-code workspace sync` to send them.", ) }) @@ -20,3 +36,33 @@ describe("unsentMemoryNotice", () => { expect(unsentMemoryNotice(null)).toContain("workspace status") }) }) + +describe("unsentMemoryNoticeAfterRun", () => { + test("a run that saved no memory does not check at all", async () => { + const d = deps({ requested: () => false }) + expect(await unsentMemoryNoticeAfterRun("/p", d)).toBeNull() + expect(d.calls).toBe(0) + }) + + test("a run that saved memory reports the project's pending count", async () => { + expect(await unsentMemoryNoticeAfterRun("/p", deps())).toContain("2 memories on this machine") + }) + + test("an unreadable local count, a failed check or a slow one is reported as unknown", async () => { + const unreadable = deps({ status: async () => ({ binding, memory: null, memoryUnreadable: true }) as any }) + expect(await unsentMemoryNoticeAfterRun("/p", unreadable)).toContain("Could not confirm") + const failed = deps({ status: async () => Promise.reject(new Error("down")) }) + expect(await unsentMemoryNoticeAfterRun("/p", failed)).toContain("Could not confirm") + const slow = deps({ timeoutMs: 20, status: () => new Promise(() => {}) }) + const started = Date.now() + expect(await unsentMemoryNoticeAfterRun("/p", slow)).toContain("Could not confirm") + expect(Date.now() - started).toBeLessThan(1000) + }) + + test("an unlinked project, or memory off in this build, says nothing", async () => { + const unlinked = deps({ status: async () => ({ binding: null, memory: { local: 1, unsynced: 0 } }) as any }) + expect(await unsentMemoryNoticeAfterRun("/p", unlinked)).toBeNull() + const off = deps({ status: async () => ({ binding, memory: null }) as any }) + expect(await unsentMemoryNoticeAfterRun("/p", off)).toBeNull() + }) +}) diff --git a/packages/opencode/test/cli/run/run-stdin.test.ts b/packages/opencode/test/cli/run/run-stdin.test.ts index e0427eb002..3740129e2c 100644 --- a/packages/opencode/test/cli/run/run-stdin.test.ts +++ b/packages/opencode/test/cli/run/run-stdin.test.ts @@ -3,15 +3,16 @@ import { readRunStdin, type StdinSource } from "../../../src/altimate/cli/run-st const encoder = new TextEncoder() -/** A stdin whose stream yields `chunks` (the first at once, each later one after `delayMs`), then ends unless `open` is set. */ -function source(opts: { chunks?: string[]; delayMs?: number; open?: boolean; file?: string }): StdinSource & { +/** A socket stdin (unless `file` or `fifo` is set) whose stream yields `chunks` (the first at once, each later one after `delayMs`), then ends unless `open` is set. */ +function source(opts: { chunks?: string[]; delayMs?: number; open?: boolean; file?: string; fifo?: string }): StdinSource & { cancelled: () => boolean } { let cancelled = false return { cancelled: () => cancelled, isFile: () => opts.file !== undefined, - readAll: async () => opts.file ?? "", + isSocket: () => opts.file === undefined && opts.fifo === undefined, + readAll: async () => opts.file ?? opts.fifo ?? "", stream: () => new ReadableStream({ async start(controller) { @@ -35,6 +36,11 @@ describe("readRunStdin", () => { expect(result).toEqual({ text: "from a file\n", timedOut: false }) }) + test("a shell pipeline (not a socket) is read to EOF however long its producer takes", async () => { + const result = await readRunStdin(source({ fifo: "dbt build output\n" }), 10) + expect(result).toEqual({ text: "dbt build output\n", timedOut: false }) + }) + test("reads piped text to EOF", async () => { const result = await readRunStdin(source({ chunks: ["hello ", "world\n"] }), 1000) expect(result).toEqual({ text: "hello world\n", timedOut: false }) diff --git a/packages/opencode/test/cli/workspace-command.test.ts b/packages/opencode/test/cli/workspace-command.test.ts index e316b8dbf1..c1faef2db7 100644 --- a/packages/opencode/test/cli/workspace-command.test.ts +++ b/packages/opencode/test/cli/workspace-command.test.ts @@ -13,7 +13,8 @@ import { runUnlink, type WorkspaceDeps, } from "../../src/cli/cmd/workspace" -import { linkHeadless, matchWorkspace, type LinkHeadlessDeps } from "../../src/cli/cmd/link" +import { SIGN_IN_COMMAND, linkHeadless, matchWorkspace, type LinkHeadlessDeps } from "../../src/cli/cmd/link" +import { AltimateAuthPlugin } from "../../src/altimate/plugin/altimate" const binding = { datamateId: 35, @@ -23,6 +24,17 @@ const binding = { linkedAt: 0, } as any +describe("SIGN_IN_COMMAND", () => { + test("names a login method the altimate-backend provider has, so --method skips the picker", async () => { + const hooks = await AltimateAuthPlugin({} as any) + expect(hooks.auth?.provider).toBe("altimate-backend") + const labels = hooks.auth!.methods.map((m) => m.label) + const named = SIGN_IN_COMMAND.match(/--method "([^"]+)"/)?.[1] + expect(named).toBeDefined() + expect(labels).toContain(named!) + }) +}) + describe("workspace status wording", () => { test("an unlinked project says so and how to link", () => { const lines = describeStatus({ binding: null, memory: null, skillsEnabled: true, skillsSyncedAt: null }) From 882d35b708ffa49d60d2eded4cc3339f3abd34cb Mon Sep 17 00:00:00 2001 From: Haider Date: Tue, 20 Oct 2026 06:19:53 +0530 Subject: [PATCH 06/11] test: point the stdin source assertion at the bounded read Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011chv9Rmu7jQXokum1WNcs7 --- .../test/release-validation/question-937-codex.test.ts | 4 ++-- .../opencode/test/release-validation/question-937.test.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/opencode/test/release-validation/question-937-codex.test.ts b/packages/opencode/test/release-validation/question-937-codex.test.ts index d389e04524..e1d0657dc7 100644 --- a/packages/opencode/test/release-validation/question-937-codex.test.ts +++ b/packages/opencode/test/release-validation/question-937-codex.test.ts @@ -345,10 +345,10 @@ describe("release validation PR #937 source-level env plumbing", () => { expect(source).not.toContain('process.env["ALTIMATE_NON_INTERACTIVE"] === undefined') }) - test("run command guards Bun.stdin.text() behind an existing non-TTY stdin", async () => { + test("run command guards the stdin read behind an existing non-TTY stdin", async () => { const source = await Bun.file(new URL("../../src/cli/cmd/run.ts", import.meta.url)).text() - expect(source).toContain("if (process.stdin && !process.stdin.isTTY) message +=") + expect(source).toContain("if (process.stdin && !process.stdin.isTTY) {\n const { readRunStdin, STDIN_WAIT_MS }") expect(source).not.toContain("if (!process.stdin?.isTTY)") }) diff --git a/packages/opencode/test/release-validation/question-937.test.ts b/packages/opencode/test/release-validation/question-937.test.ts index f61748810f..21849c3142 100644 --- a/packages/opencode/test/release-validation/question-937.test.ts +++ b/packages/opencode/test/release-validation/question-937.test.ts @@ -386,8 +386,8 @@ describe("run.ts ALTIMATE_NON_INTERACTIVE guard contract", () => { // --------------------------------------------------------------------------- // Gap #9 — null-safe stdin read does not stall when process.stdin is undefined. // -// The guard is inline in src/cli/cmd/run.ts:463 and not exported: -// if (process.stdin && !process.stdin.isTTY) message += "\n" + (await Bun.stdin.text()) +// The guard is inline in src/cli/cmd/run.ts and not exported: +// if (process.stdin && !process.stdin.isTTY) { ... await readRunStdin() ... } // Helper mirrors that predicate and the append behavior so the regression // (undefined stdin satisfied `!stdin?.isTTY` and stalled awaiting EOF) is // locked: undefined stdin must NOT trigger the read. From c973733721430a9fe4b215dd16f75667bab0f219 Mon Sep 17 00:00:00 2001 From: Haider Date: Tue, 20 Oct 2026 06:31:18 +0530 Subject: [PATCH 07/11] fix: address review follow-ups on the `run` stdin wait and unsent-memory notice - `readRunStdin` bounds a socket stdin only when `run` already has a message or `--command`; with nothing else to run, stdin is the prompt and is read to EOF however late it is written. `ALTIMATE_RUN_STDIN_WAIT_MS` changes the wait, and `0` always waits. - `mirrorBlock` records the workspace and directory it uploaded for (`lastMirrorTarget`) instead of a bare count, and skips the record when the workspace has memory turned off, since that block stays local by design. - The exit check passes that binding to `Manage.status`, so the count is read locally and never leaves a request in flight past the bound. - The SIGINT/SIGTERM exit prints the notice too, within a 1s bound after the 2s mirror drain. - Tests: a slow FIFO and a late socket are read in full, the wait override is parsed, the unknown-count message is asserted exactly, and the mirror record is reset per test with a memory-off case. - Docs: the stdin wait and its override in `usage/cli.md`; `workspace list` in the command table. One `.command(...)` per line in `workspace`. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011chv9Rmu7jQXokum1WNcs7 --- docs/docs/usage/cli.md | 4 +- .../opencode/src/altimate/cli/run-stdin.ts | 15 ++++++- .../src/altimate/cli/unsent-memory.ts | 34 +++++++-------- .../src/altimate/workspace/memory-sync.ts | 31 +++++++++----- packages/opencode/src/cli/cmd/run.ts | 20 ++++++--- packages/opencode/src/cli/cmd/workspace.ts | 6 ++- .../test/altimate/cli/unsent-memory.test.ts | 41 +++++++++++-------- .../altimate/workspace/memory-sync.test.ts | 17 ++++++-- .../opencode/test/cli/run/run-stdin.test.ts | 41 ++++++++++++++++--- .../question-937-codex.test.ts | 2 +- 10 files changed, 150 insertions(+), 61 deletions(-) diff --git a/docs/docs/usage/cli.md b/docs/docs/usage/cli.md index b18b1a7f52..11b2d60124 100644 --- a/docs/docs/usage/cli.md +++ b/docs/docs/usage/cli.md @@ -40,7 +40,7 @@ altimate --agent analyst | `import` | Import session data | | `session` | Session management | | `link` | Link this project to an Altimate workspace | -| `workspace` | Show, refresh, sync or unlink this project's workspace from a shell | +| `workspace` | List workspaces, or show, refresh, sync or unlink this project's workspace from a shell | | `trace` | List and view session traces (recordings of agent sessions) | | `github` | GitHub integration | | `pr` | Pull request tools | @@ -190,6 +190,8 @@ altimate --print-logs --log-level DEBUG run "test query" altimate run --no-trace "quick question" ``` +`run` reads piped input to the end, however long the command before the pipe takes. One exception: when `run` already has a message or `--command` and its stdin is a socket, which is what an agent's shell tool or a Node `spawn` hands a child, it waits only 3 seconds for the first byte and then continues without stdin. Set `ALTIMATE_RUN_STDIN_WAIT_MS` to change that wait, or to `0` to always wait. A caller that never writes to or closes stdin, and whose stdin is not a socket (for example a pipe on Windows), should redirect it from an empty source: `< /dev/null`, or `< NUL` in `cmd.exe`. + For CI pipelines and headless automation, see [CI & Automation](ci-headless.md). ## Trace diff --git a/packages/opencode/src/altimate/cli/run-stdin.ts b/packages/opencode/src/altimate/cli/run-stdin.ts index 076a09ec28..a878d4714f 100644 --- a/packages/opencode/src/altimate/cli/run-stdin.ts +++ b/packages/opencode/src/altimate/cli/run-stdin.ts @@ -4,6 +4,14 @@ import { fstatSync } from "fs" /** How long `run` waits for the first byte on a socket stdin. */ export const STDIN_WAIT_MS = 3000 +/** `ALTIMATE_RUN_STDIN_WAIT_MS` overrides the wait, for a caller that writes its stdin late; 0 waits for EOF. */ +export function stdinWaitMs(env: Record = process.env): number { + const raw = env["ALTIMATE_RUN_STDIN_WAIT_MS"]?.trim() + if (!raw) return STDIN_WAIT_MS + const ms = Number(raw) + return Number.isFinite(ms) && ms >= 0 ? ms : STDIN_WAIT_MS +} + export interface StdinSource { /** True when fd 0 is a regular file (`run < prompt.txt`), which always reaches EOF. */ isFile(): boolean @@ -40,13 +48,16 @@ export const bunStdin: StdinSource = { * it a socket they never write to or close. Reading that to EOF blocks forever, so a delegated task * hangs before it starts. A socket is read only if its first chunk arrives within `waitMs`, and then * to EOF. Anything else (a file, or a shell pipeline whose producer may take minutes, like - * `dbt build | altimate-code run`) is read to EOF as before. + * `dbt build | altimate-code run`) is read to EOF as before. So is a socket when `bounded` is false + * (the caller passed nothing else to run, so stdin is the prompt) or `waitMs` is 0. */ export async function readRunStdin( source: StdinSource = bunStdin, waitMs: number = STDIN_WAIT_MS, + bounded = true, ): Promise<{ text: string; timedOut: boolean }> { - if (source.isFile() || !source.isSocket()) return { text: await source.readAll(), timedOut: false } + if (!bounded || waitMs <= 0 || source.isFile() || !source.isSocket()) + return { text: await source.readAll(), timedOut: false } const reader = source.stream().getReader() let timer: ReturnType | undefined try { diff --git a/packages/opencode/src/altimate/cli/unsent-memory.ts b/packages/opencode/src/altimate/cli/unsent-memory.ts index ebbce7cc4e..2df148013e 100644 --- a/packages/opencode/src/altimate/cli/unsent-memory.ts +++ b/packages/opencode/src/altimate/cli/unsent-memory.ts @@ -1,15 +1,16 @@ // altimate_change - new file import * as Manage from "@/altimate/workspace/manage" -import { mirrorWasRequested } from "@/altimate/workspace/memory-sync" +import { lastMirrorTarget, type MirrorTarget } from "@/altimate/workspace/memory-sync" -/** How long `run`'s exit waits for the count. Without a local link row, `Manage.status` asks the service. */ +/** How long `run`'s exit waits for the count. It is read from local files, so this only bounds a stalled disk. */ export const UNSENT_CHECK_MS = 5000 /** * What `run` prints at exit about memory on this machine that the workspace has not received, or null * when there is nothing to say. `unsynced` is `workspace status`'s count for the project, not only this - * run's: an upload that failed, was deferred or declined, or was still in flight when the exit flush gave - * up is pending there; a block deleted since is not. + * run's: an upload that failed, was deferred or declined by the service, or was still in flight when the + * exit flush gave up is pending there; a block deleted since is not. A workspace with memory turned off + * keeps its blocks here by design and is not reported. */ export function unsentMemoryNotice(unsynced: number | null): string | null { if (unsynced === null) @@ -22,29 +23,30 @@ export function unsentMemoryNotice(unsynced: number | null): string | null { } export interface UnsentMemoryDeps { - requested(): boolean - status(directory: string): Promise - timeoutMs: number + target(): MirrorTarget | undefined + status(directory: string, binding: MirrorTarget["binding"]): Promise } const defaultDeps: UnsentMemoryDeps = { - requested: mirrorWasRequested, - status: (directory) => Manage.status(directory), - timeoutMs: UNSENT_CHECK_MS, + target: lastMirrorTarget, + // Passing the binding the upload used keeps `Manage.status` off the network. + status: (directory, binding) => Manage.status(directory, { binding }), } -/** Counted only when this process saved memory for a linked workspace, so other runs pay nothing. A count - * that cannot be read, or not in time, is reported as unknown. */ +/** Counted only when this process tried to upload memory to a linked workspace, so other runs pay nothing. + * A count that cannot be read, or not within `timeoutMs`, is reported as unknown. */ export async function unsentMemoryNoticeAfterRun( - directory: string, + fallbackDirectory: string, + timeoutMs: number = UNSENT_CHECK_MS, deps: UnsentMemoryDeps = defaultDeps, ): Promise { - if (!deps.requested()) return null + const target = deps.target() + if (!target) return null let timer: ReturnType | undefined const report = await Promise.race([ - deps.status(directory).catch(() => null), + deps.status(target.directory ?? fallbackDirectory, target.binding).catch(() => null), new Promise((resolve) => { - timer = setTimeout(() => resolve(null), deps.timeoutMs) + timer = setTimeout(() => resolve(null), timeoutMs) }), ]).finally(() => clearTimeout(timer)) if (!report || report.memoryUnreadable) return unsentMemoryNotice(null) diff --git a/packages/opencode/src/altimate/workspace/memory-sync.ts b/packages/opencode/src/altimate/workspace/memory-sync.ts index 285e43bf3c..33c78e92fc 100644 --- a/packages/opencode/src/altimate/workspace/memory-sync.ts +++ b/packages/opencode/src/altimate/workspace/memory-sync.ts @@ -907,16 +907,23 @@ function serialize(scope: "global" | "project", blockId: string, op: () => Pr * `skill-sync.flushPendingSyncs` holds it for a cold skill sync. */ const mirrorsInFlight = syncState.mirrorsInFlight -/** Whether this process tried to mirror a block to a linked workspace. A one-shot `run` checks it at exit - * and, only then, counts the memory still not in the workspace (`Manage.status`): nothing in `run` retries - * an upload, so without the check a block left behind stays on this machine unannounced. Process-global for - * the same two-module-graph reason as `syncState`. */ -const mirrorRequests: { count: number } = ((globalThis as unknown as Record)[ +/** The linked workspace this process last tried to upload a block to, and the directory it was bound from. + * A one-shot `run` reads it at exit and, only then, counts the memory still not in the workspace + * (`Manage.status`, given this binding so the count stays local): nothing in `run` retries an upload, so + * without the check a block left behind stays on this machine unannounced. Not set when the workspace has + * memory turned off, since that block stays here by design. Process-global for the same two-module-graph + * reason as `syncState`. */ +const mirrorRequests: { last?: MirrorTarget } = ((globalThis as unknown as Record)[ Symbol.for("altimate.memory-sync.mirror-requests") -] ??= { count: 0 }) +] ??= {}) -export function mirrorWasRequested(): boolean { - return mirrorRequests.count > 0 +export interface MirrorTarget { + directory: string | undefined + binding: CachedBinding +} + +export function lastMirrorTarget(): MirrorTarget | undefined { + return mirrorRequests.last } /** Hold a task in `mirrorsInFlight` for its lifetime. */ @@ -965,9 +972,11 @@ export async function mirrorBlock(block: MemoryBlock, directory?: string): Promi // binding governs only whether we upload at all. const binding = await currentBinding(directory) if (!binding) return - // Counted before the memory-setting check: a check that fails also leaves the block unsent. - mirrorRequests.count++ - if (!(await memoryEnabled(binding))) return + const memory = await memoryStatus(binding) + if (memory === "disabled") return + // Recorded before an unknown setting returns: that block is left unsent too. + mirrorRequests.last = { directory, binding } + if (memory !== "enabled") return await push(block, binding, undefined, directory) }) return tracked(task) diff --git a/packages/opencode/src/cli/cmd/run.ts b/packages/opencode/src/cli/cmd/run.ts index 73a3c47721..af1579371e 100644 --- a/packages/opencode/src/cli/cmd/run.ts +++ b/packages/opencode/src/cli/cmd/run.ts @@ -563,13 +563,15 @@ export const RunCommand = cmd({ // Bun.stdin.text() on a stream that would never EOF. Skip the read // entirely when there is no stdin to read from. An agent's shell tool can // also hand `run` a socket stdin it never writes to or closes, so that - // read is bounded: see `readRunStdin`. + // read is bounded when there is a message or command to run without it: + // see `readRunStdin`. if (process.stdin && !process.stdin.isTTY) { - const { readRunStdin, STDIN_WAIT_MS } = await import("../../altimate/cli/run-stdin") - const piped = await readRunStdin() + const { readRunStdin, stdinWaitMs } = await import("../../altimate/cli/run-stdin") + const waitMs = stdinWaitMs() + const piped = await readRunStdin(undefined, waitMs, message.trim().length > 0 || !!args.command) if (piped.timedOut) process.stderr.write( - `No input on stdin after ${STDIN_WAIT_MS / 1000}s; continuing without it. Redirect stdin from /dev/null to skip the wait.${EOL}`, + `No input on stdin after ${waitMs / 1000}s; continuing without it. Redirect stdin from /dev/null to skip the wait, or set ALTIMATE_RUN_STDIN_WAIT_MS=0 to wait for it.${EOL}`, ) message += "\n" + piped.text } @@ -1548,8 +1550,16 @@ You are speaking to a non-technical business executive. Follow these rules stric void Promise.race([ Promise.allSettled(drains), new Promise((resolve) => { timer = setTimeout(resolve, 2_000) }), - ]).finally(() => { + ]).finally(async () => { clearTimeout(timer) + // An interrupted upload is the likeliest to stay on this machine, so say so here too. The count + // is local; a second signal still exits at once. + if (!CoreFlag.ALTIMATE_DISABLE_WORKSPACE) { + const notice = await import("../../altimate/cli/unsent-memory") + .then((m) => m.unsentMemoryNoticeAfterRun(process.cwd(), 1_000)) + .catch(() => null) + if (notice) process.stderr.write(notice + EOL) + } process.exit(code) }) } diff --git a/packages/opencode/src/cli/cmd/workspace.ts b/packages/opencode/src/cli/cmd/workspace.ts index a4ffa99704..2238a71882 100644 --- a/packages/opencode/src/cli/cmd/workspace.ts +++ b/packages/opencode/src/cli/cmd/workspace.ts @@ -386,6 +386,10 @@ export const WorkspaceCommand = cmd({ builder: (yargs) => yargs .command(ListCommand) - .command(StatusCommand).command(RefreshCommand).command(SyncCommand).command(UnlinkCommand).demandCommand(), + .command(StatusCommand) + .command(RefreshCommand) + .command(SyncCommand) + .command(UnlinkCommand) + .demandCommand(), async handler() {}, }) diff --git a/packages/opencode/test/altimate/cli/unsent-memory.test.ts b/packages/opencode/test/altimate/cli/unsent-memory.test.ts index 6d8d584034..b56938803b 100644 --- a/packages/opencode/test/altimate/cli/unsent-memory.test.ts +++ b/packages/opencode/test/altimate/cli/unsent-memory.test.ts @@ -1,15 +1,17 @@ // altimate_change - new file import { describe, expect, test } from "bun:test" import { unsentMemoryNotice, unsentMemoryNoticeAfterRun, type UnsentMemoryDeps } from "../../../src/altimate/cli/unsent-memory" +import type { MirrorTarget } from "../../../src/altimate/workspace/memory-sync" const binding = { datamateId: 46, datamateName: "Growth" } as any -function deps(over: Partial = {}): UnsentMemoryDeps & { calls: number } { +function deps(over: Partial = {}): UnsentMemoryDeps & { calls: number; dirs: string[] } { const d = { calls: 0, - requested: () => true, - timeoutMs: 1000, - status: async () => { + dirs: [] as string[], + target: (): MirrorTarget | undefined => ({ directory: "/bound", binding }), + status: async (directory: string) => { + d.dirs.push(directory) d.calls++ return { binding, memory: { local: 3, unsynced: 2 }, skillsEnabled: true, skillsSyncedAt: null } as any }, @@ -33,36 +35,43 @@ describe("unsentMemoryNotice", () => { }) test("an unknown count is reported as unknown, never as zero", () => { - expect(unsentMemoryNotice(null)).toContain("workspace status") + expect(unsentMemoryNotice(null)).toBe( + "Could not confirm that the memory saved here reached the workspace. Run `altimate-code workspace status` to check.", + ) }) }) describe("unsentMemoryNoticeAfterRun", () => { - test("a run that saved no memory does not check at all", async () => { - const d = deps({ requested: () => false }) - expect(await unsentMemoryNoticeAfterRun("/p", d)).toBeNull() + test("a run that uploaded no memory to a linked workspace does not check at all", async () => { + const d = deps({ target: () => undefined }) + expect(await unsentMemoryNoticeAfterRun("/p", 1000, d)).toBeNull() expect(d.calls).toBe(0) }) - test("a run that saved memory reports the project's pending count", async () => { - expect(await unsentMemoryNoticeAfterRun("/p", deps())).toContain("2 memories on this machine") + test("a run that saved memory reports the pending count of the directory it uploaded from", async () => { + const d = deps() + expect(await unsentMemoryNoticeAfterRun("/p", 1000, d)).toContain("2 memories on this machine") + expect(d.dirs).toEqual(["/bound"]) + const unscoped = deps({ target: () => ({ directory: undefined, binding }) }) + await unsentMemoryNoticeAfterRun("/p", 1000, unscoped) + expect(unscoped.dirs).toEqual(["/p"]) }) test("an unreadable local count, a failed check or a slow one is reported as unknown", async () => { const unreadable = deps({ status: async () => ({ binding, memory: null, memoryUnreadable: true }) as any }) - expect(await unsentMemoryNoticeAfterRun("/p", unreadable)).toContain("Could not confirm") + expect(await unsentMemoryNoticeAfterRun("/p", 1000, unreadable)).toContain("Could not confirm") const failed = deps({ status: async () => Promise.reject(new Error("down")) }) - expect(await unsentMemoryNoticeAfterRun("/p", failed)).toContain("Could not confirm") - const slow = deps({ timeoutMs: 20, status: () => new Promise(() => {}) }) + expect(await unsentMemoryNoticeAfterRun("/p", 1000, failed)).toContain("Could not confirm") + const slow = deps({ status: () => new Promise(() => {}) }) const started = Date.now() - expect(await unsentMemoryNoticeAfterRun("/p", slow)).toContain("Could not confirm") + expect(await unsentMemoryNoticeAfterRun("/p", 20, slow)).toContain("Could not confirm") expect(Date.now() - started).toBeLessThan(1000) }) test("an unlinked project, or memory off in this build, says nothing", async () => { const unlinked = deps({ status: async () => ({ binding: null, memory: { local: 1, unsynced: 0 } }) as any }) - expect(await unsentMemoryNoticeAfterRun("/p", unlinked)).toBeNull() + expect(await unsentMemoryNoticeAfterRun("/p", 1000, unlinked)).toBeNull() const off = deps({ status: async () => ({ binding, memory: null }) as any }) - expect(await unsentMemoryNoticeAfterRun("/p", off)).toBeNull() + expect(await unsentMemoryNoticeAfterRun("/p", 1000, off)).toBeNull() }) }) diff --git a/packages/opencode/test/altimate/workspace/memory-sync.test.ts b/packages/opencode/test/altimate/workspace/memory-sync.test.ts index 352c867fec..aa6f1636bb 100644 --- a/packages/opencode/test/altimate/workspace/memory-sync.test.ts +++ b/packages/opencode/test/altimate/workspace/memory-sync.test.ts @@ -45,7 +45,7 @@ const { isEnabled, memoryEnabledCached, mirrorBlock, - mirrorWasRequested, + lastMirrorTarget, flushPendingMirrors, overlayBlocks, refresh, @@ -183,6 +183,9 @@ beforeEach(() => { stubCreds("acme", "https://api.example.com") stubFetch() resetOverlay() + // Process-global (see `lastMirrorTarget`), so an earlier test's mirror would otherwise satisfy a later one. + ;(globalThis as unknown as Record)[Symbol.for("altimate.memory-sync.mirror-requests")].last = + undefined syncInternals.resolveBinding = async () => BINDING as any // The store is not on disk in these tests; the delete-race guard has its // own coverage below. @@ -449,10 +452,18 @@ describe("blockTitle", () => { // ── write path ────────────────────────────────────────────────────────────── describe("mirrorBlock", () => { test("a mirror to a linked workspace is recorded, failed or not, so `run`'s exit checks what stayed local", async () => { + expect(lastMirrorTarget()).toBeUndefined() createFails = true - await expect(mirrorBlock(block({ id: "unsent" }))).rejects.toThrow() + await expect(mirrorBlock(block({ id: "unsent" }), "/repo")).rejects.toThrow() createFails = false - expect(mirrorWasRequested()).toBe(true) + expect(lastMirrorTarget()).toEqual({ directory: "/repo", binding: BINDING as any }) + }) + + test("a workspace with memory turned off is not recorded: the block stays local by design", async () => { + workspaces = [{ id: 42, name: "acme", memory_enabled: false }] + await mirrorBlock(block({ id: "kept-local" }), "/repo") + expect(lastMirrorTarget()).toBeUndefined() + expect(captured.some((c) => c.method === "POST")).toBe(false) }) test("flushPendingMirrors waits for a mirror a short-lived process would abandon (#1332)", async () => { diff --git a/packages/opencode/test/cli/run/run-stdin.test.ts b/packages/opencode/test/cli/run/run-stdin.test.ts index 3740129e2c..03b01c2d6c 100644 --- a/packages/opencode/test/cli/run/run-stdin.test.ts +++ b/packages/opencode/test/cli/run/run-stdin.test.ts @@ -1,10 +1,18 @@ import { describe, expect, test } from "bun:test" -import { readRunStdin, type StdinSource } from "../../../src/altimate/cli/run-stdin" +import { readRunStdin, stdinWaitMs, STDIN_WAIT_MS, type StdinSource } from "../../../src/altimate/cli/run-stdin" const encoder = new TextEncoder() -/** A socket stdin (unless `file` or `fifo` is set) whose stream yields `chunks` (the first at once, each later one after `delayMs`), then ends unless `open` is set. */ -function source(opts: { chunks?: string[]; delayMs?: number; open?: boolean; file?: string; fifo?: string }): StdinSource & { +/** A socket stdin (unless `file` or `fifo` is set) whose stream yields `chunks` (the first at once, each later one after `delayMs`), then ends unless `open` is set. Its read-to-EOF path returns `file`, `fifo` or `late` after `readAllMs`. */ +function source(opts: { + chunks?: string[] + delayMs?: number + open?: boolean + file?: string + fifo?: string + late?: string + readAllMs?: number +}): StdinSource & { cancelled: () => boolean } { let cancelled = false @@ -12,7 +20,7 @@ function source(opts: { chunks?: string[]; delayMs?: number; open?: boolean; fil cancelled: () => cancelled, isFile: () => opts.file !== undefined, isSocket: () => opts.file === undefined && opts.fifo === undefined, - readAll: async () => opts.file ?? opts.fifo ?? "", + readAll: () => Bun.sleep(opts.readAllMs ?? 0).then(() => opts.file ?? opts.fifo ?? opts.late ?? ""), stream: () => new ReadableStream({ async start(controller) { @@ -37,10 +45,20 @@ describe("readRunStdin", () => { }) test("a shell pipeline (not a socket) is read to EOF however long its producer takes", async () => { - const result = await readRunStdin(source({ fifo: "dbt build output\n" }), 10) + const result = await readRunStdin(source({ fifo: "dbt build output\n", readAllMs: 60 }), 10) expect(result).toEqual({ text: "dbt build output\n", timedOut: false }) }) + test("a socket is read to EOF when there is nothing else to run, however late it is written", async () => { + const result = await readRunStdin(source({ chunks: [], open: true, late: "the prompt\n", readAllMs: 60 }), 10, false) + expect(result).toEqual({ text: "the prompt\n", timedOut: false }) + }) + + test("a wait of 0 reads a socket to EOF", async () => { + const result = await readRunStdin(source({ chunks: [], open: true, late: "late\n", readAllMs: 60 }), 0) + expect(result).toEqual({ text: "late\n", timedOut: false }) + }) + test("reads piped text to EOF", async () => { const result = await readRunStdin(source({ chunks: ["hello ", "world\n"] }), 1000) expect(result).toEqual({ text: "hello world\n", timedOut: false }) @@ -69,3 +87,16 @@ describe("readRunStdin", () => { expect(stdin.cancelled()).toBe(true) }) }) + +describe("stdinWaitMs", () => { + test("defaults to the 3s wait, and ALTIMATE_RUN_STDIN_WAIT_MS overrides it", () => { + expect(stdinWaitMs({})).toBe(STDIN_WAIT_MS) + expect(stdinWaitMs({ ALTIMATE_RUN_STDIN_WAIT_MS: "10000" })).toBe(10000) + expect(stdinWaitMs({ ALTIMATE_RUN_STDIN_WAIT_MS: " 0 " })).toBe(0) + }) + + test("an unusable value keeps the default", () => { + for (const raw of ["", " ", "soon", "-1", "Infinity"]) + expect(stdinWaitMs({ ALTIMATE_RUN_STDIN_WAIT_MS: raw })).toBe(STDIN_WAIT_MS) + }) +}) diff --git a/packages/opencode/test/release-validation/question-937-codex.test.ts b/packages/opencode/test/release-validation/question-937-codex.test.ts index e1d0657dc7..97165a7167 100644 --- a/packages/opencode/test/release-validation/question-937-codex.test.ts +++ b/packages/opencode/test/release-validation/question-937-codex.test.ts @@ -348,7 +348,7 @@ describe("release validation PR #937 source-level env plumbing", () => { test("run command guards the stdin read behind an existing non-TTY stdin", async () => { const source = await Bun.file(new URL("../../src/cli/cmd/run.ts", import.meta.url)).text() - expect(source).toContain("if (process.stdin && !process.stdin.isTTY) {\n const { readRunStdin, STDIN_WAIT_MS }") + expect(source).toContain("if (process.stdin && !process.stdin.isTTY) {\n const { readRunStdin, stdinWaitMs }") expect(source).not.toContain("if (!process.stdin?.isTTY)") }) From 533cfe835da8a83fa23c86f4669cadb958926a7c Mon Sep 17 00:00:00 2001 From: Haider Date: Tue, 20 Oct 2026 11:59:20 +0530 Subject: [PATCH 08/11] fix: tidy the `run` signal-exit notice and the stdin source assertion - Load `unsent-memory` alongside the 2s mirror drain on SIGINT/SIGTERM, so only the local count runs after it. - Assert the stdin guard and the `readRunStdin(` call separately instead of pinning the newline and indent between them. - Test `workspace list --json` through the yargs command, not only `runList`. - Docs: a slow pipe reaching `run` through a socket (for example over `ssh`) is subject to the same 3s wait. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011chv9Rmu7jQXokum1WNcs7 --- docs/docs/usage/cli.md | 2 +- packages/opencode/src/cli/cmd/run.ts | 14 ++++++---- .../test/cli/workspace-command.test.ts | 28 +++++++++++++++++++ .../question-937-codex.test.ts | 3 +- 4 files changed, 39 insertions(+), 8 deletions(-) diff --git a/docs/docs/usage/cli.md b/docs/docs/usage/cli.md index 11b2d60124..2e12b5ff5e 100644 --- a/docs/docs/usage/cli.md +++ b/docs/docs/usage/cli.md @@ -190,7 +190,7 @@ altimate --print-logs --log-level DEBUG run "test query" altimate run --no-trace "quick question" ``` -`run` reads piped input to the end, however long the command before the pipe takes. One exception: when `run` already has a message or `--command` and its stdin is a socket, which is what an agent's shell tool or a Node `spawn` hands a child, it waits only 3 seconds for the first byte and then continues without stdin. Set `ALTIMATE_RUN_STDIN_WAIT_MS` to change that wait, or to `0` to always wait. A caller that never writes to or closes stdin, and whose stdin is not a socket (for example a pipe on Windows), should redirect it from an empty source: `< /dev/null`, or `< NUL` in `cmd.exe`. +`run` reads piped input to the end, however long the command before the pipe takes. One exception: when `run` already has a message or `--command` and its stdin is a socket, which is what an agent's shell tool or a Node `spawn` hands a child, it waits only 3 seconds for the first byte and then continues without stdin. That can also catch a slow pipe that reaches `run` through a socket, such as `slow-cmd | ssh host altimate-code run "summarize"`. Set `ALTIMATE_RUN_STDIN_WAIT_MS` to change that wait, or to `0` to always wait. A caller that never writes to or closes stdin, and whose stdin is not a socket (for example a pipe on Windows), should redirect it from an empty source: `< /dev/null`, or `< NUL` in `cmd.exe`. For CI pipelines and headless automation, see [CI & Automation](ci-headless.md). diff --git a/packages/opencode/src/cli/cmd/run.ts b/packages/opencode/src/cli/cmd/run.ts index af1579371e..4bff295407 100644 --- a/packages/opencode/src/cli/cmd/run.ts +++ b/packages/opencode/src/cli/cmd/run.ts @@ -1540,6 +1540,10 @@ You are speaking to a non-technical business executive. Follow these rules stric // flush takes; what is already on the wire is what gets the 2s. eventAbort.abort() const drains: Promise[] = [] + // Loaded during the drain, so only the count itself runs after it. + const unsent = CoreFlag.ALTIMATE_DISABLE_WORKSPACE + ? undefined + : import("../../altimate/cli/unsent-memory").catch(() => undefined) if (!CoreFlag.ALTIMATE_DISABLE_WORKSPACE) drains.push(import("../../altimate/workspace/memory-sync").then((m) => m.flushPendingMirrors(2_000))) if (captureEnabled) @@ -1554,12 +1558,10 @@ You are speaking to a non-technical business executive. Follow these rules stric clearTimeout(timer) // An interrupted upload is the likeliest to stay on this machine, so say so here too. The count // is local; a second signal still exits at once. - if (!CoreFlag.ALTIMATE_DISABLE_WORKSPACE) { - const notice = await import("../../altimate/cli/unsent-memory") - .then((m) => m.unsentMemoryNoticeAfterRun(process.cwd(), 1_000)) - .catch(() => null) - if (notice) process.stderr.write(notice + EOL) - } + const notice = await unsent + ?.then((m) => m?.unsentMemoryNoticeAfterRun(process.cwd(), 1_000)) + .catch(() => null) + if (notice) process.stderr.write(notice + EOL) process.exit(code) }) } diff --git a/packages/opencode/test/cli/workspace-command.test.ts b/packages/opencode/test/cli/workspace-command.test.ts index c1faef2db7..d01d6d667c 100644 --- a/packages/opencode/test/cli/workspace-command.test.ts +++ b/packages/opencode/test/cli/workspace-command.test.ts @@ -11,10 +11,14 @@ import { runStatus, runSync, runUnlink, + WorkspaceCommand, type WorkspaceDeps, } from "../../src/cli/cmd/workspace" import { SIGN_IN_COMMAND, linkHeadless, matchWorkspace, type LinkHeadlessDeps } from "../../src/cli/cmd/link" import { AltimateAuthPlugin } from "../../src/altimate/plugin/altimate" +import { AltimateApi } from "../../src/altimate/api/client" +import { WorkspaceApi } from "../../src/altimate/workspace/api-client" +import yargs from "yargs" const binding = { datamateId: 35, @@ -450,3 +454,27 @@ describe("link --workspace / --create without prompting", () => { expect(f.calls).toEqual(["create:Marketing"]) }) }) + +describe("workspace list through the command parser", () => { + const original = { isConfigured: AltimateApi.isConfigured, listDatamates: WorkspaceApi.listDatamates, write: process.stdout.write } + afterEach(() => { + ;(AltimateApi as any).isConfigured = original.isConfigured + ;(WorkspaceApi as any).listDatamates = original.listDatamates + process.stdout.write = original.write + process.exitCode = 0 + }) + + test("`workspace list --json` is registered and prints the account's workspaces", async () => { + ;(AltimateApi as any).isConfigured = async () => true + ;(WorkspaceApi as any).listDatamates = async () => [{ id: 35, name: "Growth", memory_enabled: true }] + let written = "" + process.stdout.write = ((chunk: string) => { + written += chunk + return true + }) as typeof process.stdout.write + await yargs(["workspace", "list", "--json"]).command(WorkspaceCommand).strict().fail(false).parseAsync() + process.stdout.write = original.write + expect(JSON.parse(written)).toEqual({ ok: true, workspaces: [{ id: 35, name: "Growth" }] }) + expect(process.exitCode).toBe(0) + }) +}) diff --git a/packages/opencode/test/release-validation/question-937-codex.test.ts b/packages/opencode/test/release-validation/question-937-codex.test.ts index 97165a7167..5b0c44f629 100644 --- a/packages/opencode/test/release-validation/question-937-codex.test.ts +++ b/packages/opencode/test/release-validation/question-937-codex.test.ts @@ -348,7 +348,8 @@ describe("release validation PR #937 source-level env plumbing", () => { test("run command guards the stdin read behind an existing non-TTY stdin", async () => { const source = await Bun.file(new URL("../../src/cli/cmd/run.ts", import.meta.url)).text() - expect(source).toContain("if (process.stdin && !process.stdin.isTTY) {\n const { readRunStdin, stdinWaitMs }") + expect(source).toContain("if (process.stdin && !process.stdin.isTTY) {") + expect(source).toContain("readRunStdin(") expect(source).not.toContain("if (!process.stdin?.isTTY)") }) From f6ea10be31af45fea65693d093016bb431836523 Mon Sep 17 00:00:00 2001 From: Haider Date: Tue, 20 Oct 2026 12:13:36 +0530 Subject: [PATCH 09/11] fix: record the mirror before the memory lookup; test `run` stdin on a real fd - `mirrorBlock` records the workspace it uploads for before the memory-setting lookup, which can wait on the service, so an exit while that lookup is pending still gets the unsent-memory check. A confirmed "disabled" takes the record back. - The signal exit waits for the notice's stderr write before `process.exit`. - `ALTIMATE_RUN_STDIN_WAIT_MS` is capped at the `setTimeout` maximum, so a huge value is not turned into an immediate timeout. - New subprocess test runs `readRunStdin` on a real stdin: an idle `spawn` socket times out and the process exits, a written socket and a late write with nothing else to run come through, and a slow shell pipe is read. - The release-validation test checks that `readRunStdin(` sits inside the non-TTY guard's block, whatever the whitespace. - Tests: the status stub checks the binding it is given; the `workspace list` parser test keeps another test's exit code. - Docs: drop the `ssh` example, which depends on how sshd hands over stdin. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011chv9Rmu7jQXokum1WNcs7 --- docs/docs/usage/cli.md | 2 +- .../opencode/src/altimate/cli/run-stdin.ts | 6 ++- .../src/altimate/workspace/memory-sync.ts | 11 +++-- packages/opencode/src/cli/cmd/run.ts | 3 +- .../test/altimate/cli/unsent-memory.test.ts | 3 +- .../altimate/workspace/memory-sync.test.ts | 16 +++++++ .../test/cli/run/fixtures/read-run-stdin.ts | 6 +++ .../test/cli/run/run-stdin-process.test.ts | 43 +++++++++++++++++++ .../opencode/test/cli/run/run-stdin.test.ts | 4 ++ .../test/cli/workspace-command.test.ts | 6 ++- .../question-937-codex.test.ts | 13 +++++- 11 files changed, 103 insertions(+), 10 deletions(-) create mode 100644 packages/opencode/test/cli/run/fixtures/read-run-stdin.ts create mode 100644 packages/opencode/test/cli/run/run-stdin-process.test.ts diff --git a/docs/docs/usage/cli.md b/docs/docs/usage/cli.md index 2e12b5ff5e..11b2d60124 100644 --- a/docs/docs/usage/cli.md +++ b/docs/docs/usage/cli.md @@ -190,7 +190,7 @@ altimate --print-logs --log-level DEBUG run "test query" altimate run --no-trace "quick question" ``` -`run` reads piped input to the end, however long the command before the pipe takes. One exception: when `run` already has a message or `--command` and its stdin is a socket, which is what an agent's shell tool or a Node `spawn` hands a child, it waits only 3 seconds for the first byte and then continues without stdin. That can also catch a slow pipe that reaches `run` through a socket, such as `slow-cmd | ssh host altimate-code run "summarize"`. Set `ALTIMATE_RUN_STDIN_WAIT_MS` to change that wait, or to `0` to always wait. A caller that never writes to or closes stdin, and whose stdin is not a socket (for example a pipe on Windows), should redirect it from an empty source: `< /dev/null`, or `< NUL` in `cmd.exe`. +`run` reads piped input to the end, however long the command before the pipe takes. One exception: when `run` already has a message or `--command` and its stdin is a socket, which is what an agent's shell tool or a Node `spawn` hands a child, it waits only 3 seconds for the first byte and then continues without stdin. Set `ALTIMATE_RUN_STDIN_WAIT_MS` to change that wait, or to `0` to always wait. A caller that never writes to or closes stdin, and whose stdin is not a socket (for example a pipe on Windows), should redirect it from an empty source: `< /dev/null`, or `< NUL` in `cmd.exe`. For CI pipelines and headless automation, see [CI & Automation](ci-headless.md). diff --git a/packages/opencode/src/altimate/cli/run-stdin.ts b/packages/opencode/src/altimate/cli/run-stdin.ts index a878d4714f..d800eb812d 100644 --- a/packages/opencode/src/altimate/cli/run-stdin.ts +++ b/packages/opencode/src/altimate/cli/run-stdin.ts @@ -4,12 +4,16 @@ import { fstatSync } from "fs" /** How long `run` waits for the first byte on a socket stdin. */ export const STDIN_WAIT_MS = 3000 +/** The longest delay `setTimeout` honours; a larger one fires at once. */ +const MAX_TIMER_MS = 2_147_483_647 + /** `ALTIMATE_RUN_STDIN_WAIT_MS` overrides the wait, for a caller that writes its stdin late; 0 waits for EOF. */ export function stdinWaitMs(env: Record = process.env): number { const raw = env["ALTIMATE_RUN_STDIN_WAIT_MS"]?.trim() if (!raw) return STDIN_WAIT_MS const ms = Number(raw) - return Number.isFinite(ms) && ms >= 0 ? ms : STDIN_WAIT_MS + if (!Number.isFinite(ms) || ms < 0) return STDIN_WAIT_MS + return Math.min(ms, MAX_TIMER_MS) } export interface StdinSource { diff --git a/packages/opencode/src/altimate/workspace/memory-sync.ts b/packages/opencode/src/altimate/workspace/memory-sync.ts index 33c78e92fc..0fc6361075 100644 --- a/packages/opencode/src/altimate/workspace/memory-sync.ts +++ b/packages/opencode/src/altimate/workspace/memory-sync.ts @@ -972,10 +972,15 @@ export async function mirrorBlock(block: MemoryBlock, directory?: string): Promi // binding governs only whether we upload at all. const binding = await currentBinding(directory) if (!binding) return - const memory = await memoryStatus(binding) - if (memory === "disabled") return - // Recorded before an unknown setting returns: that block is left unsent too. + // Recorded before the memory-setting lookup, which can wait on the service: an exit while it is pending + // leaves this block unsent too. Taken back only on a confirmed "disabled". + const previous = mirrorRequests.last mirrorRequests.last = { directory, binding } + const memory = await memoryStatus(binding) + if (memory === "disabled") { + mirrorRequests.last = previous + return + } if (memory !== "enabled") return await push(block, binding, undefined, directory) }) diff --git a/packages/opencode/src/cli/cmd/run.ts b/packages/opencode/src/cli/cmd/run.ts index 4bff295407..8de4275477 100644 --- a/packages/opencode/src/cli/cmd/run.ts +++ b/packages/opencode/src/cli/cmd/run.ts @@ -1561,7 +1561,8 @@ You are speaking to a non-technical business executive. Follow these rules stric const notice = await unsent ?.then((m) => m?.unsentMemoryNoticeAfterRun(process.cwd(), 1_000)) .catch(() => null) - if (notice) process.stderr.write(notice + EOL) + // process.exit does not wait for a pending write to a piped stderr. + if (notice) await new Promise((resolve) => process.stderr.write(notice + EOL, () => resolve())) process.exit(code) }) } diff --git a/packages/opencode/test/altimate/cli/unsent-memory.test.ts b/packages/opencode/test/altimate/cli/unsent-memory.test.ts index b56938803b..30dcc95e63 100644 --- a/packages/opencode/test/altimate/cli/unsent-memory.test.ts +++ b/packages/opencode/test/altimate/cli/unsent-memory.test.ts @@ -10,7 +10,8 @@ function deps(over: Partial = {}): UnsentMemoryDeps & { calls: calls: 0, dirs: [] as string[], target: (): MirrorTarget | undefined => ({ directory: "/bound", binding }), - status: async (directory: string) => { + status: async (directory: string, given: MirrorTarget["binding"]) => { + expect(given).toBe(binding) d.dirs.push(directory) d.calls++ return { binding, memory: { local: 3, unsynced: 2 }, skillsEnabled: true, skillsSyncedAt: null } as any diff --git a/packages/opencode/test/altimate/workspace/memory-sync.test.ts b/packages/opencode/test/altimate/workspace/memory-sync.test.ts index aa6f1636bb..1830d8e275 100644 --- a/packages/opencode/test/altimate/workspace/memory-sync.test.ts +++ b/packages/opencode/test/altimate/workspace/memory-sync.test.ts @@ -459,6 +459,22 @@ describe("mirrorBlock", () => { expect(lastMirrorTarget()).toEqual({ directory: "/repo", binding: BINDING as any }) }) + test("the mirror is recorded while the memory-setting lookup is still pending, so an exit then still checks", async () => { + const inner = globalThis.fetch + let release!: () => void + const held = new Promise((resolve) => (release = resolve)) + globalThis.fetch = (async (input: any, init?: any) => { + if (String(input).includes("/datamates/") && !String(input).includes("/memory")) await held + return inner(input, init) + }) as typeof fetch + const mirror = mirrorBlock(block({ id: "pending-lookup" }), "/repo") + await Bun.sleep(20) + expect(lastMirrorTarget()).toEqual({ directory: "/repo", binding: BINDING as any }) + release() + await mirror + globalThis.fetch = inner + }) + test("a workspace with memory turned off is not recorded: the block stays local by design", async () => { workspaces = [{ id: 42, name: "acme", memory_enabled: false }] await mirrorBlock(block({ id: "kept-local" }), "/repo") diff --git a/packages/opencode/test/cli/run/fixtures/read-run-stdin.ts b/packages/opencode/test/cli/run/fixtures/read-run-stdin.ts new file mode 100644 index 0000000000..2b161f0830 --- /dev/null +++ b/packages/opencode/test/cli/run/fixtures/read-run-stdin.ts @@ -0,0 +1,6 @@ +// altimate_change - new file +// Reads this process's real stdin with `readRunStdin`, as `run` does, and prints the result. +import { readRunStdin, stdinWaitMs } from "../../../../src/altimate/cli/run-stdin" + +const result = await readRunStdin(undefined, stdinWaitMs(), process.argv[2] !== "unbounded") +process.stdout.write(JSON.stringify(result)) diff --git a/packages/opencode/test/cli/run/run-stdin-process.test.ts b/packages/opencode/test/cli/run/run-stdin-process.test.ts new file mode 100644 index 0000000000..2156fe5dc6 --- /dev/null +++ b/packages/opencode/test/cli/run/run-stdin-process.test.ts @@ -0,0 +1,43 @@ +// altimate_change - new file +// `readRunStdin` on a real stdin: the other tests inject a fake source, so only these check that +// `fstatSync(0)` sees a socket for a Node `spawn` and a FIFO for a shell pipeline. +import { describe, expect, test } from "bun:test" +import { spawn } from "node:child_process" +import path from "node:path" + +const FIXTURE = path.join(import.meta.dir, "fixtures", "read-run-stdin.ts") +const env = { ...process.env, ALTIMATE_RUN_STDIN_WAIT_MS: "200" } + +function collect(child: ReturnType): Promise<{ code: number | null; out: string; ms: number }> { + const started = Date.now() + let out = "" + child.stdout!.on("data", (chunk) => (out += chunk)) + return new Promise((resolve) => child.on("exit", (code) => resolve({ code, out, ms: Date.now() - started }))) +} + +describe("readRunStdin on a real stdin", () => { + test("a socket stdin that is never written or closed times out, and the process exits on its own", async () => { + const child = spawn(process.execPath, [FIXTURE], { env }) + const result = await collect(child) + expect(result.code).toBe(0) + expect(JSON.parse(result.out)).toEqual({ text: "", timedOut: true }) + expect(result.ms).toBeLessThan(10_000) + }) + + test("a socket stdin that is written and closed comes through", async () => { + const child = spawn(process.execPath, [FIXTURE], { env }) + child.stdin!.end("from the caller\n") + expect(JSON.parse((await collect(child)).out)).toEqual({ text: "from the caller\n", timedOut: false }) + }) + + test("with nothing else to run, a socket written after the wait still comes through", async () => { + const child = spawn(process.execPath, [FIXTURE, "unbounded"], { env }) + setTimeout(() => child.stdin!.end("the prompt\n"), 600) + expect(JSON.parse((await collect(child)).out)).toEqual({ text: "the prompt\n", timedOut: false }) + }) + + test("a shell pipeline is read to EOF even when its first byte comes after the wait", async () => { + const child = spawn("sh", ["-c", `sleep 0.6; echo piped | "${process.execPath}" "${FIXTURE}"`], { env }) + expect(JSON.parse((await collect(child)).out)).toEqual({ text: "piped\n", timedOut: false }) + }) +}) diff --git a/packages/opencode/test/cli/run/run-stdin.test.ts b/packages/opencode/test/cli/run/run-stdin.test.ts index 03b01c2d6c..ba08deeaac 100644 --- a/packages/opencode/test/cli/run/run-stdin.test.ts +++ b/packages/opencode/test/cli/run/run-stdin.test.ts @@ -95,6 +95,10 @@ describe("stdinWaitMs", () => { expect(stdinWaitMs({ ALTIMATE_RUN_STDIN_WAIT_MS: " 0 " })).toBe(0) }) + test("a wait longer than a timer can hold is capped, not turned into an immediate timeout", () => { + expect(stdinWaitMs({ ALTIMATE_RUN_STDIN_WAIT_MS: "99999999999" })).toBe(2_147_483_647) + }) + test("an unusable value keeps the default", () => { for (const raw of ["", " ", "soon", "-1", "Infinity"]) expect(stdinWaitMs({ ALTIMATE_RUN_STDIN_WAIT_MS: raw })).toBe(STDIN_WAIT_MS) diff --git a/packages/opencode/test/cli/workspace-command.test.ts b/packages/opencode/test/cli/workspace-command.test.ts index d01d6d667c..c91130fce5 100644 --- a/packages/opencode/test/cli/workspace-command.test.ts +++ b/packages/opencode/test/cli/workspace-command.test.ts @@ -457,11 +457,15 @@ describe("link --workspace / --create without prompting", () => { describe("workspace list through the command parser", () => { const original = { isConfigured: AltimateApi.isConfigured, listDatamates: WorkspaceApi.listDatamates, write: process.stdout.write } + let priorExitCode: typeof process.exitCode + beforeEach(() => { + priorExitCode = process.exitCode + }) afterEach(() => { ;(AltimateApi as any).isConfigured = original.isConfigured ;(WorkspaceApi as any).listDatamates = original.listDatamates process.stdout.write = original.write - process.exitCode = 0 + process.exitCode = priorExitCode ?? 0 }) test("`workspace list --json` is registered and prints the account's workspaces", async () => { diff --git a/packages/opencode/test/release-validation/question-937-codex.test.ts b/packages/opencode/test/release-validation/question-937-codex.test.ts index 5b0c44f629..3a7deeb244 100644 --- a/packages/opencode/test/release-validation/question-937-codex.test.ts +++ b/packages/opencode/test/release-validation/question-937-codex.test.ts @@ -348,8 +348,17 @@ describe("release validation PR #937 source-level env plumbing", () => { test("run command guards the stdin read behind an existing non-TTY stdin", async () => { const source = await Bun.file(new URL("../../src/cli/cmd/run.ts", import.meta.url)).text() - expect(source).toContain("if (process.stdin && !process.stdin.isTTY) {") - expect(source).toContain("readRunStdin(") + const guard = "if (process.stdin && !process.stdin.isTTY) {" + const start = source.indexOf(guard) + expect(start).toBeGreaterThan(-1) + // The guard's block, up to its matching brace, must contain the read. + let depth = 0 + let end = start + guard.length - 1 + for (; end < source.length; end++) { + if (source[end] === "{") depth++ + else if (source[end] === "}" && --depth === 0) break + } + expect(source.slice(start, end)).toContain("readRunStdin(") expect(source).not.toContain("if (!process.stdin?.isTTY)") }) From 2e53d46feb8ed1a46dd4579b92da28bde6b88f55 Mon Sep 17 00:00:00 2001 From: Haider Date: Tue, 20 Oct 2026 12:51:25 +0530 Subject: [PATCH 10/11] fix: keep a concurrent mirror's record and bound the signal-exit write - `mirrorBlock` undoes its record on a confirmed "disabled" only while the record is still its own, so a block mirrored concurrently keeps its target. - The signal exit waits at most 500ms for the notice's stderr write, and a write that throws does not block `process.exit`. - Tests: the subprocess test resolves on `close` so stdout is drained, and skips the `sh` pipeline case on Windows; the pending-lookup test waits on a signal from the fetch stub instead of a fixed sleep and cleans up in `finally`; new case for a concurrent record surviving a disabled lookup. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011chv9Rmu7jQXokum1WNcs7 --- .../src/altimate/workspace/memory-sync.ts | 8 ++-- packages/opencode/src/cli/cmd/run.ts | 13 ++++- .../altimate/workspace/memory-sync.test.ts | 48 ++++++++++++++++--- .../test/cli/run/run-stdin-process.test.ts | 5 +- 4 files changed, 61 insertions(+), 13 deletions(-) diff --git a/packages/opencode/src/altimate/workspace/memory-sync.ts b/packages/opencode/src/altimate/workspace/memory-sync.ts index 0fc6361075..63cff1314f 100644 --- a/packages/opencode/src/altimate/workspace/memory-sync.ts +++ b/packages/opencode/src/altimate/workspace/memory-sync.ts @@ -973,12 +973,14 @@ export async function mirrorBlock(block: MemoryBlock, directory?: string): Promi const binding = await currentBinding(directory) if (!binding) return // Recorded before the memory-setting lookup, which can wait on the service: an exit while it is pending - // leaves this block unsent too. Taken back only on a confirmed "disabled". + // leaves this block unsent too. Taken back only on a confirmed "disabled", and only while it is still + // this call's record: other blocks mirror concurrently and may have recorded since. const previous = mirrorRequests.last - mirrorRequests.last = { directory, binding } + const recorded: MirrorTarget = { directory, binding } + mirrorRequests.last = recorded const memory = await memoryStatus(binding) if (memory === "disabled") { - mirrorRequests.last = previous + if (mirrorRequests.last === recorded) mirrorRequests.last = previous return } if (memory !== "enabled") return diff --git a/packages/opencode/src/cli/cmd/run.ts b/packages/opencode/src/cli/cmd/run.ts index 8de4275477..521acce43e 100644 --- a/packages/opencode/src/cli/cmd/run.ts +++ b/packages/opencode/src/cli/cmd/run.ts @@ -1561,8 +1561,17 @@ You are speaking to a non-technical business executive. Follow these rules stric const notice = await unsent ?.then((m) => m?.unsentMemoryNoticeAfterRun(process.cwd(), 1_000)) .catch(() => null) - // process.exit does not wait for a pending write to a piped stderr. - if (notice) await new Promise((resolve) => process.stderr.write(notice + EOL, () => resolve())) + // process.exit does not wait for a pending write to a piped stderr; wait briefly, never for a + // reader that has stopped. + if (notice) + await new Promise((resolve) => { + setTimeout(resolve, 500) + try { + process.stderr.write(notice + EOL, () => resolve()) + } catch { + resolve() + } + }) process.exit(code) }) } diff --git a/packages/opencode/test/altimate/workspace/memory-sync.test.ts b/packages/opencode/test/altimate/workspace/memory-sync.test.ts index 1830d8e275..25173ee34b 100644 --- a/packages/opencode/test/altimate/workspace/memory-sync.test.ts +++ b/packages/opencode/test/altimate/workspace/memory-sync.test.ts @@ -463,16 +463,52 @@ describe("mirrorBlock", () => { const inner = globalThis.fetch let release!: () => void const held = new Promise((resolve) => (release = resolve)) + let entered!: () => void + const lookupStarted = new Promise((resolve) => (entered = resolve)) globalThis.fetch = (async (input: any, init?: any) => { - if (String(input).includes("/datamates/") && !String(input).includes("/memory")) await held + if (String(input).includes("/datamates/") && !String(input).includes("/memory")) { + entered() + await held + } return inner(input, init) }) as typeof fetch const mirror = mirrorBlock(block({ id: "pending-lookup" }), "/repo") - await Bun.sleep(20) - expect(lastMirrorTarget()).toEqual({ directory: "/repo", binding: BINDING as any }) - release() - await mirror - globalThis.fetch = inner + try { + await lookupStarted + expect(lastMirrorTarget()).toEqual({ directory: "/repo", binding: BINDING as any }) + } finally { + release() + await mirror.catch(() => {}) + globalThis.fetch = inner + } + }) + + test("a disabled lookup takes back only its own record, not one a concurrent mirror made since", async () => { + workspaces = [{ id: 42, name: "acme", memory_enabled: false }] + const inner = globalThis.fetch + let release!: () => void + const held = new Promise((resolve) => (release = resolve)) + let entered!: () => void + const lookupStarted = new Promise((resolve) => (entered = resolve)) + globalThis.fetch = (async (input: any, init?: any) => { + if (String(input).includes("/datamates/") && !String(input).includes("/memory")) { + entered() + await held + } + return inner(input, init) + }) as typeof fetch + const mirror = mirrorBlock(block({ id: "disabled-slow" }), "/repo") + const newer = { directory: "/other", binding: BINDING as any } + try { + await lookupStarted + // Another block's mirror records its own target while this lookup is pending. + ;(globalThis as unknown as Record)[Symbol.for("altimate.memory-sync.mirror-requests")].last = newer + } finally { + release() + await mirror.catch(() => {}) + globalThis.fetch = inner + } + expect(lastMirrorTarget()).toBe(newer) }) test("a workspace with memory turned off is not recorded: the block stays local by design", async () => { diff --git a/packages/opencode/test/cli/run/run-stdin-process.test.ts b/packages/opencode/test/cli/run/run-stdin-process.test.ts index 2156fe5dc6..2fc06710d0 100644 --- a/packages/opencode/test/cli/run/run-stdin-process.test.ts +++ b/packages/opencode/test/cli/run/run-stdin-process.test.ts @@ -12,7 +12,8 @@ function collect(child: ReturnType): Promise<{ code: number | null const started = Date.now() let out = "" child.stdout!.on("data", (chunk) => (out += chunk)) - return new Promise((resolve) => child.on("exit", (code) => resolve({ code, out, ms: Date.now() - started }))) + // `close`, not `exit`: it fires only after stdout is drained. + return new Promise((resolve) => child.on("close", (code) => resolve({ code, out, ms: Date.now() - started }))) } describe("readRunStdin on a real stdin", () => { @@ -36,7 +37,7 @@ describe("readRunStdin on a real stdin", () => { expect(JSON.parse((await collect(child)).out)).toEqual({ text: "the prompt\n", timedOut: false }) }) - test("a shell pipeline is read to EOF even when its first byte comes after the wait", async () => { + test.skipIf(process.platform === "win32")("a shell pipeline is read to EOF even when its first byte comes after the wait", async () => { const child = spawn("sh", ["-c", `sleep 0.6; echo piped | "${process.execPath}" "${FIXTURE}"`], { env }) expect(JSON.parse((await collect(child)).out)).toEqual({ text: "piped\n", timedOut: false }) }) From 88b5508b85181055c403988334ebb50f647d945b Mon Sep 17 00:00:00 2001 From: Haider Date: Tue, 20 Oct 2026 13:24:33 +0530 Subject: [PATCH 11/11] test: share the held workspace-lookup gate between the mirror tests Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011chv9Rmu7jQXokum1WNcs7 --- .../altimate/workspace/memory-sync.test.ts | 56 +++++++++---------- 1 file changed, 26 insertions(+), 30 deletions(-) diff --git a/packages/opencode/test/altimate/workspace/memory-sync.test.ts b/packages/opencode/test/altimate/workspace/memory-sync.test.ts index 25173ee34b..c2a0e3c8d2 100644 --- a/packages/opencode/test/altimate/workspace/memory-sync.test.ts +++ b/packages/opencode/test/altimate/workspace/memory-sync.test.ts @@ -449,6 +449,24 @@ describe("blockTitle", () => { }) }) +/** Holds the workspace-list request (the memory-setting lookup) open until `release`; `started` resolves + * once it is waiting. */ +function holdWorkspaceLookup() { + const inner = globalThis.fetch + let release!: () => void + let entered!: () => void + const held = new Promise((resolve) => (release = resolve)) + const started = new Promise((resolve) => (entered = resolve)) + globalThis.fetch = (async (input: any, init?: any) => { + if (String(input).includes("/datamates/") && !String(input).includes("/memory")) { + entered() + await held + } + return inner(input, init) + }) as typeof fetch + return { started, release, restore: () => (globalThis.fetch = inner) } +} + // ── write path ────────────────────────────────────────────────────────────── describe("mirrorBlock", () => { test("a mirror to a linked workspace is recorded, failed or not, so `run`'s exit checks what stayed local", async () => { @@ -460,53 +478,31 @@ describe("mirrorBlock", () => { }) test("the mirror is recorded while the memory-setting lookup is still pending, so an exit then still checks", async () => { - const inner = globalThis.fetch - let release!: () => void - const held = new Promise((resolve) => (release = resolve)) - let entered!: () => void - const lookupStarted = new Promise((resolve) => (entered = resolve)) - globalThis.fetch = (async (input: any, init?: any) => { - if (String(input).includes("/datamates/") && !String(input).includes("/memory")) { - entered() - await held - } - return inner(input, init) - }) as typeof fetch + const lookup = holdWorkspaceLookup() const mirror = mirrorBlock(block({ id: "pending-lookup" }), "/repo") try { - await lookupStarted + await lookup.started expect(lastMirrorTarget()).toEqual({ directory: "/repo", binding: BINDING as any }) } finally { - release() + lookup.release() await mirror.catch(() => {}) - globalThis.fetch = inner + lookup.restore() } }) test("a disabled lookup takes back only its own record, not one a concurrent mirror made since", async () => { workspaces = [{ id: 42, name: "acme", memory_enabled: false }] - const inner = globalThis.fetch - let release!: () => void - const held = new Promise((resolve) => (release = resolve)) - let entered!: () => void - const lookupStarted = new Promise((resolve) => (entered = resolve)) - globalThis.fetch = (async (input: any, init?: any) => { - if (String(input).includes("/datamates/") && !String(input).includes("/memory")) { - entered() - await held - } - return inner(input, init) - }) as typeof fetch + const lookup = holdWorkspaceLookup() const mirror = mirrorBlock(block({ id: "disabled-slow" }), "/repo") const newer = { directory: "/other", binding: BINDING as any } try { - await lookupStarted + await lookup.started // Another block's mirror records its own target while this lookup is pending. ;(globalThis as unknown as Record)[Symbol.for("altimate.memory-sync.mirror-requests")].last = newer } finally { - release() + lookup.release() await mirror.catch(() => {}) - globalThis.fetch = inner + lookup.restore() } expect(lastMirrorTarget()).toBe(newer) })