Skip to content

refactor(agents): remove the shell fallback for discovery and worktree readiness #772

Open

Description

@tbhb-agent

Problem

The discovery and worktree-readiness port (#755) keeps a frozen shell path for consumers whose pinned agentrepo lacks the new subcommand. That path needs a planned end. Left in place after supported releases ship, it keeps two implementations of the same discovery logic and the probe that chooses between them.

Part of #588.

Acceptance criteria

  1. AC1: After the change, dt_root, dt_git_dir, dt_repo_name, dt_state_dir, dt_digest, dt_head, dt_harness, dt_session_id, dt_path, dt_agent_context and worktree-ready.sh call the Go subcommand only. No shell implementation of them remains in packages/agents/source/lib/.
  2. AC2: A caller whose agentrepo lacks the subcommand exits nonzero, writes a message that names the minimum supported version to standard error, and writes nothing to standard output.
  3. AC3: The existing parity cases for the migrated values pass against the Go path alone.
  4. AC4: Generated skill output is regenerated and mise run check-generated-skills passes.

Scope and non-goals

Delete the shell implementations and the capability probe for the values #755 migrates, and raise the documented minimum agentrepo version to the first release that has the subcommand.

Non-goal: porting any further script. Non-goal: removing a fallback owned by another port.

Approach

Remove the shell bodies and the probe, call the Go subcommand directly, regenerate skills, and update the migration note with the new floor.

Dependencies

Blocked by #755 and by evidence that a supported release containing its subcommand has shipped and that the supported-version window (N-2) no longer includes a binary without it. The pull request touches protected paths and needs owner review.

Size

XS. One deletion pull request.

Verify plan

  • AC1: git grep for the removed function bodies and the probe returns nothing in the source tree.
  • AC2: a Bats case with a stub binary that lacks the subcommand asserts the nonzero exit status, the minimum-version message on standard error and empty standard output.
  • AC3: the parity cases from refactor(agents): port skill capability and worktree context discovery to Go #755 run unchanged.
  • AC4: mise run generate-skills, mise run skill-tokens and apm install leave no diff, then mise run check-generated-skills passes.

Risks

Consumers pinned below the floor break. The N-2 dependency gate is what prevents this. Undo: revert the commit.

Operator authority

A standing rule names the operator: the change edits protected shell paths, so the pull request needs the owner's exact-head review. It grants no write authority.

Activity

  1. added theissue type on Oct 26, 2026
  2. tbhb-agent commented on Oct 26, 2026

    @tbhb-agent
    ContributorAuthor

    Refinement: NOT READY — refusal contract and delegation verification need clarification
    Model: gpt-6.1-sol

    Checked

    Gaps

    1. Medium — AC2 leaves the refusal status and diagnostic stream implicit.
      It requires a minimum-version message and empty stdout, but does not explicitly require a nonzero exit status or place the message on stderr. Its verification likewise asserts neither. These matter when callers consume discovery results. The current version-floor refusal prints to stderr and exits 1 (packages/agents/source/lib/lib.sh:45, packages/agents/source/lib/lib.sh:172).

      Action: State that an unsupported binary produces a nonzero status, a diagnostic on stderr naming the minimum version, and empty stdout. Assert those outcomes in the AC2 check.

    2. Medium — AC1’s negative search does not prove its positive requirement.
      Finding no removed bodies or capability probe proves their absence; it does not establish that every listed function and worktree-ready.sh actually invokes Go. A missing or disconnected entry point could satisfy that search. The current entry points include both value-producing functions and a status-only context check (packages/agents/source/lib/lib.sh:70, packages/agents/source/lib/agent.sh:49).

      Action: Supplement the search with caller-level evidence that every listed entry point reaches the Go command. Identify existing refactor(agents): port skill capability and worktree context discovery to Go #755 coverage that supplies this evidence, or add a check recording invocation for each entry point. Preserve the existing parity checks.

    Split

    One deletion PR remains plausible. Keep #755 and the supported-release/N-2 evidence as prerequisites. No further split is warranted.

    Definition of done

    The definition of done for #772 (Feature). Tick each item and give its evidence.

    Boxes indicate planning coverage, not completed work.

    • D1 The author verified each AC on the merged head, and the Verification section of the pull request cites each AC ID with its evidence: the command and its result, or a link.

      Evidence: The Verify plan maps AC IDs, but does not require merged-head verification and corresponding PR evidence.

    • D2 An independent reviewer approved the exact merged head, and the author resolved each finding or filed it as a follow-up.

      Evidence: Exact-head owner review is required; finding resolution or follow-up recording is not specified.

    • D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact head.

      Evidence: Replace complex shell with tested Go, starting with gates and merging #588 AC8 requires checks, but the complete same-head gate-loosening condition is not explicit.

    • D4 The leak scans come back clean: gitleaks, devtools' check-leaks.sh with the repository's private-strings list, and the leak review of each post.

      Evidence: Replace complex shell with tested Go, starting with gates and merging #588 requires applicable scanner gates without identifying this complete set.

    • D5 The author signed each commit, the squash included.

      Evidence: The named issues do not require signing every delivery commit and squash.

    • D6 The docs match the change (README, docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why.

      Evidence: Scope requires raising the documented minimum version; Approach requires updating the migration note. The existing floor is documented at docs/agent-skills-migration.md:9.

    • D7 The commits and the squash have Assisted-by trailers, and the pull request has the provenance:agent-assisted label.

      Evidence: These delivery requirements are absent from the named issues.

    • D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit.

      Evidence: The XS estimate does not specify size-limit compliance or exception approval.

    • D9 The author files follow-ups and deferred findings as issues with at least a Problem and numbered AC and lists them under the Related section of the pull request. Any work left in the code links its issue.

      Evidence: The named dependencies are traceable, but the complete follow-up requirements are not specified.

    • D10 The merge closes the issue through a closing reference and the board shows Done. The final Status: comment names the merge commit.

      Evidence: The named issues do not specify this closure evidence.

    Round: 1 of 3

  3. tbhb-agent commented on Oct 26, 2026

    @tbhb-agent
    ContributorAuthor

    Refinement: READY — bounded fallback removal with observable compatibility requirements
    Model: gpt-6.1-sol

    Checked

    Gaps

    None.

    Split

    One removal PR is appropriate after #755 and the supported-release/N-2 prerequisites are satisfied. No further split is needed.

    Definition of done

    The definition of done for #772 (Feature). Tick each item and give its evidence.

    Boxes indicate planning coverage, not completed work. Unchecked delivery requirements below do not add refinement blockers.

    • D1 The author verified each AC on the merged head, and the Verification section of the pull request cites each AC ID with its evidence: the command and its result, or a link.

      Evidence: The Verify plan maps every AC ID to checks, but does not explicitly require merged-head verification and PR evidence.

    • D2 An independent reviewer approved the exact merged head, and the author resolved each finding or filed it as a follow-up.

      Evidence: Exact-head owner review is required; finding resolution or follow-up recording is not specified.

    • D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact head.

      Evidence: Replace complex shell with tested Go, starting with gates and merging #588 AC8 requires applicable checks, but the complete same-head gate-loosening condition is not explicit.

    • D4 The leak scans come back clean: gitleaks, devtools' check-leaks.sh with the repository's private-strings list, and the leak review of each post.

      Evidence: Replace complex shell with tested Go, starting with gates and merging #588 requires applicable scanner gates without naming this complete set.

    • D5 The author signed each commit, the squash included.

      Evidence: The named issues do not specify signing every commit and squash.

    • D6 The docs match the change (README, docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why.

      Evidence: Scope requires raising the documented minimum version; Approach requires updating the migration note. The existing documentation is at docs/agent-skills-migration.md:9.

    • D7 The commits and the squash have Assisted-by trailers, and the pull request has the provenance:agent-assisted label.

      Evidence: These delivery requirements are not specified in the named issues.

    • D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit.

      Evidence: The XS estimate supports one PR but does not specify size-limit compliance or exception approval.

    • D9 The author files follow-ups and deferred findings as issues with at least a Problem and numbered AC and lists them under the Related section of the pull request. Any work left in the code links its issue.

      Evidence: Dependencies are named, but the complete follow-up requirements are not specified.

    • D10 The merge closes the issue through a closing reference and the board shows Done. The final Status: comment names the merge commit.

      Evidence: The named issues do not specify this closure evidence.

    Round: 2 of 3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    refinement:readyPassed the Definition of Ready; may be picked up

    Type

    Fields

    Priority

    None yet

    Work type

    None yet

    Projects

    • Status
      Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions