fix(local): redirect all @agent-relay/sdk subpaths and @agent-relay/config in loader#92
Conversation
…onfig in loader The sdk-runtime-loader.mjs that Ricky generates per-run only redirected `@agent-relay/sdk/workflows` to the bundled SDK. Workflow files routinely import other SDK subpaths (e.g. `@agent-relay/sdk/github` for `createGitHubStep`) and the sibling `@agent-relay/config` package (for `ClaudeModels` / `CodexModels`). Those import paths fell through to standard node resolution and failed in consumer repos that hadn't also `npm install`ed the SDK locally — defeating the point of having Ricky bundle it. Reproducible failure (cloud worktree, no local @agent-relay deps): Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@agent-relay/sdk' imported from .../workflows/proactive-runtime-m1.ts at resolve (file:///.../sdk-runtime-loader.mjs:6:10) at nextResolve (...) The bundled SDK already exposes the missing subpaths via its package exports map (`./github`, `./client`, `./communicate/...`, `./broker-path`, etc.), and `@agent-relay/config` ships alongside in the same scope dir inside Ricky's node_modules. The loader can resolve all of them against the bundled location without consumer-repo state. Behavior after this change: • `@agent-relay/sdk/workflows` → bundled (unchanged) • `@agent-relay/sdk` (root) → bundled • `@agent-relay/sdk/<anything>` → resolved against bundled SDK root, so the package's own exports map handles the subpath • `@agent-relay/config` → bundled • `@agent-relay/config/<anything>` → resolved against bundled config package root • All other specifiers → unchanged (delegate to nextResolve) All 113 src/local/entrypoint.test.ts tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughGenerates an enhanced Node ESM loader to map ChangesESM Loader Package Resolution & Regression
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/local/entrypoint.ts`:
- Around line 936-966: The loader's resolve function is concatenating file://
URLs (sdkRootUrl/configRootUrl) into a resolved path and calling nextResolve,
which bypasses package exports; instead, for specifiers that start with
SDK_SUBPATH_PREFIX or CONFIG_SUBPATH_PREFIX call nextResolve with the original
bare subpath (e.g. the incoming specifier) but pass a context.parentURL anchored
inside the target package root (use sdkRootUrl + '/' or configRootUrl + '/' as
the parentURL) so Node will re-run package exports resolution; update the
exported resolve function's branches that handle SDK_SUBPATH_PREFIX and
CONFIG_SUBPATH_PREFIX to re-resolve the original specifier via
nextResolve(specifier, { ...context, parentURL: sdkRootUrl + '/' }, ...) and
similarly for configRootUrl.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 46098ead-c708-428b-9b78-78135c451477
📒 Files selected for processing (1)
src/local/entrypoint.ts
| ' if (specifier.startsWith(SDK_SUBPATH_PREFIX)) {', | ||
| ' return nextResolve(sdkRootUrl + "/" + specifier.slice(SDK_SUBPATH_PREFIX.length), context);', | ||
| ' }', |
There was a problem hiding this comment.
🔴 Wildcard SDK/config subpath resolution produces incorrect file URLs that miss the dist/ directory and .js extension
The wildcard subpath resolution for @agent-relay/sdk/* and @agent-relay/config/* constructs sdkRootUrl + "/" + subpath (e.g. file:///<sdk>/github), but the actual SDK exports map every subpath into the dist/ directory with a .js extension (e.g. "./github" → "./dist/github.js"). Since file URLs in Node.js ESM bypass the package's exports map and do NOT get automatic .js extension resolution, nextResolve("file:///<sdk>/github", context) will fail with ERR_MODULE_NOT_FOUND.
Verified against the actual @agent-relay/sdk@6.0.13 package
The SDK package exports (downloaded and inspected from npm):
"./github"→"./dist/github.js""./relay"→"./dist/relay.js""./communicate"→"./dist/communicate/index.js""./communicate/a2a-types"→"./dist/communicate/a2a-types.js"
The loader constructs file:///<sdk>/github but no file exists at <sdk>/github — the file is at <sdk>/dist/github.js. The same issue applies to all @agent-relay/config subpaths (e.g. "./relay-config" → "./dist/relay-config.js").
The three exact-match cases (@agent-relay/sdk, @agent-relay/sdk/workflows, @agent-relay/config) work correctly because they hardcode paths to the actual .js files under dist/.
Prompt for agents
The wildcard subpath resolution in the generated ESM loader hook constructs file URLs by concatenating sdkRootUrl (the package root) with the subpath name, but the SDK's exports map all subpaths into the dist/ directory with .js extensions. For example, @agent-relay/sdk/github should resolve to <sdk>/dist/github.js, not <sdk>/github.
The fix needs to handle two patterns seen in the SDK exports:
1. Simple subpaths: ./github → ./dist/github.js (most exports)
2. Directory subpaths: ./workflows → ./dist/workflows/index.js, ./communicate → ./dist/communicate/index.js
Possible approaches:
- Read the SDK package.json exports map at build time and generate explicit resolve entries for each known subpath
- Construct the URL as sdkRootUrl + /dist/ + subpath and use nextResolve, relying on it to find .js files (but ESM wont add .js automatically)
- Try resolving sdkRootUrl + /dist/ + subpath + .js first, falling back to sdkRootUrl + /dist/ + subpath + /index.js
- Use import.meta.resolve or a createRequire-based resolver pointed at the SDK package root to properly consult the exports map
The same fix is needed for the CONFIG_SUBPATH_PREFIX case at line 965-967. The three exact-match cases (sdk, sdk/workflows, config) are correct and do not need changes.
Was this helpful? React with 👍 or 👎 to provide feedback.
Ricky Eval ReviewRun: Passed: 8 | Needs human: 43 | Reviewable: 43 | Missing output: 0 | Failed: 0 | Skipped: 0 Human Review CasesThese cases passed deterministic checks and include captured Ricky output for a human verdict against their REVIEW
|
…// concat
CodeRabbit + Devin both flagged the bug correctly: the previous
subpath branch did
return nextResolve(sdkRootUrl + "/" + "github", context);
which passes a fully-qualified file:// URL to nextResolve. Per node's
ESM resolution algorithm (Node docs `esm.html` + `packages.html`),
file:// URLs are treated as already-resolved and Node SKIPS package-
exports lookup. So `@agent-relay/sdk/github` would try to load a
literal file at `<sdkRoot>/github` instead of going through the SDK's
exports map (which maps `./github` → `./dist/github.js`). Verified
against actual @agent-relay/sdk@6.0.13.
The right shape: pass the original BARE specifier to nextResolve
with `parentURL` anchored inside the target package root. Node then
walks up looking for node_modules, finds the bundled package, and
applies its exports map.
Behavior after this commit (the three exact-match branches were
already correct and stay):
• `@agent-relay/sdk/workflows` → bundled (shortCircuit)
• `@agent-relay/sdk` → bundled root index
• `@agent-relay/sdk/<anything>` → re-resolved via parentURL anchor
so the SDK's exports map handles the subpath
• `@agent-relay/config` → bundled config index
• `@agent-relay/config/<anything>` → re-resolved via config
parentURL anchor
Adds a regression test that imports a non-`workflows` SDK subpath
(`@agent-relay/sdk/github`) AND two `@agent-relay/config` subpaths
(`/relay-config`, `/agent-config`) from a workflow file and asserts
the run succeeds without consumer-repo node_modules. Test fails
against the previous file-URL-concat implementation; passes against
this one.
114/114 src/local/entrypoint.test.ts pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Pushed return nextResolve(sdkRootUrl + "/" + "github", context); // file:// URLPer node's ESM resolution algorithm, file:// URLs are treated as already-resolved and the package's FixPass the original bare specifier with if (specifier.startsWith(SDK_SUBPATH_PREFIX)) {
return nextResolve(specifier, { ...context, parentURL: sdkParentUrl });
}Regression testAdded 114/114 in |
Pulls in agent-relay/relay#838 ("Drain broker stdout after SDK startup")
which is the upstream/root fix for the broker pipe-buffer deadlock that
wedged overnight runs of the proactive-runtime workflows.
Reproducible symptom prior to this bump: after M1 fans out to 9 PTY
workers, every worker's log goes silent within seconds of each other,
broker process parks in `write()` (or `_pthread_cond_wait` once enough
events queue), and the workflow node's `step.run` awaits a drain signal
that never arrives. Two diagnostic captures (~/wedge-92b45d3e-diag/ and
~/wedge-c9600674-diag/, ~14h apart on independent runs) showed the same
freeze-at-fanout shape.
Ricky already shipped the loader-level unblocker in #94. This bump
brings in the upstream SDK fix so direct SDK consumers — including
Ricky-driven workflows whose runtime SDK is the bundled copy under
Ricky's node_modules (per #92's loader redirect) — pick up the proper
drain.
Verification
- npm run typecheck — clean
- npm run build — bundles cleanly, externals unchanged
- npm test — 1075 / 1075 pass, including the existing
"drains broker stdout after SDK startup so event floods cannot wedge
the workflow node" regression in src/local/entrypoint.test.ts
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
@agent-relay/sdk/workflowsto the bundled SDK. Workflow files routinely import other SDK subpaths (@agent-relay/sdk/github,@agent-relay/sdk/relay, etc.) and the sibling@agent-relay/configpackage — those fell through to standard node resolution and failed in consumer repos that hadn'tnpm installed the SDK locally, defeating the point of having Ricky bundle it.@agent-relay/sdk/*subpath and every@agent-relay/config*import against the bundled location.Reproducer
Run any workflow that imports a non-
workflowsSDK subpath from a consumer repo without the SDK installed locally:Pre-fix error:
What changed (
src/local/entrypoint.ts)The generated loader now matches:
@agent-relay/sdk/workflows@agent-relay/sdk@agent-relay/sdk/<anything>@agent-relay/config@agent-relay/config/<anything>nextResolve(unchanged)Test plan
npm run typecheck— cleannpx vitest run src/local/entrypoint.test.ts— 113 / 113 pass🤖 Generated with Claude Code