Skip to content

fix(process): observe already-due trailing callbacks before settling the capture journal - #1700

Merged
morluto merged 1 commit into
morluto:mainfrom
JasonBuildAI:fix/process-capture-journal-settle-6f2c
Oct 11, 2026
Merged

morluto merged 1 commit into
morluto:mainfrom
JasonBuildAI:fix/process-capture-journal-settle-6f2c

Conversation

@JasonBuildAI

Copy link
Copy Markdown
Contributor

Summary

Fixes #1697. Process-capture Evidence can silently drop the terminal tail when the host stalls the event loop around exit: settleProcessCaptureJournal judged the journal quiet by wall-clock time alone, so a trailing observation callback that was already due ran after the settle returned and never reached the immutable capture. PTY capture results now retain the trailing observations they were always meant to keep.

Problem and expected behavior

When the event loop stalls longer than the quiet window (quietMs, 25 ms by default) and then resumes, the settle's own wake-up timer and a trailing observation callback are both overdue. Due timers run in expiry order, so the earlier wake-up evaluates the journal before the trailing callback has appended its observation, decides the journal is quiet, and finalizes the capture. The trailing observation is dropped without any truncation flag.

Smallest trigger (unit level, deterministic): a trailing 12 ms timer that appends to the journal, plus a 40 ms synchronous stall right after starting settleProcessCaptureJournal(journal, 20, 200).

Expected: the settle does not declare silence while callbacks that were already due are still queued; trailing observations delivered before the capture becomes immutable are retained.

Change and scope

  • settleProcessCaptureJournal now awaits setImmediate() after each quiet-window sleep, so timer and poll callbacks that were already due run before silence is judged.
  • Regression test: does not settle while a trailing observation is already due behind a stalled event loop in src/process/capture/ProcessHarness.test.ts.

Intentionally not included: no change to quiet/max-wait defaults, no rework of capture finalization, no new APIs.

Contract and boundary impact

  • Semantic owner and earliest changed stage: process capture lifecycle; the earliest affected stage is finalization after process exit.
  • CLI and MCP/tool-catalog contract: none; tool schemas and results are unchanged.
  • Provider, bridge, target-format, or platform compatibility: none; the logic is platform-independent.
  • Evidence, artifact, provenance, or reconstruction contract: capture Evidence keeps trailing observations that it could previously drop; no format change.
  • Process execution, authorization, cleanup, or containment impact: none.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.

Evidence and regression coverage

Evidence is executed on the change branch and on the base revision.

  • Tests added or updated: new deterministic regression in src/process/capture/ProcessHarness.test.ts. The existing waits for terminal observations delivered after the exit callback started failing 1 in 5 standalone runs (and reliably in a 65-file run) on Windows; it passes 6/6 runs after this change.
  • Base reproduction (base ee46323 with only the new test applied, fix reverted):
FAIL  |adapters| src/process/capture/ProcessHarness.test.ts > does not settle while a trailing observation is already due behind a stalled event loop
AssertionError: expected [ { capture_order: +0, …(2) } ] to have a length of 2 but got 1
 Test Files  1 failed (1)
      Tests  1 failed | 30 skipped (31)
  • After the change:
 Test Files  1 passed (1)
      Tests  1 passed | 30 skipped (31)
  • Project focused lane after the change:
npm run test:focused -- src/process/capture/ProcessHarness.test.ts src/process/capture/ProcessCaptureLifecycle.test.ts
 Test Files  2 passed (2)
      Tests  55 passed (55)
  • User-visible CLI/MCP output (if applicable): none; result shape is unchanged.
  • Remaining proof gaps: no whole-harness PTY stall repro (would need a controlled host stall during a real capture); the unit-level regression pins the settle contract directly.

For evidence-bearing changes:

  • Observed, derived, and inferred claims remain distinguishable.
  • Artifact identity, source provenance, and failed attempts remain preserved.
  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

  • npm run test:focused -- src/process/capture/ProcessHarness.test.ts src/process/capture/ProcessCaptureLifecycle.test.ts — 55 passed
  • npm run check:fast — typecheck and lint passed (also ran in the pre-push hook)
  • npx oxfmt --check src/process/capture/ProcessHarness.test.ts src/process/capture/ProcessCaptureLifecycle.ts — clean
  • Note: a broader Windows source-test run shows 12 pre-existing platform failures unrelated to this change (symlink creation EPERM, simulated win32 platform paths, missing locally built native artifact). This change removes the one failure it targets.

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable; the changed code is the platform-independent capture settle path.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: none; the change adds one event-loop turn per quiet-window iteration.

Review checklist

  • This PR addresses a concrete problem or an agreed enhancement.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed (none needed).
  • User-visible CLI/MCP changes include representative output (not applicable).
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

AI disclosure: this change was drafted with AI assistance at my request, under my direction and review.

…the capture journal

A stalled event loop can leave the settle wake-up and a trailing observation
callback due at the same resume. The earlier wake-up runs first, declares
silence, and the capture becomes immutable before the trailing callback
appends its observation.

Drain the loop turn after the quiet sleep so timer and poll callbacks that
were already due run before silence is judged, and cover the stalled-loop
case with a deterministic regression test.

Fixes morluto#1697
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@morluto

morluto commented Oct 11, 2026

Copy link
Copy Markdown
Owner

@codex

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-11T11:04:34.948567Z 1658564 Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: 1658564d90

ℹ️ 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".

@morluto
morluto merged commit ed084b3 into morluto:main Oct 11, 2026
31 checks 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.

[Bug] Process capture can settle its event journal while already-due trailing observations are still queued

2 participants