Skip to content

initial implementation - #1

Merged
plicease merged 14 commits into
mainfrom
graham/impl
Oct 14, 2026
Merged

plicease merged 14 commits into
mainfrom
graham/impl

Conversation

@plicease

@plicease plicease commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

For the initial implemetnation.

plicease and others added 10 commits October 14, 2026 13:43
Persistent humbug sidecar client per the v0.1 requirements: executable
discovery, direct fork/exec with drained stderr and per-command timeouts,
NDJSON transport with id/type checks, byte-exact headers (always base64)
and bodies (absent vs empty), structured OpenAPI::Humbug::Error
exceptions, and reliable cleanup. Includes fake-sidecar unit tests and
opt-in integration tests (OPENAPI_HUMBUG_EXECUTABLE).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
plicease and others added 3 commits October 14, 2026 14:35
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Split the line exchange into an internal OpenAPI::Humbug::Transport base
class and a Transport::Command subclass that runs humbug as a child
process, so other connectors (unix or tcp sockets) can be added later.
The transport decides whether the session ends with shutdown.

Rename the error kind 'process' to 'transport'.

Protocol tests now run against an in-memory fake transport; the fake
sidecar process tests cover transport behavior only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread lib/OpenAPI/Humbug/Transport/Command.pm Outdated
# child: fds above $^F are close-on-exec, so only 0, 1 and 2 survive exec,
# and $status_w reports exec failure to the parent.
eval {
POSIX::dup2(fileno $child_in, 0) // die;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dup2 returns a file descriptor on success, including 0 for stdin, so using // is correct here. However, the exec-failure reporting pipe appears to rely on Perl's $^F close-on-exec behavior. I'd add a test specifically verifying that an executable which fails at exec reports a structured client.spawn_failed error and leaves no child behind.

A second area worth testing is close() after the subprocess has already died. The implementation is careful about reaping and escalation, but subprocess lifecycle edge cases are notoriously difficult to get right.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in ee06b15.

Exec failure / close-on-exec

  • Every pipe end is now marked FD_CLOEXEC explicitly rather than relying on $^F, and the child copies its three ends above fd 2 (F_DUPFD) before dup2-ing them onto 0–2, so one can't overwrite another if the caller has closed a standard handle. (For the record, the previous code turned out to be safe in practice because of the order the pipes are created in; this removes the dependency on that ordering.)
  • New tests in t/openapi_humbug_transport_command.t: an executable that passes -f/-x but fails at exec (#!/nonexistent/interpreter) gives transport / client.spawn_failed with no child left behind (waitpid(-1, WNOHANG) == -1), both directly and through OpenAPI::Humbug->new; construction returns while the child is still running (spawns are under a 5s alarm, so a leaked status pipe fails instead of hanging — verified by temporarily breaking it); and spawning works with STDIN, STDIN+STDOUT, and all three standard handles closed.

close() after the child died

  • Killed with SIGKILL: close doesn't throw, returns false, reaps, and doesn't wait out the grace period.
  • Exited 0 on its own: close doesn't throw and returns true.
  • Already reaped by someone else: close doesn't throw.
  • Destroyed after the child died: no error; validate after death is a transport error (client.eof/client.broken_pipe), never a result.

…icitly

Address review on PR #1:

- Mark every pipe end close-on-exec explicitly instead of relying on
  $^F, and copy the child's ends above fd 2 before dup2 so one cannot
  overwrite another when the caller has closed a standard handle.
- Test that an executable failing at exec reports client.spawn_failed
  and leaves no child, that construction does not block on the child,
  and that spawning works with closed standard handles.
- Test close(), destroy and validate after the child was killed,
  exited on its own, or was reaped elsewhere.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@plicease
plicease merged commit f15d568 into main Oct 14, 2026
4 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.

1 participant