refactor(agents): port the gh-post requirement adapter to Go

Problem

dt_require_gh_post and dt_gh_post in packages/agents/source/lib/lib.sh check that the gh-post binary exists and forward arguments to it in shell. A missing binary must refuse with a fixed status and message, and no other path may stand in for the post. Shell argument forwarding is where quoting and stream bugs hide.

Part of #588.

Acceptance criteria

AC1: With gh-post absent, the port refuses with the existing status and message, never falls back to a direct GitHub write, and prints nothing else.
AC2: Forwarding parity: the port passes argument boundaries, empty arguments, whitespace and newlines, special characters, and stdin through unchanged. It keeps stdout and stderr separate, returns gh-post's exit status on success and on nonzero, and the standalone requirement check (create-pr.sh:96) stays silent on success.
AC3: With an older agentrepo that lacks the new subcommand, the frozen shell adapter runs, and the probe makes no write. With a supported subcommand that fails or returns partial output, the caller refuses with that status and diagnostic, runs no shell adapter, and makes no second attempt.
AC4: Generated skill output is regenerated in the same pull request, and mise run check-generated-skills passes.

Scope and non-goals

Replace the shell binary requirement check and argument forwarding with a tested implementation that satisfies AC1 to AC4. Keep the current refusal status and message. Add no version gate, because the runtime has none. Consumers whose pinned agentrepo predates the replacement keep today's behaviour.

Non-goal: changing any behaviour a consumer on an older pinned binary relies on. Non-goal: removing the shell adapter, which a separate XS issue owns once supported releases exist.

Approach

Meet AC1 to AC4 with failing cases first for the refusal and capability rules. The mechanism is the implementer's choice within those outcomes.

Dependencies

Blocked by #753 (protection for the replacement packages). Touches packages/agents/source/lib/lib.sh, a protected shell path, so the pull request needs owner review. A follow-up XS issue removes the shell adapter after supported releases ship.

Size

S. One pull request.

Verify plan

AC1: a Go test and a Bats case with gh-post off PATH, written first, asserting status, message and empty stdout.
AC2: Go tests for each forwarding case listed, plus the content-post cases in tests/skills/source/lib/lib.bats run against both implementations with identical expected output.
AC3: stub binaries for four states: the subcommand absent, present and succeeding, present and failing with no output, and present and failing after partial output. Assert zero writes during the capability probe, the expected legacy result when absent, and for each supported failure the exact status and diagnostic reach the caller with zero fallback execution and zero repeat execution, with and without partial output.
AC4: mise run generate-skills leaves no diff and mise run check-generated-skills passes.
Definition of done: D1 each AC is verified at the merged head, with the commands and results cited in the pull request's Verification. D2 an independent review approves the exact merged head, and every finding has a recorded disposition. D3 required checks pass with no gate loosened. D4 the leak scans the repository specifies are clean, and every post made for this work (comments, descriptions) passes the per-post leak review before it is posted. D5 every commit is signed. D6 the documentation matches, or the description says No doc impact with the reason. D7 every commit carries an Assisted-by trailer and the pull request carries the provenance:agent-assisted label. D8 the change stays within the pull request size limits. D9 follow-up work, including deferred review findings, is filed as issues, each with a Problem section and numbered acceptance criteria and linking this issue; the pull request's Related section lists them, and any code that defers work links its issue. D10 the merge closes this issue through a closing reference, the project board shows the state Done, and a final Status: comment names the merge commit.

Risks

A fallback after a supported-command failure would hide an error or post twice. AC3 pins that. Undo: revert the commit, which returns to the shell adapter.

Operator authority

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

Refinement: NOT READY — parity, verification, dependency, and authority gaps
Model: gpt-6.1-sol

Checked

Read live refactor(agents): port the gh-post requirement adapter to Go #756 and its comments once; there are no comments. Read its named epic, Replace complex shell with tested Go, starting with gates and merging #588.
Verified the independent clone’s origin identifies tbhb-dev/devtools. Compared against origin/main at f130498bd7383fe02f8b9d21965f555866abd98c.
AC1 describes an observable refusal. The original returns 2 and prints gh-post is missing; install the gh-post tool before publishing content. followed by a newline to stderr (packages/agents/source/lib/lib.sh:812).
AC2 describes observable forwarding, but its referenced fixtures prove only missing-binary refusal and one ordinary argument list with failure status 2 (tests/skills/source/lib/lib.bats:820, tests/skills/source/lib/lib.bats:827).
AC3 states one independently checkable generated-output outcome. The generator supports checking output names, bytes, and executable modes (tools/generate-skills.sh:121, tools/generate-skills.sh:144).
The risk of weakening a publication gate is correctly identified. The authority value is not.

Gaps

High — Behavior parity is incomplete. AC2 covers argument forwarding only for the existing fixtures. The original forwards "$@", inherits stdin, passes stdout/stderr through, and returns the invoked command’s status (packages/agents/source/lib/lib.sh:818). Add observable requirements for argument boundaries, empty arguments, whitespace/newlines and literal special characters, stdin, separate output streams, success, and nonzero statuses. Cover the standalone requirement check’s silent success as well: callers use it without posting (packages/agents/source/skills/pr/shared/scripts/create-pr.sh:96). Explicitly exclude any original behavior intentionally omitted.
High — Compatibility and enforcement requirements lack acceptance coverage. The scope requires legacy fallback when the new capability is absent and refusal without fallback after a supported command fails or emits partial output. No AC or verification case proves these outcomes. Add separately observable criteria and checks showing that capability detection causes no writes, legacy consumers retain behavior, and supported-command failures cause neither fallback posting nor duplicate attempts.
High — The Verify plan does not prove its ACs. “Write its failing case first, then show it pass” specifies a sequence without identifying assertions. AC2 reuses AC1’s missing-binary fixtures, which cannot prove forwarding. AC3 reuses them too, which cannot prove generation. Map each AC to checks and expected results: exact status and separate streams for AC1; argument-boundary and transport comparisons for AC2; generated-output consistency for AC3. Include the compatibility cases from gap 2. Replace complex shell with tested Go, starting with gates and merging #588’s generation check provides a suitable AC3 check: mise run check-generated-skills.
High — Dependency and owner authority are unresolved. “Replacement-protection issue (P2)” identifies no issue or observable completion condition, so independence and rollout readiness cannot be established. Link the actual prerequisite and state what must hold before this change can roll out. Replace “Operator authority: None” with the applicable owner requirement: Replace complex shell with tested Go, starting with gates and merging #588 explicitly requires owner-approved protection of replacement enforcement paths and the existing owner review process for protected changes. This adapter gates publication (packages/agents/source/lib/lib.sh:811).
Medium — The body prescribes design outside Operator authority. “Behind an agentrepo subcommand” and the prescribed shell-caller arrangement select implementation structure. Move those choices to the PR. Retain requirements for Go migration, compatibility, write-free capability detection, and refusal behavior as observable outcomes.
Medium — Definition-of-done coverage is incomplete. Neither refactor(agents): port the gh-post requirement adapter to Go #756 nor its named epic states the full completion obligations below. Add explicit coverage or identify qualifying child requirements, particularly merged-head AC evidence, independent approval, the specified leak scans, signing/provenance, documentation, size exceptions, follow-ups, and closure evidence.

Split

S, one PR, once the protection prerequisite is identified. The two leaf functions are small, but complete parity, compatibility checks, caller integration, and generated output make XS optimistic. Keep fallback removal separate as already scoped; identify that issue rather than leaving its ownership unnamed.

Definition of done

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

Boxes indicate planning coverage, not completed work. None is fully covered by the live requirements.

Round: 1 of 3

Refinement: NOT READY — verification, design prescriptions, and completion coverage remain
Model: gpt-6.1-sol

Checked

Read live #756, its Round 1 comment once, and the named issues #588 and #753. Verified the independent clone’s origin identifies tbhb-dev/devtools. Compared claims against origin/main at f130498bd7383fe02f8b9d21965f555866abd98c.

Round 1 findings:

Resolved — Behavior parity. AC2 now requires arguments and stdin to pass “through unchanged,” separate output streams, success and failure statuses, and silent standalone success. These match packages/agents/source/lib/lib.sh:813, packages/agents/source/lib/lib.sh:820, and the standalone caller at packages/agents/source/skills/pr/shared/scripts/create-pr.sh:96. AC1 retains the original missing-binary refusal: status 2 and one stderr diagnostic (packages/agents/source/lib/lib.sh:814).
Resolved — Compatibility requirements. AC3 explicitly covers legacy fallback, a write-free probe, supported-command failure and partial output, and “no second attempt.” These are observable requirements.
Partly resolved — Verification. The plan now maps AC1–AC4 to relevant checks. AC2 adds tests beyond the original ordinary-argument fixture (tests/skills/source/lib/lib.bats:827). AC4’s generation check compares names, content, and executable modes (tools/generate-skills.sh:121). However, AC3’s stated assertions cover only “which implementation ran and how many times”; they do not establish every required outcome. See gap 1.
Resolved — Dependency and authority. “Blocked by build(agents): protect replacement Go enforcement paths #753” identifies the protection prerequisite. build(agents): protect replacement Go enforcement paths #753 AC4 requires protection to merge before callers switch, consistent with Replace complex shell with tested Go, starting with gates and merging #588’s rollout requirement. The live Operator authority section requires owner review of the exact head and grants no write authority. The shell path is protected by .agents/agent-skills.conf:4, .agents/agent-skills.conf:5, and .agents/agent-skills.conf:9.
Unresolved — Design prescriptions. The body still requires migration “behind an agentrepo subcommand” and prescribes the shell-caller arrangement outside Operator authority. See gap 2.
Unresolved — Definition-of-done coverage. The live body’s “Verify plan” and its named issues still do not cover the complete obligations below. See gap 3.

No additional substantive gap found. AC1, AC2, and AC4 describe testable outcomes; S remains a reasonable estimate for the bounded port and integration work.

Gaps

High — AC3’s verification does not prove its full contract. Implementation selection and invocation counts alone cannot prove a write-free capability probe or preservation of the supported command’s status and diagnostic. Extend the plan to assert zero probe writes, the expected legacy result, exact supported-failure status and diagnostic, and zero fallback or repeat execution. Cover failure both with and without partial output; the current plan names only the partial-output failure case.
Medium — Implementation choices remain prescribed outside Operator authority. Scope, Approach, and Size select a subcommand, shell-caller arrangement, test-first sequence, and Go package structure. Keep the Go migration and observable compatibility, refusal, forwarding, and generation requirements. Leave implementation structure and development sequence to the PR, or record an actual operator decision in Operator authority. No additional design specification is needed.
Medium — Completion requirements remain incomplete. Add explicit planning coverage for D1–D10, directly or through an identified applicable requirement. Replace complex shell with tested Go, starting with gates and merging #588 covers required checks and replacement protection, and refactor(agents): port the gh-post requirement adapter to Go #756 covers exact-head owner review, but neither establishes the full completion contract. In particular, merged-head AC evidence, independent-review finding disposition, the specified leak checks, signing, documentation, provenance, size exceptions, follow-up handling, and closure evidence remain uncovered.

Split

S, one PR is appropriate. Keep the port, compatibility integration, parity checks, and generated output together. Retain #753 as the protection prerequisite and keep fallback removal separate. No further implementation split is needed.

Definition of done

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

Boxes indicate planning coverage, not completed work. None is fully covered by the live requirements.

Round: 2 of 3

Refinement: NOT READY — completion requirements remain partly specified
Model: gpt-6.1-sol

Checked

Read live #756, its Round 1 comment once, and the named issues #588 and #753. Verified the independent clone’s origin identifies tbhb-dev/devtools. Checked claims against origin/main at f130498bd7383fe02f8b9d21965f555866abd98c.

Round 1 findings:

Resolved — Behavior parity. AC2 requires forwarding “through unchanged,” including argument boundaries, stdin, separate streams, exit statuses, and silent requirement-check success. These match packages/agents/source/lib/lib.sh:813, packages/agents/source/lib/lib.sh:820, and packages/agents/source/skills/pr/shared/scripts/create-pr.sh:96. AC1 preserves status 2 and the sole missing-binary stderr diagnostic at packages/agents/source/lib/lib.sh:814.
Resolved — Compatibility and enforcement. AC3 requires the legacy result when capability is absent, a write-free probe, preservation of supported-command failures, and “no second attempt.” The Verify plan explicitly checks these outcomes with and without partial output.
Resolved — Verification mapping. The plan now supplies checks for AC1–AC4, including “zero fallback execution and zero repeat execution.” AC2 adds transport cases beyond the limited existing fixtures at tests/skills/source/lib/lib.bats:820 and tests/skills/source/lib/lib.bats:827. AC4’s generation checks compare output names, bytes, and executable modes through tools/generate-skills.sh:121 and tools/generate-skills.sh:144.
Resolved — Dependency and authority. “Blocked by build(agents): protect replacement Go enforcement paths #753” identifies the prerequisite; build(agents): protect replacement Go enforcement paths #753 AC4 requires its merge before callers switch and requires the port PR to cite its merged commit. Operator authority requires exact-head owner review and grants no write authority. Existing protection is confirmed at .agents/agent-skills.conf:4, .agents/agent-skills.conf:5, and .agents/agent-skills.conf:9.
Resolved — Design prescriptions. The body now leaves the mechanism to “the implementer’s choice.” The remaining compatibility and generation requirements specify observable outcomes without selecting package layout or library structure.
Partly resolved — Definition of done. The added summary requires evidence, review, checks, signing, documentation, provenance, size compliance, follow-ups, and closure. However, “Provenance is recorded” and similarly broad statements do not establish every required obligation below.

No additional substantive gap found. The ACs are observable and testable, and their verification checks address the stated behavior.

Gaps

Medium — Complete the existing definition-of-done coverage. Expand the summary or reference an explicit applicable requirement covering the unchecked items below. Specifically require merged-head AC verification with commands/results or links; disposition of independent-review findings; all three named leak checks; a justified documentation exception in Verification; the precise trailers and label; follow-up issue structure and links; and closing-reference, board, and final status evidence. These are outstanding parts of Round 1 gap 6, rather than additional implementation requirements.

Split

S, one PR remains appropriate for the bounded port, compatibility integration, parity checks, and generated output. Keep #753 as the prerequisite and fallback removal separate. No further split is needed.

Definition of done

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

Boxes indicate planning coverage, not completed work.

Round: 3 of 3

Refinement: READY — Round 1 gaps resolved
Model: gpt-6.1-sol

Checked

Read the live #756 body, its Round 1 refinement comment once, and named issues #588 and #753. Verified the independent clone’s origin identifies tbhb-dev/devtools. Checked against origin/main at f130498bd7383fe02f8b9d21965f555866abd98c.

Round 1 findings:

Resolved — Behavior parity. AC2 requires forwarding “through unchanged,” including argument boundaries, empty arguments, whitespace, newlines, special characters, stdin, separate streams, and success/failure statuses. It also requires silent standalone success. These match packages/agents/source/lib/lib.sh:813, packages/agents/source/lib/lib.sh:820, and packages/agents/source/skills/pr/shared/scripts/create-pr.sh:96. AC1 preserves the sole stderr diagnostic and status 2 at packages/agents/source/lib/lib.sh:814.
Resolved — Compatibility and enforcement. AC3 requires a write-free capability probe, legacy behavior for an absent capability, propagation of supported-command failures, and “no second attempt,” including partial-output failures.
Resolved — Verification mapping. Each AC has relevant checks. AC3 explicitly asserts statuses, diagnostics, probe writes, fallback execution, and repeat execution. AC2 adds transport cases beyond the existing limited fixtures at tests/skills/source/lib/lib.bats:820 and tests/skills/source/lib/lib.bats:827. AC4 checks generated consistency; the generator compares names, bytes, and executable modes at tools/generate-skills.sh:121.
Resolved — Dependency and authority. “Blocked by build(agents): protect replacement Go enforcement paths #753” identifies the prerequisite. build(agents): protect replacement Go enforcement paths #753 AC3 requires protection to merge before caller migration and requires the port PR to cite that commit. Operator authority requires exact-head owner review and grants no write authority. Existing shell protection is recorded at .agents/agent-skills.conf:4, .agents/agent-skills.conf:5, and .agents/agent-skills.conf:9.
Resolved — Design prescriptions. The body leaves the mechanism to “the implementer’s choice.” The remaining requirements establish the Go migration and observable compatibility, refusal, forwarding, and generation outcomes without prescribing package layout or library choices.
Resolved — Definition of done. The Verify plan now addresses D1–D10 explicitly, including merged-head evidence, independent review and finding disposition, signing, provenance, documentation, follow-ups, and closure. Its requirement for repository-specified leak scans identifies existing checks at .devtools/tasks/security.toml:24 and .devtools/tasks/leaks.toml:2; private-literal scanning is documented at .devtools/scripts/check-leaks.sh:4.

No new substantive gap found. The ACs are observable and testable, and the verification plan supplies checks capable of proving their outcomes.

Gaps

None.

Split

S, one PR is appropriate for the bounded port, compatibility integration, parity checks, and generated output. Retain #753 as the prerequisite and keep adapter removal separate. No further split is needed.

Definition of done

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

Ticked boxes indicate planning coverage, not completed work.

Round: 1 of 3

Navigation Menu

Repository navigation

refactor(agents): port the gh-post requirement adapter to Go #756

Problem

Acceptance criteria

Scope and non-goals

Approach

Dependencies

Size

Verify plan

Risks

Operator authority

Checked

Gaps

Split

Definition of done

Checked

Gaps

Split

Definition of done

Checked

Gaps

Split

Definition of done

Checked

Gaps

Split

Definition of done

Metadata

Assignees

Labels

Type

Fields

Projects

Milestone

Relationships

Development

refactor(agents): port the gh-post requirement adapter to Go #756

Description

Problem

Acceptance criteria

Scope and non-goals

Approach

Dependencies

Size

Verify plan

Risks

Operator authority

Activity

tbhb-agent commented on Oct 26, 2026

Checked

Gaps

Split

Definition of done

tbhb-agent commented on Oct 26, 2026

Checked

Gaps

Split

Definition of done

tbhb-agent commented on Oct 26, 2026

Checked

Gaps

Split

Definition of done

tbhb-agent commented on Oct 26, 2026

Checked

Gaps

Split

Definition of done

Metadata

Metadata

Assignees

Labels

Type

Fields

Projects

Milestone

Relationships

Development

Issue actions