refactor: break import cycles; module-boundary law + design doc (isolation 1/7)#116
Merged
Conversation
First PR of the module-isolation series (see docs/design/module-isolation-2026-06.md, distilled from a full import-graph audit): - src/version.ts extracted — orchestrator.ts and cli.ts imported VERSION from index.ts while index.ts imported run() back from the orchestrator. The cycle only worked because a hoisted const survives ESM live-binding order. - RunOptions/RunSummary declarations moved to orchestrator/options.ts so prepare.ts no longer imports upward from its own module entry. - tests/module-boundaries.test.ts — the dependency matrix is now law, enforced by the standard test gate. Both rules active: matrix membership (zero exceptions needed — the layering already held everywhere except the two cycles) and contracted-module index-only imports (ratchet; grows as contract PRs land). - oxlint import/no-cycle enabled as a secondary check (verified it fires on a deliberate cycle before trusting it). Zero behavior change; public API byte-identical.
This was referenced Jun 10, 2026
Exelord
added a commit
that referenced
this pull request
Jun 10, 2026
A SIGTERM to the vx process alone (CI cancellation, kill <pid>) left running children orphaned — including persistent dev servers — with no orderly shutdown. Terminal Ctrl-C only worked via process-group propagation. run() now keeps a run-scoped liveChildren registry maintained by the spawn sites (runCommand, runPersistent, runSandboxed); SIGINT/SIGTERM handlers installed for exactly the run's duration forward SIGTERM to every live child, close the cache, and exit 128+signo. Handlers are removed in a finally so repeated runs never stack listeners. New RunOptions.handleSignals (default true): the watch loop passes false — it owns signal disposition for its lifetime, and an in-flight cycle must never exit the process out from under it (the naive version killed the test suite via watch's in-process SIGINT simulation). Known limit (documented): only direct children are signalled — a double-forking task can still orphan grandchildren. Squash-merge of claude/run-signal-handling rebased onto the #115/#116 state: the branch's own signalExitCode duplicate was dropped in favor of #115's os.constants-based helper, and handleSignals lives in orchestrator/options.ts (its post-#116 home).
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
1 participant
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.
First PR of the module-isolation series — full design + audit in
docs/design/module-isolation-2026-06.md(distilled from an exhaustive import-graph audit; adopts PR #108's modules-first principles in place, no rebuild).Changes
src/version.tsextracted —orchestrator.ts/cli.tsimportedVERSIONfromindex.tswhileindex.tsimportedrun()back from the orchestrator. The cycle survived only via ESM hoisted-const live bindings.orchestrator/options.ts—RunOptions/RunSummarydeclarations move out of the module entry soprepare.tsno longer imports upward (type-only cycle).tests/module-boundaries.test.ts— the dependency matrix is now law under the normal test gate. Notably it needed zero seeded exceptions: the layering already held everywhere except the two cycles fixed here. Second rule (contracted modules importable only via theirindex.ts) is a ratchet that grows as the contract PRs land.import/no-cycleenabled — verified it actually fires on a deliberate cycle first.Zero behavior change; public API byte-identical. Remaining series: relocations (nested-dirs/fingerprint→workspace, plan-format→cli, ProjectEntry→workspace), task-hash split, per-module contracts, entry moves, docs refresh.
Test plan
🤖 Generated with Claude Code