feat(javascript): replace the in-situ test runner with voxgig/omni - #84
Conversation
First port migrated per omni's DOCS section 8: the 302-line hand-ported
test/runner.js is deleted, and test/omni.js (a ~30-line resolver) now
locates a local voxgig/omni checkout (OMNI_HOME, then sibling paths -
the same convention sekreto's ports use; omni is deliberately not
published) and re-exposes omni's struct-compat shim behind the old
require('./runner') API. struct.test.js and client.test.js change by
one import line each; the corpus, the SDK, and every assertion are
untouched. The library itself never depends on omni - test-only, so the
zero-runtime-dependency rule is unaffected.
CI's test-javascript job checks out voxgig/omni to .omni and exports
OMNI_HOME. Verified locally: all 95 tests pass through the shim - the
same 95 that passed with the in-situ runner - and eslint/prettier are
clean. Per-port migration status is tracked in omni's doc/plan/
register.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EHLH9d5Jaiy6QbMKja22nt
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd5f6e65ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (process.env.OMNI_HOME) { | ||
| candidates.push(process.env.OMNI_HOME) | ||
| } |
There was a problem hiding this comment.
When OMNI_HOME is relative, such as OMNI_HOME=../../omni npm test from javascript/, existsSync resolves it against the process working directory, but the subsequent require() resolves the returned relative path against test/omni.js. The marker can therefore be found successfully and then the compatible runner fails with MODULE_NOT_FOUND; normalize the environment value with Path.resolve() before adding it to the candidates.
AGENTS.md reference: javascript/AGENTS.md:L63-L65
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 05eabad — OMNI_HOME is now resolved against the cwd before probing, with a comment explaining the existsSync/require asymmetry. Verified OMNI_HOME=../../omni npm test passes all 95 tests.
Generated by Claude Code
| } | ||
| } | ||
|
|
||
| throw new Error('struct: voxgig/omni checkout not found - set OMNI_HOME') |
There was a problem hiding this comment.
On a fresh standalone checkout, the documented JavaScript workflow runs only npm install followed by npm test, but npm does not install Omni and none of these filesystem candidates exists, so both test files abort here before any tests execute. The workflow checkout fixes this only in GitHub Actions; local contributors and other CI consumers need an explicit clone/setup step, an automated bootstrap, or an in-tree fallback.
AGENTS.md reference: javascript/AGENTS.md:L35-L42
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed via docs in 05eabad: javascript/AGENTS.md now states up front that tests need a local voxgig/omni checkout (sibling clone or OMNI_HOME), with the exact command. An automated clone-on-test bootstrap was considered and rejected — implicit network operations inside npm test would be worse than the resolver's current instructive abort (struct: voxgig/omni checkout not found - set OMNI_HOME). This mirrors how voxgig/sekreto's ten ports already consume omni.
Generated by Claude Code
| repository: voxgig/omni | ||
| path: .omni |
There was a problem hiding this comment.
Because this checkout specifies another repository without a ref, it follows that repository's default branch; the actions/checkout ref documentation states that a different repository otherwise uses its default branch. As a result, rerunning the same Struct commit after an Omni push can execute different runner code, change corpus semantics, or fail CI without any change here, so pin the known-compatible Omni commit or tag and update it deliberately.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Deliberately left floating for now, matching the ecosystem's existing convention: voxgig/sekreto's conformance CI checks out omni's default branch the same way, and omni's own struct-compat gate floats on struct's default branch in the reverse direction. During active migration that coupling is the point — a breaking omni change should redden consumers immediately rather than after a pin bump. Versioning the cross-repo contract (pins/tags in both directions, decided together) is tracked as plan item 0.5 in omni's doc/plan/progress.md register.
Generated by Claude Code
…requirement existsSync probes a relative OMNI_HOME against the process cwd, but require() resolves relative paths against test/omni.js - so a relative OMNI_HOME could pass the probe and then fail the require. Resolve it against the cwd up front. Verified: OMNI_HOME=../../omni npm test now passes (95 tests). javascript/AGENTS.md now states up front that the tests need a local voxgig/omni checkout (sibling or OMNI_HOME) - on a fresh standalone clone the resolver aborts with an instructive message rather than a module error. Addresses two review comments on #84. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EHLH9d5Jaiy6QbMKja22nt
Register rows 0.2 and 4.1 now name the port count (13 of 23) and the three defects review found in the version-1 validation; the struct javascript row cites voxgig/struct#84. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EHLH9d5Jaiy6QbMKja22nt
…truct-compat
Five more ports carrying the corrected design (checkset over the
authored group, presence checks for the OMNI block, requires list and
entry id, and the negative tests pinning both), plus kotlin's late
entrysummary extraction, which matches canonical and handles the
non-map entry a strict-mode failure can now produce.
The struct-compat gate was broken by struct's own migration, in two
ways, both found by running it:
- It rewrote require('./runner'), but a migrated struct port imports
require('./omni') - its resolver for this very shim. Both spellings
are now rewritten, since struct migrates port by port and the gate
must work either side of each swap. Without this, omni CI would have
gone red the moment voxgig/struct#84 merged.
- A relative STRUCT path (the documented ) was
substituted verbatim into test files that run from a temp directory,
so struct's SDK and corpus could not be found. STRUCT is now resolved
to an absolute path up front.
Gate verified green against the migrated struct: 95/95, struct's own
test files, omni's runner.
DOCS section 9 records zig's variance for spec-load refusals: typed
error values rather than the canonical message strings, one-for-one
with the canonical failure modes.
NOT EXECUTED: clojure, dart, scala, swift, zig and kotlin toolchains are
absent here; these six were written and design-audited against canonical
but not run. The eleven ports with toolchains all pass (typescript,
javascript, python, ruby, php, perl, go, rust, java, cpp, c).
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EHLH9d5Jaiy6QbMKja22nt
The register's cross-repo rows update when the change merges there. With voxgig/struct#84 merged, struct's javascript port is off its in-situ runner: 1 of 24. Verified against both merged mains: make struct-compat is 95/95 with struct's javascript tests importing require('./omni'), which is the cross-repo path the gate fix in #5 was written for. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EHLH9d5Jaiy6QbMKja22nt
2 participants
First port migrated to the shared voxgig/omni test runner, per omni's DOCS §8 migration path. The cross-repo plan and per-port status live in omni's
doc/plan/register (added in the companion omni PR).What changed
javascript/test/runner.js(302 lines) is deleted. The runner is now voxgig/omni, consumed as a local checkout — omni is deliberately not published to a registry for now.javascript/test/omni.js(~30 lines) resolves the checkout ($OMNI_HOME, then sibling paths, first directory carryingspec/fib.json— the same convention sekreto's ten ports already use) and re-exposes omni's struct-compat shim behind the oldrequire('./runner')API.struct.test.js/client.test.jschange by one import line each. The corpus, the SDK, and every assertion are untouched.test-javascriptjob checks outvoxgig/omnito.omniand exportsOMNI_HOME.AGENTS.mdnote the migration and the resolver.The library itself never depends on omni — test-only, so the zero-runtime-dependency rule is unaffected.
Verification
npm test, node 24).npm run lint(ESLint) clean;test/omni.jspassesprettier --check.struct-compatCI gate independently runs this repo'sstruct.test.js/client.test.jsagainst the shim on every omni push.Notes for review
build/test/test.jsoncarries noOMNIversion block, so it runs under omni's legacy (version-0) semantics — behaviour is identical by construction; any future tightening is opt-in via the spec version.doc/plan/progress.md(typescript next, resolving themakeContext/contextifydrift and the@voxgig/sdkgencoupling noted there).🤖 Generated with Claude Code
https://claude.ai/code/session_01EHLH9d5Jaiy6QbMKja22nt
Generated by Claude Code