jared/cc-os

Watch

Fork 0

Hook dispatch in-process (supersede ADR-0172 item 4) #632

New issue

Closed

opened 2026-09-23 12:03:58 +00:00 by jared · 3 comments

jared commented

2026-09-23 12:03:58 +00:00

Owner

Problem

plugins/os-sdlc/lib/os_sdlc/hook/pipe.rb:41 runs Open3.capture3("ruby", runner_bin, @subcommand, chdir: @event.cwd, stdin_data: @raw). This boots a second Ruby process for every SubagentStart, SubagentStop, and PostToolUse hook event. The hooks in plugins/os-sdlc/hooks/*.rb already require_relative "../lib/os_sdlc" — the same boot file bin/os-sdlc-runner uses — so the second boot is redundant work paid on every hook firing, not just in tests.

Observed

plugins/os-sdlc/lib/os_sdlc/hook/pipe.rb:41 forks ruby <runner_bin> <subcommand> per event.
The hook process has already loaded os_sdlc before Pipe runs.
Pipeline::CLI.start rescues StandardError internally and calls exit 3; Thor itself may also raise SystemExit. Both must be mapped to the same stderr-plus-nonzero-status contract the subprocess path gives today.
Stdout payload contract is unchanged: non-empty runner stdout becomes hookSpecificOutput.additionalContext.

Reproduce

n/a: design change. Latency is measured as part of this ticket's own acceptance criteria, not reproduced as a defect.

Approach

Pipe builds an OsSdlc::CLI::Invocation.new(cwd: @event.cwd, stdin: StringIO.new(@raw), stdout: StringIO.new, stderr: StringIO.new, env: ENV.to_h) and calls OsSdlc::Pipeline::CLI.start([@subcommand], invocation: ...) in-process instead of forking. No Dir.chdir: every command reads @invocation.cwd. surface_failure must rescue both StandardError and SystemExit from the in-process call and map each to the same stderr text and nonzero exit status the subprocess path produces today.

Required work:

Replace the Open3.capture3 fork with the in-process Invocation/Pipeline::CLI.start call — check: grep -n "Open3.capture3" plugins/os-sdlc/lib/os_sdlc/hook/pipe.rb returns nothing.
Map both SystemExit and StandardError from the in-process call to the existing stderr-plus-status contract — check: plugins/os-sdlc/tests/hook/pipe_missing_cwd_test.rb stays green.
Measure hook latency before and after on a sample SubagentStop payload with time; post both numbers as a ticket comment.
Record the reversal with /os-adr:create, superseding ADR-0172 item 4. Cite ADR-0128 as still satisfied (Pipe adds no decision logic, only relays). Note ADR-0156 is itself superseded by ADR-0182; cite ADR-0182 as the precedent for the pre_tool_use in-process pattern instead.
bin/refresh-plugins so the change reaches active sessions.

Rejected: a long-lived daemon. That is a different shape (persistent process, IPC, lifecycle management) and out of scope for this ticket.

Condition that would make this plan wrong: class-level memoized state in Pipeline::CLI or its dispatch commands that assumes one command per process. If such state exists, the in-process call would leak state across hook events and the approach needs revisiting.

VERDICT: CREATE

Acceptance criteria

Given a SubagentStart, SubagentStop, or PostToolUse hook event, when Pipe dispatches it, then no ruby <runner_bin> subprocess is forked — the runner subcommand runs in-process.
Given the in-process call raises StandardError or SystemExit, when Pipe rescues it, then the hook process does not crash, and the emitted stderr and exit status match what the subprocess path produced for the same failure today.
Given the change is complete, when hook latency is measured before and after on a sample SubagentStop payload, then both numbers are recorded on this ticket.
Given the dispatch mechanism has changed, when the ticket is closed, then an ADR superseding ADR-0172 item 4 exists, citing ADR-0128 (still satisfied) and ADR-0182 (precedent), and bin/refresh-plugins has been run.

Owner sketch

module OsSdlc
  module Hook
    class Pipe
      def relay
        InProcessRun.new(subcommand: @subcommand, cwd: @event.cwd, raw: @raw).call
      end
    end

    class InProcessRun
      def call
        invocation = OsSdlc::CLI::Invocation.new(
          cwd: @cwd, stdin: StringIO.new(@raw),
          stdout: StringIO.new, stderr: StringIO.new, env: ENV.to_h
        )
        OsSdlc::Pipeline::CLI.start([@subcommand], invocation: invocation)
        # ... map invocation.stdout/stderr/status to the hook payload contract
      rescue StandardError, SystemExit => e
        surface_failure(e)
      end
    end
  end
end

Pinning test: plugins/os-sdlc/tests/hook/pipe_missing_cwd_test.rb stays green.

Non-goals

The pre_tool_use hook path (already in-process via ADR-0182).
Changing runner subcommand behavior itself.

Origin

Trigger: shell-call/ENV audit, session 16bf37b3-8cff-4121-896c-4a5fc61f2e9e (2026-09-23), user-requested.
Improvised this session: none.
Chain: hook latency <- Open3.capture3 forks a second Ruby boot per hook event <- DESIGN, ADR-0172 item 4 chose subprocess pipe couriers for hooks.
Root candidate: this ticket.
Where: plugins/os-sdlc/lib/os_sdlc/hook/pipe.rb:41
Session: c15fe063-c64c-417b-b4e1-53a52ffca319 (#624 suite-speed background); 16bf37b3-8cff-4121-896c-4a5fc61f2e9e (audit that produced this ticket)
Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os/c15fe063-c64c-417b-b4e1-53a52ffca319.jsonl ; /home/jared/.claude/projects/-home-jared-dev-cc-os/16bf37b3-8cff-4121-896c-4a5fc61f2e9e.jsonl
Rewritten 2026-09-24 in session 082c3e8b after blast-radius review.

## Problem `plugins/os-sdlc/lib/os_sdlc/hook/pipe.rb:41` runs `Open3.capture3("ruby", runner_bin, @subcommand, chdir: @event.cwd, stdin_data: @raw)`. This boots a second Ruby process for every SubagentStart, SubagentStop, and PostToolUse hook event. The hooks in `plugins/os-sdlc/hooks/*.rb` already `require_relative "../lib/os_sdlc"` — the same boot file `bin/os-sdlc-runner` uses — so the second boot is redundant work paid on every hook firing, not just in tests. ## Observed - `plugins/os-sdlc/lib/os_sdlc/hook/pipe.rb:41` forks `ruby <runner_bin> <subcommand>` per event. - The hook process has already loaded `os_sdlc` before Pipe runs. - `Pipeline::CLI.start` rescues `StandardError` internally and calls `exit 3`; Thor itself may also raise `SystemExit`. Both must be mapped to the same stderr-plus-nonzero-status contract the subprocess path gives today. - Stdout payload contract is unchanged: non-empty runner stdout becomes `hookSpecificOutput.additionalContext`. ## Reproduce n/a: design change. Latency is measured as part of this ticket's own acceptance criteria, not reproduced as a defect. ## Approach Pipe builds an `OsSdlc::CLI::Invocation.new(cwd: @event.cwd, stdin: StringIO.new(@raw), stdout: StringIO.new, stderr: StringIO.new, env: ENV.to_h)` and calls `OsSdlc::Pipeline::CLI.start([@subcommand], invocation: ...)` in-process instead of forking. No `Dir.chdir`: every command reads `@invocation.cwd`. `surface_failure` must rescue both `StandardError` and `SystemExit` from the in-process call and map each to the same stderr text and nonzero exit status the subprocess path produces today. Required work: 1. Replace the `Open3.capture3` fork with the in-process `Invocation`/`Pipeline::CLI.start` call — check: `grep -n "Open3.capture3" plugins/os-sdlc/lib/os_sdlc/hook/pipe.rb` returns nothing. 2. Map both `SystemExit` and `StandardError` from the in-process call to the existing stderr-plus-status contract — check: `plugins/os-sdlc/tests/hook/pipe_missing_cwd_test.rb` stays green. 3. Measure hook latency before and after on a sample SubagentStop payload with `time`; post both numbers as a ticket comment. 4. Record the reversal with `/os-adr:create`, superseding ADR-0172 item 4. Cite ADR-0128 as still satisfied (Pipe adds no decision logic, only relays). Note ADR-0156 is itself superseded by ADR-0182; cite ADR-0182 as the precedent for the pre_tool_use in-process pattern instead. 5. `bin/refresh-plugins` so the change reaches active sessions. Rejected: a long-lived daemon. That is a different shape (persistent process, IPC, lifecycle management) and out of scope for this ticket. Condition that would make this plan wrong: class-level memoized state in `Pipeline::CLI` or its dispatch commands that assumes one command per process. If such state exists, the in-process call would leak state across hook events and the approach needs revisiting. VERDICT: CREATE ## Acceptance criteria 1. Given a SubagentStart, SubagentStop, or PostToolUse hook event, when Pipe dispatches it, then no `ruby <runner_bin>` subprocess is forked — the runner subcommand runs in-process. 2. Given the in-process call raises `StandardError` or `SystemExit`, when Pipe rescues it, then the hook process does not crash, and the emitted stderr and exit status match what the subprocess path produced for the same failure today. 3. Given the change is complete, when hook latency is measured before and after on a sample SubagentStop payload, then both numbers are recorded on this ticket. 4. Given the dispatch mechanism has changed, when the ticket is closed, then an ADR superseding ADR-0172 item 4 exists, citing ADR-0128 (still satisfied) and ADR-0182 (precedent), and `bin/refresh-plugins` has been run. ## Owner sketch ```ruby module OsSdlc module Hook class Pipe def relay InProcessRun.new(subcommand: @subcommand, cwd: @event.cwd, raw: @raw).call end end class InProcessRun def call invocation = OsSdlc::CLI::Invocation.new( cwd: @cwd, stdin: StringIO.new(@raw), stdout: StringIO.new, stderr: StringIO.new, env: ENV.to_h ) OsSdlc::Pipeline::CLI.start([@subcommand], invocation: invocation) # ... map invocation.stdout/stderr/status to the hook payload contract rescue StandardError, SystemExit => e surface_failure(e) end end end end ``` Pinning test: `plugins/os-sdlc/tests/hook/pipe_missing_cwd_test.rb` stays green. ## Non-goals - The `pre_tool_use` hook path (already in-process via ADR-0182). - Changing runner subcommand behavior itself. ## Origin - Trigger: shell-call/ENV audit, session 16bf37b3-8cff-4121-896c-4a5fc61f2e9e (2026-09-23), user-requested. - Improvised this session: none. - Chain: hook latency <- Open3.capture3 forks a second Ruby boot per hook event <- DESIGN, ADR-0172 item 4 chose subprocess pipe couriers for hooks. - Root candidate: this ticket. - Where: plugins/os-sdlc/lib/os_sdlc/hook/pipe.rb:41 - Session: c15fe063-c64c-417b-b4e1-53a52ffca319 (#624 suite-speed background); 16bf37b3-8cff-4121-896c-4a5fc61f2e9e (audit that produced this ticket) - Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os/c15fe063-c64c-417b-b4e1-53a52ffca319.jsonl ; /home/jared/.claude/projects/-home-jared-dev-cc-os/16bf37b3-8cff-4121-896c-4a5fc61f2e9e.jsonl - Rewritten 2026-09-24 in session 082c3e8b after blast-radius review.

jared added the

plugin/os-sdlc

filed-by/user

labels

2026-09-23 12:03:58 +00:00

jared referenced this issue

2026-09-23 12:50:54 +00:00

Add Sdlc/ProcessBoundary cop restricting process-starting calls to named boundary classes #637

jared referenced this issue

2026-09-23 13:04:18 +00:00

os-sdlc-runner: Pipeline::CLI receives an Invocation; bin/os-sdlc-runner is the only reader of process globals #638

jared referenced this issue

2026-09-23 13:05:40 +00:00

Repo-wide guard against forks of this repo's own bin/ executables in tests; hook-test harness runner calls in-process; hang-guard 30 s -> 5 s #642

jared referenced this issue

2026-09-23 20:34:48 +00:00

Repo-wide guard against forks of this repo's own bin/ executables in tests; hook-test harness runner calls in-process; hang-guard 30 s -> 5 s #642

jared added the

ready-for-agent

label

2026-09-24 20:08:14 +00:00

jared commented

2026-09-24 21:11:37 +00:00

Author

Owner

Work started in session 082c3e8b on branch ticket-632 via /os-sdlc:implement.

jared referenced this issue

2026-09-24 21:36:50 +00:00

poodr-ticket-implementation map: a red diff-test caused by a test-file defect after the green phase has no route to test-writer; three programmer-repair rounds are spent for nothing #709

jared commented

2026-09-24 21:38:37 +00:00

Author

Owner

Latency, SubagentStop hook with a probe payload, 6 timed runs per side after one warm-up, ruby 4.0.2: before (Open3 fork of bin/os-sdlc-runner) median 0.315 s, min 0.31 s; after (in-process via Invocation) median 0.17 s, min 0.17 s; delta -0.145 s, about -46%. Both sides took the same code path (identical harness error, exit 3).

jared commented

2026-09-24 21:38:48 +00:00

Author

Owner

Resolution

Done: OsSdlc::Hook::Pipe#relay calls OsSdlc::Pipeline::CLI.start in-process through an Invocation (cwd from the event, stdin from the raw payload, StringIO stdout/stderr). SystemExit passes its status through; StandardError is written to stderr and exits 3; stdout still becomes hookSpecificOutput. The runner is injectable via runner: for tests. ADR-0217 amends ADR-0172 item 4.

Evidence: Pipeline implementation 35 (escalated at programmer-repair bound: the test file used process-global stubs under parallelize_me!; the test seam was hand-fixed to inject the runner, no production change of substance; #709 captures the routing gap). Latency posted on the ticket: median 0.315 s to 0.17 s (-46%). Tests: tests/hook/pipe_in_process_test.rb (3 cases, green across seeds 1-5), pipe_missing_cwd_test.rb; os-sdlc suite 1449 runs 0 failures; rubocop clean. Commit 26c4eccd on branch ticket-632.

Follow-ups: #709 captured (post-green test defect has no route to test-writer, VERDICT: CREATE). Dropped: env: ENV.to_h in Hook::Pipe is the hook entry point's own environment, the same read bin/os-sdlc-runner performs; not a new ENV fallback under ADR-0213.

## Resolution **Done:** OsSdlc::Hook::Pipe#relay calls OsSdlc::Pipeline::CLI.start in-process through an Invocation (cwd from the event, stdin from the raw payload, StringIO stdout/stderr). SystemExit passes its status through; StandardError is written to stderr and exits 3; stdout still becomes hookSpecificOutput. The runner is injectable via runner: for tests. ADR-0217 amends ADR-0172 item 4. **Evidence:** Pipeline implementation 35 (escalated at programmer-repair bound: the test file used process-global stubs under parallelize_me!; the test seam was hand-fixed to inject the runner, no production change of substance; #709 captures the routing gap). Latency posted on the ticket: median 0.315 s to 0.17 s (-46%). Tests: tests/hook/pipe_in_process_test.rb (3 cases, green across seeds 1-5), pipe_missing_cwd_test.rb; os-sdlc suite 1449 runs 0 failures; rubocop clean. Commit 26c4eccd on branch ticket-632. **Follow-ups:** #709 captured (post-green test defect has no route to test-writer, VERDICT: CREATE). Dropped: env: ENV.to_h in Hook::Pipe is the hook entry point's own environment, the same read bin/os-sdlc-runner performs; not a new ENV fallback under ADR-0213.

jared closed this issue

2026-09-24 21:38:48 +00:00

jared referenced this issue from a commit

2026-09-26 13:57:00 +00:00

feat(os-sdlc): hook pipe runs the runner subcommand in-process through the Invocation seam; SystemExit and StandardError map to the hook exit contract (#632, ADR-0217)

jared referenced this issue

2026-10-18 18:58:40 +00:00

Pipeline cannot converge when a red suite-check needs a test-side edit #544