Skip to content

External room participant: stop escalation, stderr visibility, timeout wording (#198 follow-ups) #1164

Closed

Description

@Birfy

Owner: Dev

From the merge review of #1153 (#198, merged 78c7fb5f5) — three non-blocking findings, all in core/src/host/room-participant-external.js. Filed together because they are one surface (the external driver) and each is a few lines.

  1. A stop that is ignored leaves the child alive, and the next turn talks to it. endChild (:155) writes { type: "stop" } and sends one SIGTERM 50 ms later (:163), with no SIGKILL escalation. A child that ignores both keeps running; start() reuses a live child, so the next turn in that room is handed to a process whose seat was already closed. Expected: after the grace, escalate to SIGKILL, or refuse to reuse an entry whose stopping is set.
  2. The timeout room line does not read as a sentence. :78 renders "… 's external process timeout (no answer inside the room turn timeout)". Expected timed out for the state timeout, with the detail unchanged.
  3. stderr is collected and never surfaced. :134-135 keeps the child's stderr (capped at MAX_OUT) but nothing reads entry.stderr, so a child that fails with a helpful message on stderr shows only exit N. Expected: include the tail with the crash line, or drop the buffer.

Verified against main 78c7fb5f5; the room-turn shape, the restart and the interpreter refusal are not in scope here.

Activity

  1. added
    bugSomething isn't working
    agent-readyMaintainer-approved spec: an agent may take it. Only maintainers set this; never in a template
    area:automationScheduled tasks and automation
    on Oct 25, 2026
  2. Birfy commented on Oct 25, 2026

    @Birfy
    CollaboratorAuthor

    Written-by: Triage

    bug|agent-ready|area:automation, Owner: Dev — he shipped #1153 and this is the surface it added, so the invariants are his. Not a duplicate: #198 is closed, and the three items are one driver (core/src/host/room-participant-external.js), which is why they are one ticket rather than three.

    Line numbers check out on main 78c7fb5f: endChild at :155, the single SIGTERM at :163, the timeout sentence at :78, stderr buffering at :134-135. Triage picks the direction for each so nothing is left open to interpretation:

    1. Both halves, and they are not alternatives. After the grace, escalate to SIGKILL (a SIGTERM-only stop is a courtesy, not a stop), and start() must refuse to reuse an entry whose stopping is set. The escalation handles the normal case; the reuse guard is the fail-closed invariant if a kill path ever fails or the entry outlives it. A test that closes a seat on a child which ignores SIGTERM and then asks that room for a turn must not reach the old process.
    2. Wording: timed out for state === 'timeout', detail unchanged. Keep the other states' sentences as they are — this is a render fix, not a rewrite of the line.
    3. Surface the tail, do not drop it. The crash line carries the last ~200 characters of stderr, flattened to one line and capped (say the cap in the code), so a child that dies with a message is diagnosable from the room. Dropping the buffer entirely is the other legal answer, but then whoever debugs exit 1 has nothing — pick surfacing.

    Each item gets its own test and its own knife: reverting one declaration reds exactly one case. room-external suite green, 0 skipped; no change to the room-turn shape, the restart or the interpreter refusal — those are #198's invariants and they stay.

    Reviewer: must not be Dev.

  3. added
    agent-workingAn agent has claimed this issue (the claim comment names the branch)
    and removed
    agent-readyMaintainer-approved spec: an agent may take it. Only maintainers set this; never in a template
    on Oct 25, 2026
  4. removed
    agent-workingAn agent has claimed this issue (the claim comment names the branch)
    on Oct 25, 2026
  5. Birfy commented on Oct 25, 2026

    @Birfy
    CollaboratorAuthor

    done — #1170 merged at 2521aab1c (PASS at 5283b2f7 → queue → merge_group → merge, 12:55:09Z). Four closing items, read back on main 2521aab1c:

    1. Issue: closed/completed by Fixes #1164 in the PR body — the merge closed it.
    2. Changelog: changelog.d/1164-external-driver-hardening.fixed.md on main — SIGKILL escalation, the no-reuse guard, the stderr tail and the timed out wording.
    3. Docs: docs/subsystems/core-host.md on main carries the escalation and the stderr tail, and #1165's target/decision sentences are still there (the final tip rebuilt the file from base 7fe7149a with only the external-participant sentence changed).
    4. Fixes: matched — Fixes #1164 closes this issue and only this one.

    agent-working removed from this issue and agent-pr from the PR; labels here are now bug, area:automation.

    Three INC rounds closed on this branch: the escalation/reuse guard, the close-may-never-arrive drain fallback, and a docs rebase that would have deleted #1165's documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:automationScheduled tasks and automationbugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions