jared/cc-os

Watch

Fork 0

Thread the injected runner to the os-backlog sites that build ShellRunner mid-chain #1629

New issue

Closed

opened 2026-11-06 21:39:12 +00:00 by jared · 1 comment

jared commented

2026-11-06 21:39:12 +00:00

Owner

Problem

Three places in os-backlog build Backlog::ShellRunner.new inside the call chain instead of receiving the runner from the caller. A test cannot replace the runner at those places, so those paths run real git and gh. The classical test rule for I/O is that the fake enters once at the composition root and every object below receives it. bin/os-backlog and ForgejoBackend#client follow that rule; these three sites do not.

Observed

lib/backlog/cli/issue_commands.rb:30 calls Backlog::IssueShow.build(repo_root:, forgejo:) with no runner:, so issue_show.rb:5 falls back to ShellRunner.new. The issue-show command never uses the runner that CLI.start received.
bin/decision-sweep:9 builds DecisionSweep::Command.new(program: PROGRAM) with no runner:, so decision_sweep/command.rb:6 falls back to ShellRunner.new.
tests/support/in_process_cli_run.rb:8,13: run_cli and run_captured default to the real ShellRunner.new. 18 test files call them without runner:. Only the NoRealTrackerCommands guard stops tea and gh; nothing stops git.

Reproduce

n/a: structure change; the existing suite is the proof.

Approach

Pass the runner the caller already holds. The default arguments in IssueShow.build and DecisionSweep::Command#initialize stay; the defect is callers that never override them.

Required work:

IssueCommands#issue_show passes runner: session.runner to IssueShow.build — check: present runner: session.runner in plugins/os-backlog/lib/backlog/cli/issue_commands.rb
bin/decision-sweep builds Backlog::ShellRunner.new once and passes it as runner: — check: present runner: Backlog::ShellRunner.new in plugins/os-backlog/bin/decision-sweep
run_cli and run_captured default to CliFakeLabelsRunner.new, as run_forgejo_cli does — check: absent Backlog::ShellRunner.new in plugins/os-backlog/tests/support/in_process_cli_run.rb

Rejected: a class-level or global runner slot the test can set. That was CliContext.runner; commit 744ec47b removed it after a parallel-suite race filed junk tickets.

Acceptance criteria

Given a fake runner passed to CLI.start, when issue-show runs on a github: tracker, then the fake runner receives the gh issue view call and no real process starts.
Given the existing suite, when it runs, then no test outside shell_runner_refuses_tracker_commands_test.rb and the files that name ShellRunner.new on purpose constructs a real ShellRunner.
Given bin/decision-sweep, when it runs, then behavior is unchanged.

Owner sketch

class Backlog::CLI::IssueCommands
  def issue_show(number)
    Backlog::IssueShow.build(repo_root:, forgejo:, runner: session.runner)
  end
end

Non-goals

lib/backlog/project_index/registration.rb:27 builds its own ShellRunner; #1594 and #853 replace that read with os-git.
bin/wakeup-poll calls Open3 and system directly. It is pilot-only per plugins/os-backlog/CLAUDE.md and stays as it is.
Removing ShellRunner itself. That waits on the GitHub backend moving to an API client (ADR-0256) and the git calls moving to os-git.

Origin

Trigger: review of why Backlog::ShellRunner exists, session 2026-11-06
Improvised this session: none
Chain: tests cannot fake issue-show or decision-sweep shell calls ← callers build ShellRunner.new mid-chain ← DESIGN (commit 744ec47b injected the runner at CLI.start but did not thread it to every caller)
Root candidate: none found; searched ShellRunner, runner inject, in_process_cli_run, decision-sweep, registration remote get-url, OsGit (nearest: #1594, #853 cover registration.rb only)
Where: plugins/os-backlog/lib/backlog/cli/issue_commands.rb
Session: 38f8ae3f-4000-45bc-b055-7d6cf81a42dc
Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os-plugins-os-backlog/38f8ae3f-4000-45bc-b055-7d6cf81a42dc.jsonl

VERDICT: CREATE

## Problem Three places in os-backlog build `Backlog::ShellRunner.new` inside the call chain instead of receiving the runner from the caller. A test cannot replace the runner at those places, so those paths run real `git` and `gh`. The classical test rule for I/O is that the fake enters once at the composition root and every object below receives it. `bin/os-backlog` and `ForgejoBackend#client` follow that rule; these three sites do not. ## Observed - `lib/backlog/cli/issue_commands.rb:30` calls `Backlog::IssueShow.build(repo_root:, forgejo:)` with no `runner:`, so `issue_show.rb:5` falls back to `ShellRunner.new`. The `issue-show` command never uses the runner that `CLI.start` received. - `bin/decision-sweep:9` builds `DecisionSweep::Command.new(program: PROGRAM)` with no `runner:`, so `decision_sweep/command.rb:6` falls back to `ShellRunner.new`. - `tests/support/in_process_cli_run.rb:8,13`: `run_cli` and `run_captured` default to the real `ShellRunner.new`. 18 test files call them without `runner:`. Only the `NoRealTrackerCommands` guard stops `tea` and `gh`; nothing stops `git`. ## Reproduce n/a: structure change; the existing suite is the proof. ## Approach Pass the runner the caller already holds. The default arguments in `IssueShow.build` and `DecisionSweep::Command#initialize` stay; the defect is callers that never override them. Required work: 1. `IssueCommands#issue_show` passes `runner: session.runner` to `IssueShow.build` — check: present `runner: session.runner` in `plugins/os-backlog/lib/backlog/cli/issue_commands.rb` 2. `bin/decision-sweep` builds `Backlog::ShellRunner.new` once and passes it as `runner:` — check: present `runner: Backlog::ShellRunner.new` in `plugins/os-backlog/bin/decision-sweep` 3. `run_cli` and `run_captured` default to `CliFakeLabelsRunner.new`, as `run_forgejo_cli` does — check: absent `Backlog::ShellRunner.new` in `plugins/os-backlog/tests/support/in_process_cli_run.rb` Rejected: a class-level or global runner slot the test can set. That was `CliContext.runner`; commit 744ec47b removed it after a parallel-suite race filed junk tickets. ## Acceptance criteria 1. Given a fake runner passed to `CLI.start`, when `issue-show` runs on a `github:` tracker, then the fake runner receives the `gh issue view` call and no real process starts. 2. Given the existing suite, when it runs, then no test outside `shell_runner_refuses_tracker_commands_test.rb` and the files that name `ShellRunner.new` on purpose constructs a real `ShellRunner`. 3. Given `bin/decision-sweep`, when it runs, then behavior is unchanged. ## Owner sketch ```ruby class Backlog::CLI::IssueCommands def issue_show(number) Backlog::IssueShow.build(repo_root:, forgejo:, runner: session.runner) end end ``` ## Non-goals - `lib/backlog/project_index/registration.rb:27` builds its own `ShellRunner`; #1594 and #853 replace that read with os-git. - `bin/wakeup-poll` calls `Open3` and `system` directly. It is pilot-only per `plugins/os-backlog/CLAUDE.md` and stays as it is. - Removing `ShellRunner` itself. That waits on the GitHub backend moving to an API client (ADR-0256) and the git calls moving to os-git. ## Origin - Trigger: review of why `Backlog::ShellRunner` exists, session 2026-11-06 - Improvised this session: none - Chain: tests cannot fake `issue-show` or `decision-sweep` shell calls ← callers build `ShellRunner.new` mid-chain ← `DESIGN` (commit 744ec47b injected the runner at `CLI.start` but did not thread it to every caller) - Root candidate: none found; searched ShellRunner, runner inject, in_process_cli_run, decision-sweep, registration remote get-url, OsGit (nearest: #1594, #853 cover registration.rb only) - Where: plugins/os-backlog/lib/backlog/cli/issue_commands.rb - Session: 38f8ae3f-4000-45bc-b055-7d6cf81a42dc - Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os-plugins-os-backlog/38f8ae3f-4000-45bc-b055-7d6cf81a42dc.jsonl VERDICT: CREATE

jared added the

plugin/os-backlog

filed-by/user

category/structure

labels

2026-11-06 21:39:12 +00:00

jared referenced this issue

2026-11-09 17:40:43 +00:00

Build os-git end to end (orchestrator) #1610

jared commented

2026-11-09 17:44:43 +00:00

Author

Owner

Resolution

Done: run_cli and run_captured default to CliFakeLabelsRunner; four tests that built a real ShellRunner now pass CliFakeLabelsRunner. Required work items 1 and 2 were already obsolete: Backlog::IssueShow is gone (issue_show goes through IssueTarget#show and session.ticket, which use the session runner), and DecisionSweep::Command no longer takes or builds a runner. AC1 is moot: the GitHub host uses Octokit (FakeOctokitClient), not gh.

Evidence: main 1b935f83a (79a717bb3, f5dacbc56). os-backlog suite: 551 runs, 1228 assertions, 0 failures, 0 errors. grep ShellRunner.new in plugins/os-backlog/test: only shell_runner_refuses_tracker_commands_test.rb (tests ShellRunner itself). No lib code runs git through ShellRunner; remote reads go through OsGit::Repository.

Follow-ups: none

## Resolution **Done:** run_cli and run_captured default to CliFakeLabelsRunner; four tests that built a real ShellRunner now pass CliFakeLabelsRunner. Required work items 1 and 2 were already obsolete: Backlog::IssueShow is gone (issue_show goes through IssueTarget#show and session.ticket, which use the session runner), and DecisionSweep::Command no longer takes or builds a runner. AC1 is moot: the GitHub host uses Octokit (FakeOctokitClient), not gh. **Evidence:** main 1b935f83a (79a717bb3, f5dacbc56). os-backlog suite: 551 runs, 1228 assertions, 0 failures, 0 errors. grep ShellRunner.new in plugins/os-backlog/test: only shell_runner_refuses_tracker_commands_test.rb (tests ShellRunner itself). No lib code runs git through ShellRunner; remote reads go through OsGit::Repository. **Follow-ups:** none

jared closed this issue

2026-11-09 17:44:43 +00:00

jared referenced this issue

2026-11-09 17:56:29 +00:00

Build os-git end to end (orchestrator) #1610

jared referenced this issue from a commit

2026-11-09 20:45:30 +00:00

refactor(os-backlog): callers pass the runner they hold (#1629)

jared referenced this issue from a commit

2026-11-09 20:45:30 +00:00

test(os-backlog): no test builds a real ShellRunner (#1629)