Skip to content

Phase D completion: hexagonal sealing, error boundaries, simplification#195

Merged
kangig94 merged 20 commits intomainfrom
dev
Apr 15, 2026
Merged

Phase D completion: hexagonal sealing, error boundaries, simplification#195
kangig94 merged 20 commits intomainfrom
dev

Conversation

@kangig94
Copy link
Copy Markdown
Owner

@kangig94 kangig94 commented Apr 15, 2026

Summary

  • Error boundary fixes (P0/P1): EventBus emit() crash guard, session claim leak fix, readStatusFromDisk validation, VirtualTime delay-0 + callback error handling
  • Discuss runtime hexagonal migration (F1): Complete sealing of 10+ discuss modules behind Runtime 6-port interface — durable storage ops, storage-aware readers, runtime path resolution, DiscussContext wiring, timestamp determinism, UTC transcript rendering
  • Architecture cleanup: Extract Runtime port interfaces to L0, remove L0→L1 layer violations, injectable fetch, RuntimeEnv.fullSnapshot()
  • Deep simplification: Deduplicate readers/simulation/state-machine, extract DecisionContext, remove SimulationBackend aliases, collapse ProgressStore constructor, remove process-global promise-chain locks

Test plan

  • npm run build passes (tsc + esbuild + simulation sealing)
  • npm test passes (1575 tests, 0 failures)
  • New discuss-runtime-sealing.test.ts verifies in-memory-only discuss paths
  • Import audit: no node:fs/node:crypto/process.env/native timers in sealed discuss modules
  • Simulation tests exercise discuss through SimulationRuntime without filesystem fallthrough

🤖 Generated with Claude Code

kangig94 and others added 20 commits April 15, 2026 21:54
- EventBus emit(): wrap per-listener try-catch to prevent caller crash
- job-lifecycle: release session claim when finalizeProviderSession throws
- progress-store: validate PersistedStatusRecord shape on disk read
- VirtualTime: clamp setTimeout delay to min 1ms (prevent delay-0 infinite loop)
- VirtualTime: continue processing due timers when a callback throws

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tch 1)

- Add writeAtomicDurableSync/appendFileDurableSync to RuntimeStorage with
  fdatasyncSync+parent-dir-fsync semantics in real runtime
- InMemoryStorage delegates durable ops to existing in-memory write/append
- Add backwards-compatible acquireDirectoryLockSync overload accepting
  DirectoryLockDeps for explicit storage/time injection
- Wire discuss session-store lock helpers with explicit deps
- Tests cover durable success, ENOENT race, parent-dir fsync, sync lock deps

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tion (Batch 2)

- Add DiscussPathResolver to RuntimePaths with real + InMemoryPaths impls
- Add storage-aware reader variants in client/readers.ts accepting RuntimeStorage
- Migrate session-store to use injected storage/time/paths for all reads
- persistence.ts delegates to store.readSessionEvents instead of direct fs
- read-helpers.ts receives source discovery as dependency
- executor.ts reads job status through ctx.jobStatusReader
- Wire runtime ports through backend-core construction

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…gration (Batch 3)

- Remove node:fs import from session-store.ts entirely
- Replace writeAtomicJson with storage.writeAtomicDurableSync
- Replace appendEventBatch with storage.appendFileDurableSync
- Replace native setTimeout/clearTimeout with runtime time port
- Replace flushTimer type with RuntimeTimerHandle
- All lock calls pass explicit storage/time deps

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Extend DiscussContext with runtime.ids, runtime.env, runtime.time ports
- Add DiscussJobStatusReader to context for executor recovery
- Update context-registry.getOrCreate to require runtime ports
- Wire backend-core to pass runtime ports through context construction
- executeDiscussStart uses ctx.runtime.ids.uuid() instead of randomUUID

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…/projections/transcript (Batch 5)

- tools.ts: remove node:crypto, use ctx.runtime.ids.uuid()
- operations.ts: read CORAL_DISCUSS_MAX_EPOCHS via ctx.runtime.env.get()
- operations/loop/subflows/executor: all timestamps via nowIsoString(ctx.runtime.time)
- loop.ts: observer waits and trampoline via ctx.runtime.time
- projections.ts: invalid ts falls back to epoch 0, not Date.now()
- transcript.ts: UTC getters for deterministic rendering

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ss (Batch 6)

- Add discuss-runtime-sealing.test.ts with in-memory-only discuss tests
- Tests cover: in-memory session append/load, invalid ts watch replay (epoch 0),
  legacy projectRoots source discovery, virtual-time deterministic timestamps,
  UTC transcript rendering, import audit assertions
- Update SimulationBackend harness with discuss recovery option

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…e-ports.ts

Move all Runtime port interfaces (RuntimeTime, RuntimeStorage, RuntimePaths,
RuntimeProcess, RuntimeIds, RuntimeEnv, etc.) from L1 execution/runtime.ts
to L0 shared/runtime-ports.ts. Update 6 L0 modules (shared/utils, file-tail,
session-entry, fs-lock, infra/plugin-registry, backend-info) to import from
the new location. L1 runtime.ts re-exports for existing consumers.

Fixes layer direction violation: L0 no longer imports from L1.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- F2: lifecycle.ts uses runtime.paths.installationDirForNamespace instead
  of homedir(), runtime.ids.randomBytes instead of Math.random();
  session-manager.ts routes sha256 through runtime.ids;
  kb-tools.ts uses runtime.storage instead of node:fs
- F4: add fullSnapshot() to RuntimeEnv interface + real runtime impl
- F5: make fetch() injectable in backend-core health check via fetchFn option
- F6: delete vestigial abort-registry.ts barrel (no importers)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ve TOCTOU

- readers.ts: direct-fs functions delegate to WithStorage variants (~120 lines removed)
- subflows.ts: extract ctxTs() helper replacing 12 nowIsoString(ctx.runtime.time) calls
- runtime.ts: remove existsSync pre-check in readJsonIfPresent (TOCTOU)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Consolidate isPersistedStatusRecordLike (lifecycle reuses progress-store's)
- Consolidate isWithinLiveSessionBoundary (context-registry reuses operations')
- Extract toolValidationError to tool-response.ts (shared by discuss + KB tools)
- Extract failedBidOutcome factory in subflows.ts (4 identical literals → 1)
- Add DiscussPurpose union type in executor.ts (stringly-typed → typed)
- Remove redundant comment in session-store.ts
- Simplify event-bus reset() to delegate to removeAllListeners()

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…isionContext, remove aliases

- Extract shared cloneSpawnEvent to runtime-ports.ts (was duplicated in 2 files)
- Delete simulation/core/deferred.ts (identical to shared/test-deferred.ts)
- Extract simulation constants + toError to simulation/core/constants.ts
- Replace runner.ts asErrorMessage with shared errorMessage
- Add DecisionContext type, refactor state-machine functions to use it
- Remove SimulationBackend convenience aliases (time/storage/paths/ids/env)
- Collapse ProgressStore to single constructor signature

29 files changed, -59 lines net reduction

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…lan loops

- Extract 90-line interval callback into finalizeDeadAdoptedJob() for
  clear responsibility separation (provider recovery / persisted payload / wrapper lost)
- Unify duplicate plan.register + plan.cleanup for-try-catch loops into single iteration

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Delete sessionAppendLocks, projectDiscoveryLocks, discussSourcesRegistryLocks
and their helper functions (withPromiseChainLock, tryWithPromiseChainLockSync,
sessionLockKey). Filesystem directory locks are the sole coordination primitive.

The promise-chain locks were an in-process optimization layered on top of the
filesystem locks. Since append() reads snapshots from disk inside the fs lock
and expectedSeq validates ordering, the fs lock alone guarantees correctness.
This also restores simulation isolation — lock coordination now flows entirely
through injected storage/time deps.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- VirtualTime: replace O(n) linear scan in nextDueTimer() with inline
  binary min-heap sorted by (deadline, order). O(log n) per fire.
- InMemoryStorage: replace O(total) full-map scan in readdirSync() with
  childIndex Map<string, Set<string>>. O(children) per call.
- Add regression tests for recursive mkdir, rename, rm child index

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
These Zod schemas, type aliases, and re-export wrappers were remnants
of the former MCP tool surface (codex/claude/coral-agent). MCP was
fully replaced by CLI-only invocation but the validation layer remained
with zero runtime imports.

Relocate the mapTurnStartParams effort-mapping tests to a dedicated
request-mapping.test.ts before deleting the host file.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
P0 — Wrong tests:
- inject.test.ts: fix missing-file case to throw ENOENT (was testing empty-file)
- Delete hud-canonicalization.test.ts (tested copied stub, not actual HUD/hook)

P1 — Dead code:
- schemas.test.ts: remove stale skipped test (already covered in agent-resolution.test.ts)
- Remove unused test variables (_waitForCondition, origGet, cachedRuntime)
- Remove unused imports (beforeEach, EntityGraph, redundant assignment)

P2 — Test-only prod exports:
- cli-detection.ts: mark detectCodexCli/resetCache as @internal

P3 — Legacy harness migration:
- discuss-test-helpers.ts: migrate from createRealRuntime to SimulationRuntime
- progress-store.test.ts: migrate to InMemoryStorage-based runtime

P4 — Duplicate/weak tests:
- http-client.test.ts: remove type-only test with no runtime assertions
- claude/progress.test.ts: remove duplicate formatting assertions
- format-progress.test.ts: replace smoke-only checks with concrete output assertions

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@kangig94 kangig94 merged commit 05a37b0 into main Apr 15, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant