Skip to content

fix(agent): report the failures the channel paths dropped - #651

Merged
haribo merged 1 commit into
developfrom
fix/638-swallowed-failures
Sep 10, 2026
Merged

fix(agent): report the failures the channel paths dropped#651
haribo merged 1 commit into
developfrom
fix/638-swallowed-failures

Conversation

@haribo

@haribo haribo commented Sep 10, 2026

Copy link
Copy Markdown
Owner

Summary

Three dropped failures, each turning a diagnosable problem into a puzzling one later.

  • internal/agent/channel.go:94accept() overwrote c.conn without closing it. drop()
    is the only other closer and it runs when an ask discovers the connection is dead, so a
    control host reconnecting without the agent having asked anything in between leaked a
    descriptor. The agent lives up to 2h (ADR-0005): the ceiling is the number of runs in that
    window, not one.
  • internal/agent/agent.go:35ServeOn dropped the Listen error. It now builds
    Unavailable(err): the job still runs, and any ask fails naming the cause instead of sitting
    out attachWait and reporting no control host attached.
  • internal/transport/local.go:32os.MkdirTemp dropped. Now fails the run naming what
    could not be made.

The decision this issue left open

Fail immediately, or lazily? Lazily, in the agent — but for a corrected reason. My audit
said "a dry-run reaches no primitive"; the sharper fact is that controlChannel
(cmd/shellf/main.go:245) decides from the plan, statically, so a plan declaring a primitive
inside an apply gets a channel a --dry-run never uses. Failing at Listen would break that
run.

Not lazily in the local transport, and the asymmetry is deliberate: the agent is another
process reading its workdir from argv, so there is no Unavailable to hand it — continuing
there guarantees the cause is lost.

A fourth site, same defect

internal/agent/resident.go:40 has the identical shape, and it is the main path (ADR-0005),
not one of the three the issue names. Its comment already argued best-effort correctly; it just
dropped cherr too. Fixed with the same line. Called out rather than slipped in.

Test plan

Three new tests, each mutation-tested: the fix reverted, the test red, the fix restored.

ServeOn drops the Listen error again        → FAIL TestServeOn_ListenFailureReachesTheStep
local.go swallows MkdirTemp again           → FAIL TestLocal_WorkdirThatCannotBeCreatedIsReported
accept() overwrites without closing         → FAIL TestChannel_ReplacedBridgeIsClosed
restored                                    → ok
  • TestChannel_ReplacedBridgeIsClosed attaches two bridges with no failing ask between them —
    the path drop() never takes — and reads the first client end: EOF means closed, a read
    timeout means leaked.
  • TestServeOn_ListenFailureReachesTheStep fails the listen for a real reason (the workdir
    is a file), not by injecting an error, and asserts the step's stderr.
  • sockBase becomes a var so the local failure is reachable at all; the reason is on it.

resident.go has no test of its own — it is the same one-line change as ServeOn, and its
path is what the e2e harness exercises on every run.

go test ./..., test/lint.sh, test/dead-code.sh, test/coverage-ratchet.sh (82.0%) green.

Correction to the issue: it points at internal/agent/bridge_reconnect_test.go as already
driving reconnection. That file is internal/transport/bridge_reconnect_test.go — the control
host's side, not the agent's, so it could not have caught this.

Closes #638

@haribo
haribo force-pushed the fix/638-swallowed-failures branch from 1b4b412 to 2198b18 Compare September 10, 2026 09:43
@haribo
haribo force-pushed the fix/638-swallowed-failures branch from 2198b18 to 601ded0 Compare September 10, 2026 09:50
@haribo
haribo merged commit 36e3b7c into develop Sep 10, 2026
8 checks passed
@haribo
haribo deleted the fix/638-swallowed-failures branch September 10, 2026 15:19
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