Repository navigation
chore: pin Bun 1.4.4 (#594) - #595
Merged
Merged
Conversation
Codex Review Summary: ❌ FailedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Owner
Author
|
repo-harness acceptance claim |
Ancienttwo
commented
Nov 13, 2026
Ancienttwo
left a comment
Ancienttwo
left a comment
Owner
Author
There was a problem hiding this comment.
repo-harness acceptance review
Round: 1
Subject: d2a683a
Tier: medium
Verdict: PASS
Reviewer: codex (gpt-6.1-terra)
Model relation: different-family
Read-only review against 0983ae9 and the joint decision on #594. Every remaining 1.4.3 hit is a dated record, downstream-corpus data, or an unrelated @emnapi range. The lock diff is only the workspace bin correction (matches packages/surfaces/package.json) and the bun-types 1.4.4 entries, generated with bun@1.4.4 install --lockfile-only. Workflow changes are versions and one comment. The package-manager constant, root manifest and product-version fixture agree.
NITs
- F1: PR body: the research-record count says 13 hits across 11 files; git grep returns 15 matching lines across 10 files. The grouped disposition is otherwise correct.
Owner
Author
|
repo-harness acceptance record
Model relation: different-family Findings:
|
Owner
Author
|
The chatgpt-codex-connector summary has status failed: its follow-up comment reports the Codex usage limit, and it left no inline finding on this head. The required review is the GPT receipt above (595#issuecomment-6859813112, PASS). Seat: orchestrator t7 |
Owner
Author
|
Gate sign-off for #595 at head d2a683a, base main 0983ae9.
The PR edits Seat: orchestrator t7 |
This was referenced Nov 13, 2026
Ancienttwo
added a commit
that referenced
this pull request
Nov 13, 2026
…S1) (#589) Refs #574 S1 of ADR-0058: one in-process, read-only CodeGraph read session, with the repository readiness check rebuilt on it. Base: `main` 60f2968 (0.14.1, #593 and Bun 1.4.4 from #595). ## Goal `inspectRepositoryCodeFacts` (doctor, `complete` debt, the private snapshot) and `assertRepositoryCodeFactsReady` (the checkpoint, debt, projection and CLI-provider gate) read the index only through `CodeGraph.openSync(root, { readOnly: true })`. They start no CodeGraph process and run no `git status` or `git diff`. ## Changes **Read session** (`codegraph-adapter/src/read-session.ts`, new) - `openCodeGraphReadSession` / `withCodeGraphReadSession` open one handle per operation and close it in `finally`. - The session offers `version()`, `compatibility()`, `freshness()` (memoized per session), `revision()`, `unchangedSinceOpen()`, `graph` and `close()`. - Open states, checked in this order: - `lstat(.codegraph)` fails with `ENOENT` or `ENOTDIR`: `index-missing`. Any other `lstat` error: `index-open-failed`. - `.codegraph` is a link, dangling or not: `index-open-failed`. - `stat(codegraph.db)` fails with `ENOENT` or `ENOTDIR`: `index-missing`. Any other `stat` error: `index-open-failed`. This is the same rule as for `.codegraph`. - A live writer lock (#426): `build-in-progress`, and the index is never opened. - The read-only open or the first revision read throws: `index-open-failed`. - Otherwise `open`. - **Coherent read (design G4):** `unchangedSinceOpen()` re-checks the writer lock and compares the revision taken at open with the revision after the reads. When anything moved, readiness answers not-ready (`index-incomplete`, every index field `null`), and the gate refuses with `code-facts-not-ready` and `retryable: true`. It never passes. - `freshness()` is CodeGraph's own change test: the provider's enumeration, plus its change hash (`readBoundedSourceSync`, `oversizeStamp`, `hashContent`). - An indexed file that left the enumeration or the disk counts as removed. - A file that exists but cannot be read counts as modified, because it cannot be attested. - `pinnedCodeGraphProvider()` takes its version from the manifest of the loaded platform bundle, so `REQUIRED_CODEGRAPH_VERSION` stays the only pin. - `CODEGRAPH_PROVIDER_INTERNAL_EXPORTS` is the Decision 7 table of provider-internal modules and exports. **Readiness** (`codegraph-adapter/src/index.ts`) - **One reader.** `inspectRepositoryCodeFacts` reads both tiers, compatibility and freshness. `assertRepositoryCodeFactsReady` reads compatibility only and reads no source file. - `withExistingCodeGraphSnapshot` calls the same `inspectRepositoryCodeFacts(snapshotRoot)`. The CLI probe and `CodeGraphSnapshotOptions` are deleted. S4 removes the snapshot itself. - `RepositoryCodeFactsV1` keeps its shape. **C2:** `unavailable.reasonCode` gains `"index-open-failed"`. - **`binary` removed from the read options.** `RepositoryCodeFactsInspectionOptions` is now `{ provider? }`. `CodeGraphCliProvider` takes an optional third argument, `readProvider`; its `binary` now only builds the index (`sync`). `prepareArchitectureDocumentationProjectionSnapshot` accepts `readProvider` for its gate. `runtime-daemon/src/flow-selector-check.ts` no longer passes `binary` to the snapshot. **A `.codegraph` without `codegraph.db` is `index-missing`** (round-2 item 1). The design table maps an absent database to `AC_CODE_FACTS_UNAVAILABLE` / `index-missing`, and the surviving snapshot path (`index.ts:1225` on main) already refused it that way. Round 1 had returned `not-initialized` (`AC_CODE_FACTS_NOT_READY`); that state is gone. Doctor reports `index-missing` with `indexPresent: false`, and the checkpoint gate refuses with `index-missing`. A 0-byte or schema-less `codegraph.db` now gives `index-open-failed` instead of `not-initialized`. **Contracts** - **C2:** the `failure.ts` policy table gains `index-open-failed` (`AC_CODE_FACTS_UNAVAILABLE`, `inspect-diagnostics`, `retryable: false`). **Agreed scope widening** - `runtime-daemon/src/architecture-debt-code-facts.ts`: the two inspect calls drop `binary` and take the injected `provider`. - **1a:** `surfaces/cli/src/main.ts` passes `process.execArgv` to the daemon it starts, only when `process.versions.bun` is set. Under Node nothing new is passed. - **2a was removed under Bun 1.4.4.** The read session, selector index and symbol reader close exactly as on main, with no GC (see "Bun 1.4.4"). - **1b:** the `bun.lock` hand-edit is dropped. Main's lock was regenerated by Bun 1.4.4 (#595) and already points the workspace `archctx` bin at `./cli/bin/archctx`. This PR changes no byte of `bun.lock`. `bun install --frozen-lockfile` under 1.4.4 is accepted, and `readlink node_modules/.bin/archctx` → `../@archcontext/surfaces/cli/bin/archctx`. - **Dev runtime on darwin:** `packages/surfaces/cli/bin/bun-darwin-sqlite.ts` runs only under Bun on darwin. It calls `bun:sqlite` `Database.setCustomSQLite` with a library from a closed list (`/opt/homebrew/opt/sqlite/lib/libsqlite3.dylib`, `/usr/local/opt/sqlite/lib/libsqlite3.dylib`), and throws with a `brew install sqlite` hint when neither exists. - The `bunfig.toml` test preload imports it. Dev `bin/archctx` imports it and preloads it into the CLI process it starts. - The probe's CI stderr line is removed. - Still needed under Bun 1.4.4 (see below). - It never reaches the npm tarball: the package boundary audit passes, and a release-style bundle has no `setCustomSQLite`, no `bun-darwin-sqlite` and no `libsqlite3.dylib`. **Refactors** - SDK loading, the platform-bundle lookup and the writer-lock check move from `index.ts` into `read-session.ts`. `REQUIRED_CODEGRAPH_PACKAGE` and `REQUIRED_CODEGRAPH_VERSION` are re-exported unchanged. - `codeGraphSnapshotRuntime` and the snapshot's source scanner use the shared bundle lookup. - `repositoryCodeFactsFromStatus` is the one evaluator of an index state. - The gate's unreachable `pending-source-changes` branch is removed, because the compatibility tier never measures pending changes. **Paths not yet moved, each with the slice that moves it.** These are sequencing, not fallbacks. - The projection handshake (`prepareProjectionCodeFacts`) and the import population still use the CLI `status` and `query`. S2+S3 move them. - The snapshot copy and its Node query child stay until S4. ## Round 4 - **R3-1:** the database check is a `stat`, with the same rule as the `.codegraph` `lstat`: only `ENOENT` and `ENOTDIR` mean `index-missing`, and any other error is `index-open-failed`. - Test: an unreadable `.codegraph` (chmod 000, so `codegraph.db` cannot be stat'ed) and an unsearchable project directory (chmod 000, so `.codegraph` cannot be lstat'ed) are both `index-open-failed`, and neither opens the index. - The test is skipped on win32 and when running as root. - **Opus F1:** the open-state list above now states the `lstat` rule exactly. - **Opus F3:** the snapshot comment now says readiness opens the copied index in-process through the read session, while the queries still run in the provider's Node child until S4. The snapshot's `rmSync` is unchanged. ## Bun 1.4.4 (rebased onto 60f2968) Both checks were run locally with Bun 1.4.4 on macOS. - **macOS sidecar: still needed.** After `codegraph sync` removed the sidecar files, Bun 1.4.4 with the system SQLite (3.51.0) still fails the read-only open with "unable to open database file". The darwin module stays unchanged. - **Windows EBUSY guard: removed.** - Measured with `lsof`: after `close()`, Bun 1.4.4 holds 0 of the 5 database files it had open. Bun 1.4.3 held 6 of 6 until a GC after the handle was released. - In all 22 protocol fixtures, the test process held 0 files under the fixture root after `daemon.stop()` without any GC, against 8 under 1.4.3, and had no child process. - So `collectReleasedCodeGraphHandles` and its three call sites are deleted, and so is the test-side `Bun.gc(true)`. - The protocol fixture keeps only rmSync's `maxRetries: 5, retryDelay: 100` on win32, for a transient hold outside the process. - This is a macOS measurement. The Windows shards confirm it on this head; if `EBUSY` returns there, the drop commit (9db4488) is the one to revert. ## Round 3 - **R2-1:** the snapshot keeps C2. When the session inspector answers `unavailable` / `index-open-failed`, `withExistingCodeGraphSnapshot` throws `index-open-failed`; every other unavailable answer stays `code-facts-unavailable`. Test: a non-SQLite `codegraph.db` through the snapshot gives `AC_CODE_FACTS_UNAVAILABLE` / `index-open-failed`. - **R2-2:** `lstat(.codegraph)` failing with `ENOENT` or `ENOTDIR` is `index-missing`. Any other failure (permission, I/O) is `index-open-failed`, because it does not prove the index is absent. - **Opus R1:** the fake gains `readFailsAfterOpen`, which throws only from the build info that `compatibility()` reads after the open. That test asserts `index-open-failed` with open = close = 1. The former `readFails` test is renamed to say it covers a read during the open. - **Opus R2:** a moving index refuses with `retryable: true` only when the loaded provider has the pinned version. With a version mismatch it refuses `retryable: false` with `install-compatible-codegraph`, asserted in the G4 test. ## Windows EBUSY (round-2 item 5, superseded by Bun 1.4.4) Under Bun 1.4.3, an instrumented local run (not committed) of all 23 protocol tests showed: - 8 files held under the fixture root before `Bun.gc(true)`, and 0 after it; - no child process, with the daemon running in-process. The remaining Windows hold was therefore outside the test process; the rmSync retries cover it. Under 1.4.4 no GC is needed (above), and the retries stay. ## Changed or removed assertions, and their coverage **`codegraph-adapter/test/projection-codegraph-status.test.ts`** (inspect section, rewritten against an injected provider). The `projectionCodeGraphStatusIssues` describe is kept unchanged, because S2+S3 still evaluate CLI `status -j` with it. - **"unavailable CodeGraph" (nonexistent binary)** → "a provider that cannot load". The fake is a `Proxy` whose `version` getter throws `CodeGraphProviderUnavailableError`. That is the error `pinnedCodeGraphProvider()` throws when the SDK, the platform bundle, its manifest or a Decision 7 export is missing. The real loader cannot be made to fail in a running process. The Decision 7 bump guard (`read-session.test.ts`) asserts that every listed module path and export exists in the pinned bundle. - **"missing index from --version alone, never status or sync"** → "missing index from the provider version alone and opens nothing" (open count 0). - **"ready index calls only --version and status"** → "ready index from one read-only session and closes it" (open = close = 1). In addition, `read-session.test.ts` reads a real index with no holder process, and its git spy proves no `git status` or `git diff` runs. - **Per-issue `test.each`:** - not-initialized, index-incomplete, extraction, project-path, worktree-mismatch and pending → incompatible-index `test.each` (bundle version, build version, unknown build version, partial, pending refs, stale extraction, other project root), plus the pending-changes test. - `not-initialized` → `index-missing` (item 1). - **`worktree-mismatch` is dropped** for the session. `openSync(root)` never walks up to another worktree's index (ADR-0058 table), so the session sets `worktreeMismatch: null`. The evaluator's `worktree-mismatch` branch stays covered by the kept `projectionCodeGraphStatusIssues` tests. - **"binary version diagnostics do not copy raw output"** → "issue messages are the closed vocabulary, never provider output". The open-failure test also asserts the raw provider error never reaches the facts. - **"stale extraction and pending as issues"** → the stale-extraction case of the `test.each` plus the pending-changes test. - **"binary of another version"** → the bundle-version case. - **"failing or invalid status is unavailable" and "status JSON not an object"** → no status JSON is parsed any more. Replaced by "an open that fails" and "a read that fails after the open", both `index-open-failed`. - **"always passes CODEGRAPH_NO_DOWNLOAD to --version and status"** → removed, because readiness starts no process at all: no environment to pass. `code-facts-not-ready.test.ts` asserts an empty process log for every readiness path. The remaining CLI read, the import `query` until S2, still passes `CODEGRAPH_NO_DOWNLOAD=1`, and `architecture-debt-code-facts.test.ts` still asserts it (`query=1`). - **"packaged shim with no platform bundle does not download"** → removed, because readiness no longer runs the npm shim. The SDK loader (`npm-sdk.js`) never downloads ("we never trigger a network download from inside require()"); it only reuses an installed or cached bundle. A missing bundle surfaces as `CodeGraphProviderUnavailableError` → `codegraph-unavailable`, which the Proxy test covers. The shim still builds the index (`init`, `sync`), and that is unchanged. **`codegraph-adapter/test/build-lock.test.ts`.** Live, stale and malformed lock, ported from the CLI call log to provider open counts: a live lock never opens the index, a stale or malformed one opens and closes it once. **`codegraph-adapter/test/codegraph-adapter.test.ts`.** The two shim tests inject `readProvider`; their `sync` and `query` assertions are unchanged. **`surfaces/cli/test/code-facts-not-ready.test.ts`** - Process-log expectations change from `--version\nstatus\n` to empty: no readiness process. - The checkpoint and debt gate's issues are compatibility-only. - "present uninitialized index" → "`.codegraph` without a database is `index-missing`" (item 1). - The projection-snapshot gate injects `readProvider`. - The real incomplete index (`initSync`) still yields version-mismatch plus index-incomplete. **`surfaces/cli/test/doctor-code-facts.test.ts`.** An empty `.codegraph` is now `index-missing` with `indexPresent: false` (item 1). Doctor `ok` is unchanged. **`runtime-daemon/test/architecture-debt-code-facts.test.ts`.** Readiness comes from the injected provider, which opens and closes twice around the reads. The process log keeps only `query=1` (S2), and a missing or stale index starts nothing. **`runtime-daemon/test/flow-selector-check.test.ts`, `surfaces/cli/test/ref-impact.test.ts`.** Their subprocess runs the dev launcher `bin/archctx` instead of `src/main.ts`. The snapshot's readiness now reads in-process (item 3), so on darwin the CLI process needs the same SQLite library as the test process. The assertions are unchanged. **`surfaces/cli/test/projection-protocol-lifecycle.test.ts`.** Cleanup uses rmSync retries on win32; no assertion changed. ## New tests - `read-session.test.ts`: - each open state, the dangling link, and the coherent-read case (an injected revision change between reads gives not-ready and a retryable refusal); - compatibility/revision, freshness counts and memoization, an unreadable source, and close-on-return/throw with refusal after close; - a real index read in-process with no holder: ready; edit, add and delete counted; an oversize file fresh; `.git/index` mtime and the database sha256 unchanged; the git spy; - the Decision 7 bump guard. - `scripts/codegraph-selector-node-integration.mjs` (`verify:pre`, all three OSes) runs the read session under Node with no holder: compatibility and freshness, pending counts, the compatibility-only gate, the git spy, and `.git/index` and the database unchanged. A live-writer case did not fit this script cheaply and is not added; the injected revision test covers G4. **Test budget** (`git diff --numstat origin/main`, head 8649579) - Product (`src`, `bin`, `main.ts`, contracts): +556 / −144. - Config: `bunfig.toml` +1 / −1. `bun.lock` unchanged. - Tests: +656 / −302, including the 109-line fixture. - Script: +74 / −2. ## Verification (Bun 1.4.4) - `bun install --frozen-lockfile`: accepted. - `bun run typecheck`: exit 0. - Head 9db4488: `bun test --timeout 120000` over codegraph-adapter, code-facts-not-ready, doctor-code-facts, architecture-debt-code-facts, flow-selector-check, ref-impact, complete-architecture-debt, projection-protocol-lifecycle, docs-anchors, local-product-e2e and the contracts tests: 699 pass, 0 fail. - Head 8649579: `bun run typecheck` exit 0. codegraph-adapter, code-facts-not-ready, doctor-code-facts, architecture-debt-code-facts, flow-selector-check, ref-impact and complete-architecture-debt: 162 pass, 0 fail. - `bun run test:codegraph-selector-node`: verified. - `node scripts/packaged-cli-smoke.mjs`: OK. - `node scripts/package-boundary-audit.mjs`: passed. ## Risk - On macOS, the Bun dev and test runtime needs a Homebrew SQLite (the error says how to install it). The product runtime, Node, is unchanged. - The pinned provider-internal modules break loudly on a provider bump, through the bump guard. - A `.codegraph` without a database moves from not-ready to `index-missing`, the same refusal as no index at all. ## Rollback Revert this PR. There is no data migration; the WAL sidecar files are managed by SQLite.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #594. Tier Medium (one GPT review).
Pins Bun 1.4.4 everywhere a 1.4.3 pin is still in force, and regenerates
bun.lockwith Bun 1.4.4 itself (never hand-edited). This is the deliberate lock regeneration #594 asks for, and it replaces the abandoned hand-edit ofbun.lock:56in 574#issuecomment-6856897068.What changed
package.jsonpackageManagerbun@1.4.3→bun@1.4.4; devDependencybun-types1.4.3→1.4.4.github/workflows/verify.ymlbun-version:sites (L42, L71, L114, L182, L245, L295) and the L82 comment.github/workflows/windows-acl-diagnostics.ymlbun-version:at L33scripts/verify-ci.mjspackages/contracts/src/product-version.tsARCHCONTEXT_PACKAGE_MANAGERpackages/contracts/fixtures/valid/product-version-manifest.jsonpackageManagerdocs/runbooks/governance-revalidation.mddocs/runbooks/verify-sharding.mdbun.lockBun version guard
scripts/check-bun-version.mjshas no literal pin: it comparespackage.json'spackageManageragainstprocess.versions.bun. Updatingpackage.jsonmoves the guard, so the file itself is unchanged.tests/toolchain-guard.test.tsstill passes (it asserts againstbun@0.0.0, not a real pin).Every
git grep -n "1\.4\.3"hitgit grep -n "1\.4\.3"on the branch after the change returns only the hits below. Everything else is gone.Changed (in-force pins and descriptions of them)
.github/workflows/verify.yml:42,71,114,182,245,295.github/workflows/verify.yml:82.github/workflows/windows-acl-diagnostics.yml:33package.json:6packageManagerpackage.json:89bun-typesdevDependencypackages/contracts/src/product-version.ts:3ARCHCONTEXT_PACKAGE_MANAGERpackages/contracts/fixtures/valid/product-version-manifest.json:8scripts/verify-ci.mjs:132docs/runbooks/governance-revalidation.md:3docs/runbooks/verify-sharding.md:23,33bun.lock:15,375Left (out of scope, with reason)
bun.lock:197@emnapi/core/@emnapi/runtimesemver ranges^1.4.3packages/local-runtime/model-store-yaml/test/fixtures/native-context/downstream-corpus/corpus.json:7(oracleBunVersion)deploy/release-checklists/archctx-0.8.0.md:61,-0.11.4.md:16,-0.12.0.md:35,-0.13.0.md:34,42,-0.14.1.md:34docs/researches/*(15 hits across 10 files)docs/verification/issue-328-d5-read-snapshot-benchmark.json:23docs/verification/issue-328-d5-read-snapshot-evidence/README.md:40Corpus assertion check: no test asserts
oracleBunVersion.native-context.test.tsreadsinputCount,inputs,sourceDigest,expectedandbaselineSharedValueDigestfromcorpus.jsonand never touchesoracleBunVersion, so no test fails and the fixture stays untouched.bun.lockdiffProduced by, with no other arguments and no hand edit:
Exactly the two expected changes and nothing else: the stale workspace bin line (
./cli/src/main.ts→./cli/bin/archctx) and thebun-types1.4.3 → 1.4.4 entry (workspace devDependency line plus the package entry with its new integrity hash).packages/surfaces/cli/bin/archctxexists on this commit, so the corrected bin target resolves. No other lock entry moved; 3 insertions, 3 deletions.Verification under Bun 1.4.4
All commands run on
d2a683abwithnpx -y bun@1.4.4(bun install v1.4.4, revision 38afee6c8).bun install --frozen-lockfilebun-types@1.4.4bun run typecheck(tsc --noEmit)bun test packages/contractsbun test scripts/verify-sharding-workflow.test.ts scripts/governance-verify-workflow.test.ts tests/toolchain-guard.test.tsEmpty-pattern probe (
scripts/verify-ci.mjs:132)The comment claims Bun deliberately exits 1 when a test-name pattern matches no tests, and the discovery guard depends on exactly that. Probed on a scratch directory containing one test named
alpha:The behavior holds under 1.4.4: exit code 1, and the message still matches the guard's regular expression
/error: regex "a\^" matched 0 tests\. Searched (\d+) files?\b/. Only the version in the comment changed; the guard logic is untouched.Risk and rollback
Risk: low.
./cli/bin/archctx). It is the bug 1.4.4 fixes, and the path exists on this commit;bun run typecheckand the contracts suite pass on the regenerated lock.EBUSYand macOS Bun sidecar findings under 1.4.4 and shrinks those guards if either changed.scripts/check-bun-version.mjswith "This repository requires bun@1.4.4". That is the guard working as designed.Rollback: revert this PR. It is a single commit (
d2a683ab) with no migration state, no data change and no schema change, sogit revert d2a683abrestores the 1.4.3 pins and the previous lock in one step. If a hosted-runner-only failure appears, a follow-up PR can dropbun.lockback to the previous commit's copy while keeping the pin, but the revert is the default.Seat: orchestrator t7