jared/cc-os

Watch

Fork 0

Cross-plugin binary lookup by relative path #635

New issue

Closed

opened 2026-09-23 12:04:54 +00:00 by jared · 2 comments

jared commented

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

Owner

Problem

Three cross-plugin or cross-process binary/shell lookups are fragile: a hardcoded relative path with a silent fallback, a boolean-only system call, and a bare backtick that bypasses an existing shell abstraction. These are legitimate process boundaries (separate plugins, no shared load path) — the fix is to make the lookup and the call robust, not to merge the boundaries.

Observed

Verified on main today (2026-09-24):

plugins/os-sdlc/lib/os_sdlc/adapter/tea_tracker/body_fetch.rb:22-25 os_backlog_bin hardcodes File.expand_path("../../../../os-backlog/bin/os-backlog", __dir__) and silently falls back to the bare string "os-backlog" if that path is missing. Sole caller is TeaTracker#fetch_body (tea_tracker.rb:37), which wraps any StandardError into HarnessError. No behavior test pins the current fallback.
plugins/os/lib/worktree/finish_command.rb:124-126 system('os-sdlc-runner', ...) || warn(...) reports only a boolean; it cannot distinguish "binary not found" from "binary found but exited nonzero." Pinned by plugins/os/tests/worktree/finish_command_sweep_failure_does_not_block_removal_test.rb, which asserts /os-sdlc-runner/ appears in stderr and that the process exits 0.
plugins/os-backlog/lib/backlog/config_write_command.rb:39-41 uses a bare backtick, `git remote get-url origin 2>/dev/null`.strip, instead of the plugin's own Backlog::Issues::ShellRunner#capture(*cmd), which wraps Open3.capture3 and returns [out, success?] (stderr discarded, preserving the 2>/dev/null semantics). tests/cli_config_write_command_test.rb runs in a non-git tmpdir, so the remote resolves to nil either way — this test does not distinguish the two implementations.
Plugins install to ~/.claude/plugins/<name>/bin, and Claude Code puts those directories on PATH (which os-backlog resolves to /home/jared/.claude/plugins/os-backlog/bin/os-backlog in an installed session). The sibling relative path in body_fetch.rb is the path that fires in a source checkout, not an installed plugin.
ADR-0213 records that any future ENV read in plugins/os-sdlc/lib is a defect. This rules out a new environment-variable override such as OS_BACKLOG_BIN; resolution must go through PATH only.

Reproduce

grep -n 'expand_path("../../../../' plugins/os-sdlc/lib
grep -n '`git' plugins/os-backlog/lib

Approach

Add a small BinaryLocator per affected plugin. It searches PATH entries for the target binary name; on a miss it raises a named error listing every PATH entry it tried. For os-backlog, the sibling relative path used in a source checkout remains a second candidate, tried after PATH, and is included in the "tried" list on failure. os-sdlc-runner is resolved the same way from finish_command.rb: when the binary is missing, warn with "os-sdlc-runner not found on PATH (tried: ...), usage not swept"; when it is found but exits nonzero, warn with the resolved path and the exit status; sweep failure never blocks worktree removal either way. config_write_command.rb's origin_remote calls Backlog::Issues::ShellRunner.new.capture('git', 'remote', 'get-url', 'origin') and treats the result as nil unless the call succeeded and the output is non-empty.

Required work:

Add OsSdlc::Adapter::BinaryLocator and use it in body_fetch.rb in place of the hardcoded expand_path plus silent string fallback — check: grep -n 'expand_path("../../../../' plugins/os-sdlc/lib returns nothing.
On locator miss, raise a named error listing every PATH entry tried plus the sibling-checkout candidate — check: a forced-miss test asserts the error message names the tried paths.
Add Worktree::BinaryLocator (or reuse a shared one) and use it in finish_command.rb for os-sdlc-runner, replacing the boolean-only system(...) || — check: finish_command_sweep_failure_does_not_block_removal_test.rb still asserts /os-sdlc-runner/ in stderr and exit 0, unchanged.
Warn with the resolved path and exit status when the binary is found but fails, distinct from the not-found case.
Replace the bare backtick in config_write_command.rb with Backlog::Issues::ShellRunner#capture — check: grep -n '\git' plugins/os-backlog/lib` returns nothing.
Confirm cli_config_write_command_test.rb still passes (remote nil in its non-git tmpdir either way).

Rejected: an ENV variable override such as OS_BACKLOG_BIN (forbidden by ADR-0213). Merging the plugins' load paths (these are deliberate process boundaries, not an accident to fix by removing them).

Non-goals: changing what happens when the sweep or body-fetch legitimately fails for a reason other than a missing binary. No ADR change — this is a defect fix within a boundary ADR-0213 already governs.

Condition that would make this plan wrong: if any installed-plugin environment does not actually put ~/.claude/plugins/*/bin on PATH (contrary to the which os-backlog check above), then PATH-only resolution would break the installed case; verify with which os-backlog and which os-sdlc-runner in a real installed session before relying on it.

VERDICT: CREATE

Acceptance criteria

Given os-backlog is on PATH, when body_fetch.rb resolves its binary, then it uses the PATH match, not the hardcoded relative path.
Given os-backlog is on neither PATH nor the sibling checkout location, when body_fetch.rb resolves its binary, then it raises an error naming every location it tried, rather than silently falling back to a bare unresolved string.
Given os-sdlc-runner is not found on PATH, when finish_command.rb runs its sweep, then it warns "os-sdlc-runner not found on PATH (tried: ...), usage not swept" and does not block worktree removal.
Given os-sdlc-runner is found but exits nonzero, when finish_command.rb runs its sweep, then it warns with the resolved path and the exit status, and still does not block worktree removal.
Given config_write_command.rb needs the origin remote, when it looks it up, then it calls Backlog::Issues::ShellRunner#capture, not a bare backtick, and returns nil under the same conditions as before (call fails or output empty).
Given these are separate plugin process boundaries, when this ticket closes, then no plugin load paths have been merged.

Owner sketch

module OsSdlc
  module Adapter
    class BinaryLocator
      def self.resolve(name, fallback_paths: [])
        # search ENV["PATH"] entries, then fallback_paths, else raise NotFound listing all tried
      end
    end
  end
end

module Worktree
  class BinaryLocator
    def self.resolve(name)
      # same PATH-only search, raise NotFound listing tried entries
    end
  end
end

Non-goals

Merging plugin load paths.
Any environment-variable override for binary location (forbidden by ADR-0213).

Origin

Trigger: shell-call/ENV audit, session 16bf37b3-8cff-4121-896c-4a5fc61f2e9e (2026-09-23), user-requested.
Improvised this session: none.
Chain: fragile cross-plugin binary lookup <- hardcoded relative path / bare backtick instead of a resolved, injectable path <- DESIGN, plugins are separate load paths by design (not a bug to fix by merging them).
Root candidate: this ticket.
Where: plugins/os-sdlc/lib/os_sdlc/adapter/tea_tracker/body_fetch.rb:24; plugins/os/lib/worktree/finish_command.rb:125; plugins/os-backlog/lib/backlog/config_write_command.rb:40
Session: 16bf37b3-8cff-4121-896c-4a5fc61f2e9e
Transcript: /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 Three cross-plugin or cross-process binary/shell lookups are fragile: a hardcoded relative path with a silent fallback, a boolean-only system call, and a bare backtick that bypasses an existing shell abstraction. These are legitimate process boundaries (separate plugins, no shared load path) — the fix is to make the lookup and the call robust, not to merge the boundaries. ## Observed Verified on main today (2026-09-24): - `plugins/os-sdlc/lib/os_sdlc/adapter/tea_tracker/body_fetch.rb:22-25` `os_backlog_bin` hardcodes `File.expand_path("../../../../os-backlog/bin/os-backlog", __dir__)` and silently falls back to the bare string `"os-backlog"` if that path is missing. Sole caller is `TeaTracker#fetch_body` (`tea_tracker.rb:37`), which wraps any `StandardError` into `HarnessError`. No behavior test pins the current fallback. - `plugins/os/lib/worktree/finish_command.rb:124-126` `system('os-sdlc-runner', ...) || warn(...)` reports only a boolean; it cannot distinguish "binary not found" from "binary found but exited nonzero." Pinned by `plugins/os/tests/worktree/finish_command_sweep_failure_does_not_block_removal_test.rb`, which asserts `/os-sdlc-runner/` appears in stderr and that the process exits 0. - `plugins/os-backlog/lib/backlog/config_write_command.rb:39-41` uses a bare backtick, `` `git remote get-url origin 2>/dev/null`.strip ``, instead of the plugin's own `Backlog::Issues::ShellRunner#capture(*cmd)`, which wraps `Open3.capture3` and returns `[out, success?]` (stderr discarded, preserving the `2>/dev/null` semantics). `tests/cli_config_write_command_test.rb` runs in a non-git tmpdir, so the remote resolves to nil either way — this test does not distinguish the two implementations. - Plugins install to `~/.claude/plugins/<name>/bin`, and Claude Code puts those directories on `PATH` (`which os-backlog` resolves to `/home/jared/.claude/plugins/os-backlog/bin/os-backlog` in an installed session). The sibling relative path in `body_fetch.rb` is the path that fires in a source checkout, not an installed plugin. - ADR-0213 records that any future `ENV` read in `plugins/os-sdlc/lib` is a defect. This rules out a new environment-variable override such as `OS_BACKLOG_BIN`; resolution must go through `PATH` only. ## Reproduce grep -n 'expand_path("../../../../' plugins/os-sdlc/lib grep -n '`git' plugins/os-backlog/lib ## Approach Add a small `BinaryLocator` per affected plugin. It searches `PATH` entries for the target binary name; on a miss it raises a named error listing every `PATH` entry it tried. For `os-backlog`, the sibling relative path used in a source checkout remains a second candidate, tried *after* `PATH`, and is included in the "tried" list on failure. `os-sdlc-runner` is resolved the same way from `finish_command.rb`: when the binary is missing, warn with "os-sdlc-runner not found on PATH (tried: ...), usage not swept"; when it is found but exits nonzero, warn with the resolved path and the exit status; sweep failure never blocks worktree removal either way. `config_write_command.rb`'s `origin_remote` calls `Backlog::Issues::ShellRunner.new.capture('git', 'remote', 'get-url', 'origin')` and treats the result as nil unless the call succeeded and the output is non-empty. Required work: 1. Add `OsSdlc::Adapter::BinaryLocator` and use it in `body_fetch.rb` in place of the hardcoded `expand_path` plus silent string fallback — check: `grep -n 'expand_path("../../../../' plugins/os-sdlc/lib` returns nothing. 2. On locator miss, raise a named error listing every `PATH` entry tried plus the sibling-checkout candidate — check: a forced-miss test asserts the error message names the tried paths. 3. Add `Worktree::BinaryLocator` (or reuse a shared one) and use it in `finish_command.rb` for `os-sdlc-runner`, replacing the boolean-only `system(...) ||` — check: `finish_command_sweep_failure_does_not_block_removal_test.rb` still asserts `/os-sdlc-runner/` in stderr and exit 0, unchanged. 4. Warn with the resolved path and exit status when the binary is found but fails, distinct from the not-found case. 5. Replace the bare backtick in `config_write_command.rb` with `Backlog::Issues::ShellRunner#capture` — check: `grep -n '\`git' plugins/os-backlog/lib` returns nothing. 6. Confirm `cli_config_write_command_test.rb` still passes (remote nil in its non-git tmpdir either way). Rejected: an `ENV` variable override such as `OS_BACKLOG_BIN` (forbidden by ADR-0213). Merging the plugins' load paths (these are deliberate process boundaries, not an accident to fix by removing them). Non-goals: changing what happens when the sweep or body-fetch legitimately fails for a reason other than a missing binary. No ADR change — this is a defect fix within a boundary ADR-0213 already governs. Condition that would make this plan wrong: if any installed-plugin environment does not actually put `~/.claude/plugins/*/bin` on `PATH` (contrary to the `which os-backlog` check above), then PATH-only resolution would break the installed case; verify with `which os-backlog` and `which os-sdlc-runner` in a real installed session before relying on it. VERDICT: CREATE ## Acceptance criteria 1. Given `os-backlog` is on `PATH`, when `body_fetch.rb` resolves its binary, then it uses the `PATH` match, not the hardcoded relative path. 2. Given `os-backlog` is on neither `PATH` nor the sibling checkout location, when `body_fetch.rb` resolves its binary, then it raises an error naming every location it tried, rather than silently falling back to a bare unresolved string. 3. Given `os-sdlc-runner` is not found on `PATH`, when `finish_command.rb` runs its sweep, then it warns "os-sdlc-runner not found on PATH (tried: ...), usage not swept" and does not block worktree removal. 4. Given `os-sdlc-runner` is found but exits nonzero, when `finish_command.rb` runs its sweep, then it warns with the resolved path and the exit status, and still does not block worktree removal. 5. Given `config_write_command.rb` needs the origin remote, when it looks it up, then it calls `Backlog::Issues::ShellRunner#capture`, not a bare backtick, and returns nil under the same conditions as before (call fails or output empty). 6. Given these are separate plugin process boundaries, when this ticket closes, then no plugin load paths have been merged. ## Owner sketch ```ruby module OsSdlc module Adapter class BinaryLocator def self.resolve(name, fallback_paths: []) # search ENV["PATH"] entries, then fallback_paths, else raise NotFound listing all tried end end end end module Worktree class BinaryLocator def self.resolve(name) # same PATH-only search, raise NotFound listing tried entries end end end ``` ## Non-goals - Merging plugin load paths. - Any environment-variable override for binary location (forbidden by ADR-0213). ## Origin - Trigger: shell-call/ENV audit, session 16bf37b3-8cff-4121-896c-4a5fc61f2e9e (2026-09-23), user-requested. - Improvised this session: none. - Chain: fragile cross-plugin binary lookup <- hardcoded relative path / bare backtick instead of a resolved, injectable path <- DESIGN, plugins are separate load paths by design (not a bug to fix by merging them). - Root candidate: this ticket. - Where: plugins/os-sdlc/lib/os_sdlc/adapter/tea_tracker/body_fetch.rb:24; plugins/os/lib/worktree/finish_command.rb:125; plugins/os-backlog/lib/backlog/config_write_command.rb:40 - Session: 16bf37b3-8cff-4121-896c-4a5fc61f2e9e - Transcript: /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

project/cc-os

filed-by/user

labels

2026-09-23 12:04:54 +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 added the

ready-for-agent

label

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

jared commented

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

Author

Owner

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

jared referenced this issue

2026-09-24 20:10:10 +00:00

body_fetch.rb resolves os-backlog via PATH, fails naming paths tried (#635a) #702

jared referenced this issue

2026-09-24 20:10:19 +00:00

finish_command.rb sweep names the os-sdlc-runner path it tried (#635b) #703

jared referenced this issue

2026-09-24 20:10:21 +00:00

config_write_command.rb origin remote via Backlog::Issues::ShellRunner (#635c) #704

jared commented

2026-09-24 20:10:26 +00:00

Author

Owner

Resolution

Done: Split into per-plugin children because implementation-open needs one --target per run

Evidence: #702, #703, #704

Follow-ups: #702 body_fetch.rb resolves os-backlog via PATH, fails naming paths tried — VERDICT: CREATE
#703 finish_command.rb sweep names the os-sdlc-runner path it tried — VERDICT: CREATE
#704 config_write_command.rb origin remote via Backlog::Issues::ShellRunner — VERDICT: CREATE

## Resolution **Done:** Split into per-plugin children because implementation-open needs one --target per run **Evidence:** #702, #703, #704 **Follow-ups:** #702 body_fetch.rb resolves os-backlog via PATH, fails naming paths tried — VERDICT: CREATE #703 finish_command.rb sweep names the os-sdlc-runner path it tried — VERDICT: CREATE #704 config_write_command.rb origin remote via Backlog::Issues::ShellRunner — VERDICT: CREATE

jared closed this issue

2026-09-24 20:10:27 +00:00