Skip to content

Add missing test/setup-scm-loader.ts preload for .scm imports - #1286

Open
lab1207 wants to merge 2 commits into
CodebuffAI:mainfrom
lab1207:fix/missing-scm-loader
Open

Add missing test/setup-scm-loader.ts preload for .scm imports#1286
lab1207 wants to merge 2 commits into
CodebuffAI:mainfrom
lab1207:fix/missing-scm-loader

Conversation

@lab1207

@lab1207 lab1207 commented Sep 6, 2026

Copy link
Copy Markdown

Problem

cli/bunfig.toml lists three test preloads:

preload = [
  "../sdk/test/setup-env.ts",
  "../test/setup-scm-loader.ts",
  "./test/setup-agents-artifact.ts",
]

But test/setup-scm-loader.ts does not exist in the public mirror. Any test that
reaches @codebuff/sdk (which re-exports code-map, which imports .scm
tree-sitter query files) dies at import time with "Unknown file type" — which bun
reports as "Unhandled error between tests", so a fresh clone shows a wall of
failing test files with no obvious cause. (The same file is referenced in
docs/testing.md, there under a corrupted path ../test/n.ts.)

Fix

One new file: a bun plugin() that loads .scm imports as a default-exported
string — the same thing the bundled build does. Registered via the preload that
bunfig.toml already declares, so no config change is needed.

Verification

cd cli && bun test src/commands src/utils/__tests__ src/state:

  • 1,576 pass with this change
  • 13 failures remain, all pre-existing Windows path-separator issues in
    export-conversation.test.ts (expects /project/..., gets \project\...) —
    present on main without this change, happy to file a separate issue for those

Tested on Windows 11 / bun 1.3.14.

cli/bunfig.toml lists test/setup-scm-loader.ts among its preloads, but the
file was never exported to the public mirror. Any test reaching the SDK
barrel (which re-exports code-map, which imports .scm tree-sitter query
files) threw "Unknown file type" at import time, which bun surfaces as an
unhandled error between tests — a fresh clone showed a wall of dead test
files with no obvious cause.

The plugin registers a bun loader that imports .scm files as a
default-exported string, matching what the bundled build does. Verified
against the CLI suite: 1,576 pass, with only the pre-existing Windows-path
failures in export-conversation.test.ts remaining (unrelated).

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
@codebuff-team

Copy link
Copy Markdown
Contributor

Good catch and a clean, minimal fix. This is exactly the kind of gap you'd expect from a squashed export: cli/bunfig.toml references ../test/setup-scm-loader.ts, but the file itself never made it into the mirror, so any test that transitively imports @codebuff/sdk → code-map → .scm files fails at import time with an opaque "Unknown file type" / "Unhandled error between tests" message.

The fix itself is sound: a bun.plugin() onLoad hook that reads .scm files as text and wraps them as a JS module with a default string export, matching what the bundled build already does. That's the standard way to shim non-JS asset imports in bun, and it's scoped tightly to the .scm extension so it won't interfere with anything else.

A few things worth double-checking before this is ported:

  • Confirm the private repo's actual setup-scm-loader.ts (if one exists there) matches this shape, or that this is genuinely absent and needs authoring fresh — since this is inferred from bunfig.toml + code-map's import style rather than copied from a known-good source.
  • The PR mentions docs/testing.md has a corrupted path (../test/n.ts) referencing the same file; consider filing that as a doc fix too, or folding it into this PR, since it's directly related and one-line.
  • No new test asserts the loader behavior itself (e.g., a trivial test importing a .scm fixture) - worth adding so a future refactor of the loader doesn't silently regress.

Overall: small, correctly diagnosed, verified against the actual failure mode. Worth a maintainer's few minutes to confirm and port.

@codebuff-team codebuff-team added bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree labels Sep 6, 2026
Per review on CodebuffAI#1286: nothing asserted the loader behavior itself. This
imports the go tree-sitter query .scm directly and asserts it arrives as
a non-empty string with capture syntax intact. Without the preload the
file fails to load at all (verified: running it without the cli/bunfig
preload fails), so a future refactor cannot silently regress the loader.

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
@lab1207

lab1207 commented Sep 6, 2026

Copy link
Copy Markdown
Author

Thanks for the review! Pushed 9317d5c addressing the actionable ask:

  • Loader regression test: cli/src/__tests__/scm-loader.test.ts imports
    the go tree-sitter query .scm directly (a real file shipped by
    code-map, the same import the SDK barrel transitively reaches) and asserts
    it loads as a non-empty string with capture syntax intact. If the preload
    goes missing, the whole file fails to load rather than passing vacuously.
    Verified the discrimination is real: run from the repo root — where
    cli/bunfig.toml's preloads don't apply — the file fails exactly as a
    fresh clone without this PR does.

On the other two asks:

  • docs/testing.md ../test/n.ts: I could not reproduce this — the
    current mirror references the correct ../test/setup-scm-loader.ts path
    (in the "global-jsdom / --preload" example near the end), and nothing
    matches test/n.ts anywhere in the repo. That sentence in my PR body was
    an error on my part, sorry for the noise. No doc change needed; happy to
    add one if you're seeing a stale copy.
  • Does a private setup-scm-loader.ts already exist? I have no access
    to the private repo, so this is indeed authored fresh — inferred from
    cli/bunfig.toml + code-map's .scm import sites + the bundler's own
    rule (sdk/scripts/build.ts maps .scm → loader 'text' with a default
    string export, which the plugin mirrors).

CLI suite on Windows 11 / bun 1.3.14 after the new commit: 1,584 pass, only
the documented pre-existing exportConversation path-separator failures
(#1290).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants