Repository navigation
fix(detached): self-report the runner module path; fail fast on load error - #15
Merged
Merged
Conversation
…error Third recurrence of the issue #3 class, found by dogfooding a detached council run: every detached phase failed with "No subagent runner injected" while all unit tests passed. Root cause: the host serialized `runnerModule` for the detached child by resolving the relative "./runner" specifier with a .ts suffix. tsc's `rewriteRelativeImportExtensions` rewrites STATIC import statements only (string arguments of import.meta.resolve() survive verbatim), so the compiled dist/index.js pointed the child at dist/runner.ts, a file that does not exist in the published build. Dev checkouts (src) worked, which is why the suite stayed green. Fixes: - runner.ts self-reports its own path via runnerModulePath() (import.meta.url), correct under both dev (.ts) and prod (.js) by construction; index.ts serializes that instead of resolving. - detached-runner now FAILS FAST when a promised runnerModule cannot be loaded: persists a __detach__ phase with the real cause and exits 1, instead of limping on and burying the import error under N copies of the generic no-runner stub. - New regression suite (detached-runner-module.test.ts) pins the class: (1) the self-reported path exists and dynamically imports to a working runner; (2) compiled dist/index.js must not contain import.meta.resolve of any relative .ts specifier; (3) compiled dist/runner.js self-reports its own dist path. Also adds docs/internal/brainstorm-2026-07-frontier-council.md, the July 2026 frontier council report (4-way live web research, matrix refresh, tournament ideation, final ruling) whose detached first run surfaced this bug.
heggria
added a commit
that referenced
this pull request
Jul 3, 2026
heggria
added a commit
that referenced
this pull request
Jul 3, 2026
…ve + close follow-ups Integrates the single-source skill architecture (#14) with the Top 3 features (#16): the score/reflexion/idempotent documentation, previously edited directly in the generated SKILL.md files, is now authored ONCE in skills-src/taskflow/core.md and compiled to both hosts via build-skills.mjs. The drift guard (skills-build.test.ts) passes. Also resolves all four deferred review follow-ups (issues #17-#20) so the release ships without known debt: - #17 (loop): PhaseState.loop.failures records every failed iteration (bounded to 20, sanitized). A reflexion loop that continues past failures no longer loses the intermediate diagnostics — only the terminal one previously survived in `error`. - #18 (scorer-runtime): code-compiles now runs inside an isolated mkdtempSync directory used as the compiler cwd. Closes the predictable temp-name TOCTOU race AND stops `npx tsc` resolving a repo-planted node_modules/.bin/tsc from the phase cwd (defense-in-depth beyond the dynamic-flow scorer block). The cwd parameter is dropped from the scorer signature. (Process-group kill declined: the codebase's script phase kills the direct child too; orphans are bounded by the 30s timeout and cross-platform process groups would diverge from that.) - #19 (docs): the shared core.md now documents {reflexion}, {loop.*}, and the score.target/judge.task interpolation sites; codex SKILL.md reaches feature parity automatically through the single-source build. - #20 (idempotent): a non-idempotent phase that re-executes on resume (a prior attempt had completed) now surfaces a warning so operators see the side effect re-firing; validation warns that idempotent:false on approval/flow phases is a no-op. Docs (README, both SKILL.md sets) reflect the post-review score semantics: auto-pass only when the judge cannot veto (no judge, or weighted lower-bound), and the dynamic-flow code-compiles/regex blocks. Suite: 1014/1014 (drift guard + 4 new regression tests for the follow-ups). dist rebuilt.
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.
Summary
Third recurrence of the issue #3 class (runner injection), found by dogfooding a detached council run: every detached phase failed with
No subagent runner injectedwhile all 954 unit tests passed.Root cause
The host serialized
runnerModuleviaimport.meta.resolveof a relative.tsspecifier. tsc'srewriteRelativeImportExtensionsrewrites static import statements only — string arguments ofimport.meta.resolve()survive verbatim — so the compileddist/index.jspointed the detached child atdist/runner.ts, a file that does not exist in the built package. Dev checkouts resolve fromsrcwhere the.tsfile exists, which is why the suite stayed green.Fixes
runner.tsexportsrunnerModulePath()(import.meta.url) — correct under both dev (.ts) and prod (.js) by construction;index.tsserializes that.__detach__phase carrying the real import error when a promisedrunnerModulecan't load — instead of failing every phase with the generic stub.detached-runner-module.test.ts): pins the class — self-reported path dynamically imports to a working runner; compileddist/index.jsmust not containimport.meta.resolveof any relative.tsspecifier; compileddist/runner.jsself-reports its own dist path.Also lands
docs/internal/brainstorm-2026-07-frontier-council.md— the July 2026 frontier council report whose detached first run surfaced this bug.Test plan
dist/runner.jsdynamic-import +runnerModulePath()verified against the built package