Skip to content

refactor(tools): move skill generation to Go #807

Description

@tbhb-agent

Problem

Skill generation still renders manifests, assembles package output and detects drift in shell. On origin/main at a0347a715fab580ece45edfcb3d13509620459a8, these behaviors are in tools/generate-skills.sh:74, tools/generate-skills.sh:92, tools/generate-skills.sh:123 and tools/generate-skills.sh:134. A failed discovery or comparison must not produce apparently complete output or a clean check.

This is the generation split of #332. Part of #588.

Acceptance criteria

  1. AC1: Generating from the same canonical inputs produces the frozen generator's output names, file bytes and executable modes for each supported harness.
  2. AC2: Generation leaves content outside its generated ownership unchanged, including existing token records and package skills without canonical sources.
  3. AC3: Missing includes and overlapping shared/harness output paths fail with the existing diagnostic and nonzero status.
  4. AC4: Check mode reports the same missing-output and per-path drift findings as the frozen generator, with silence and status 0 only for matching output.
  5. AC5: A discovery, read, copy, fingerprint or comparison failure produces a nonzero result even when the failed operation also returns plausible output.
  6. AC6: Non-functional: check mode leaves the repository's files and Git state unchanged.
  7. AC7: Existing entry points end as thin shell wrappers that probe the Go subcommand without side effects and use the frozen shell only when that capability is absent.
  8. AC8: A supported Go invocation that fails returns failure without running the frozen shell or repeating generation.
  9. AC9: Non-functional: configurations and invocations accepted by release N-2 retain their output, statuses and behavior without requiring a newer binary or a consumer configuration change.
  10. AC10: The Go command produces the same generated bytes and executable-mode decisions on supported Linux, macOS and Windows environments.

Scope and non-goals

Preserve the complete generation contract: includes and final-line handling; harnesses with and without templates; shared and harness-specific content; conditional shared-library and worktree-helper inclusion; removal of stale generated files; preservation and exclusion of token records; check-mode comparison of names, bytes and executable modes; diagnostic ordering, totals and usage status. Evidence: tools/generate-skills.sh:47, tools/generate-skills.sh:74, tools/generate-skills.sh:96, tools/generate-skills.sh:111, tools/generate-skills.sh:121, tools/generate-skills.sh:144 and tools/generate-skills.sh:161.

The existing regression corpus is tests/tools/generate-skills.bats:31 through its comparison-error case at tests/tools/generate-skills.bats:172; every case belongs in the port's coverage.

Non-goals: changing the canonical input or package format, adding harnesses, measuring tokens, performing APM installation inside generation, changing release distribution, or removing the compatibility fallback. Keep the existing generation/token/deployment order (mise.toml:466). Fallback retirement is separately tracked under D9 after N-2 evidence permits it.

Other tooling boundaries stay with their existing owners: the already-Go version check at tools/check-versions.sh:4 needs no second port; draft-prose checking at tools/lint-draft.sh:94 belongs to #717; guard-regression behavior at tools/test-guards.sh:91 belongs to #328; and the existing forwarding adapters at tools/fuzz.sh:4, tools/go-ldflags.sh:4 and tools/lint-types.sh:4 follow #330's payload work. None is part of this generation PR.

Approach

Write failing contract and failure-path checks first, then move the generation behavior to Go. Compare the Go and frozen shell commands against the same isolated inputs. Preserve the existing task interfaces and regenerate only affected outputs through the supported generation sequence. Implementation structure remains the pull request's choice.

Dependencies

None blocking. The probe-and-fallback wrapper (AC7 to AC9) lets this split land without waiting for #542, the #333 → #544 → #382 → #545 → #546 chain or the #359 delivery chain. Removing the fallback later waits for #333, #544 and the N-2 window, and is filed as a follow-up. Skill generation is repository build tooling, not merge, commit or pull request enforcement, so it does not wait for #753. #332 tracks the tooling split; #588 tracks the migration.

Size

S. One pull request for generation, its compatibility entry points and behavioral coverage.

Verify plan

  • AC5: first demonstrate failure for discovery or comparison that returns plausible output and then fails; cover the hash, discovery, parent-lookup and comparison regressions at tests/tools/generate-skills.bats:134, tests/tools/generate-skills.bats:151, tests/tools/generate-skills.bats:164 and tests/tools/generate-skills.bats:172. Assert nonzero status and no clean verdict.
  • AC1: compare complete output trees with the frozen shell for included fragments, a final unterminated line, shared/harness content, conditional helper inclusion and executable modes. Include the existing cases at tests/tools/generate-skills.bats:31, tests/tools/generate-skills.bats:49, tests/tools/generate-skills.bats:55 and tests/tools/generate-skills.bats:114.
  • AC2: seed token records, stale generated files and a hand-maintained package skill; compare resulting trees using tests/tools/generate-skills.bats:64 and tests/tools/generate-skills.bats:103 as the baseline.
  • AC3: absent fragment and colliding-path fixtures compare status, stdout and stderr with tests/tools/generate-skills.bats:120 and tests/tools/generate-skills.bats:127.
  • AC4, AC6: exercise fresh, missing, extra, byte-edited and mode-edited output; compare exact findings and totals, then compare repository files, refs and index before/after check mode (tests/tools/generate-skills.bats:75, tests/tools/generate-skills.bats:85).
  • AC7: absent binary and older binary without the capability must invoke the frozen generator once with identical arguments and results; the read-only capability probe changes nothing.
  • AC8: supported command failure must produce zero shell-fallback calls and exactly one generation attempt; include failure after partial filesystem output and a broken capability probe.
  • AC9: exercise the unchanged tasks and direct wrappers with the N-2 compatibility corpus on the new, older and absent-binary paths; compare streams, status and outputs. Only confirmed capability absence permits fallback; an indeterminate probe fails.
  • AC10: run the same Go corpus on Linux, macOS and Windows, plus shell-wrapper smoke coverage where Bash is supported. Run the focused Go and Bats suites, generation/deployment consistency checks, mise run lint and mise run test on the reviewed head.
  • 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

Mode differences or an omitted helper can produce deployable-looking but broken packages. Re-running shell after a Go failure could overwrite failure evidence. The tree comparisons and zero-repeat checks cover these boundaries. Undo: revert this port; retain the frozen fallback throughout N-2 support.

Operator authority

None.

Activity

  1. added
    change:featPR adds or changes a feature (Conventional Commit type feat)
    on Oct 26, 2026
  2. tbhb-agent commented on Oct 26, 2026

    @tbhb-agent
    ContributorAuthor

    Refinement: NOT READY — Dependencies misclassify the #333/#359/#542 release chain as blockers despite the probe-and-fallback wrapper, and #753 conflicts with Operator authority
    Model: gemini-4-argon-high

    Checked

    • Live issue tbhb-dev/devtools#807 (0 comments) and referenced issues #332, #588, #542, #333, #544, #382, #545, #546, #359, #753, #717, #328, and #330.
    • Problem statement and baseline commit a0347a715fab580ece45edfcb3d13509620459a8 on origin/main:
      • tools/generate-skills.sh:74 (render manifest include expansion), tools/generate-skills.sh:92 (build output assembly), tools/generate-skills.sh:123 (listing mode and digest enumeration), and tools/generate-skills.sh:134 (skill loop for check and write modes).
    • Scope and non-goals citations on origin/main:
      • tools/generate-skills.sh:47 (CLI flag parsing and exit 2 usage), tools/generate-skills.sh:74 (include and unterminated final-line loop), tools/generate-skills.sh:96 (shared and <harness> tree copy and collision check), tools/generate-skills.sh:111 (conditional lib.sh, gates.py, agent.sh, signing.sh, and worktree-ready.sh copy), tools/generate-skills.sh:121 (tokens.json exclusion in listing), tools/generate-skills.sh:144 (--check comparison, not-generated and generated-drift reporting, and TOTAL summary), and tools/generate-skills.sh:161 (stale-file removal preserving tokens.json).
      • mise.toml:462 (generate-skills), mise.toml:466 (regen-generated order: generate-skills, skill-tokens, apm install), and mise.toml:470 (check-generated-skills).
      • Adjacent tooling boundaries: tools/check-versions.sh:4 (already ported to agentrepo check-versions), tools/lint-draft.sh:94 (#717), tools/test-guards.sh:91 (#328), and tools/fuzz.sh:4, tools/go-ldflags.sh:4, and tools/lint-types.sh:4 (#330).
    • Verify plan test citations on origin/main:
      • tests/tools/generate-skills.bats:31, tests/tools/generate-skills.bats:49, tests/tools/generate-skills.bats:55, tests/tools/generate-skills.bats:64, tests/tools/generate-skills.bats:75, tests/tools/generate-skills.bats:85, tests/tools/generate-skills.bats:103, tests/tools/generate-skills.bats:114, tests/tools/generate-skills.bats:120, tests/tools/generate-skills.bats:127, tests/tools/generate-skills.bats:134, tests/tools/generate-skills.bats:151, tests/tools/generate-skills.bats:164, and tests/tools/generate-skills.bats:172.
    • Protected path rules in .agents/agent-skills.conf:4-9 and N-2 compatibility rules in docs/compatibility.md:3-6.

    Gaps

    1. [Major] Dependencies falsely blocks this issue on #542, #333 → #544 → #382 → #545 → #546, and #359 despite the probe-and-fallback wrapper.

      • Evidence: Dependencies states Depends on #542, after the #333 → #544 → #382 → #545 → #546 prerequisites and the #359 delivery chain. This sequencing leaves release distribution and obsolete-release cleanup with their existing owners. However, AC7, AC8, and AC9 keep tools/generate-skills.sh as a side-effect-free probe-and-fallback wrapper that invokes the frozen shell generator whenever the Go subcommand is absent (matching the pattern in packages/agents/source/skills/rebase/shared/scripts/marker-scan.sh:6-14), and tools/generate-skills.sh is only invoked by repository tasks at mise.toml:464 and mise.toml:472. Because the wrapper falls back cleanly on older or absent binaries, this port does not have to wait for #333 and #544 (only retiring the fallback under D9 after the N-2 window waits for them). Moreover, #382 (whose live issue body is currently null), #545, #546, #359, and #542 govern release workflows and the release scripts in #332 (tools/release-verify.sh, tools/release-readiness.sh, tools/release-dispatch.sh), which this split leaves out of scope.
      • Actionable fix: Remove Depends on #542, after the #333 → #544 → #382 → #545 → #546 prerequisites and the #359 delivery chain from ## Dependencies. State that the probe-and-fallback wrapper in AC7–AC9 lets this split land without waiting for #333 and #544, while fallback removal (tracked as a D9 follow-up) waits for #333, #544, and the N-2 window (docs/compatibility.md:3).
    2. [Major] Dependencies on #753 conflicts with Operator authority: None.

      • Evidence: Dependencies states Depends on #753's protection of replacement enforcement before caller rollout, while Operator authority states None. On origin/main, tools/generate-skills.sh is not an enforcement path in .agents/agent-skills.conf:4-9, and #753 (Scope and non-goals and Dependencies) scopes its blocker requirement to enforcement ports (merge policy, gates, signing, review stamps, confirmation, delegation) while also adding internal/agentrepo/command.go (the file that registers agentrepo subcommands, at internal/agentrepo/command.go:85) to golden_paths. If #807 is sequenced after #753 and registers a subcommand on agentrepo, #753 AC1 classifies that registration change as a golden-file edit requiring merge_owner exact-head approval at merge, contradicting Operator authority: None.
      • Actionable fix: Either drop #753 as a blocker in ## Dependencies (since skill generation is repository build tooling rather than merge/commit/PR enforcement, matching #753's exclusion of non-enforcement ports and a0347a715fab580ece45edfcb3d13509620459a8), or if #807 remains sequenced after #753 and edits agentrepo subcommand registration (internal/agentrepo/command.go:85), update ## Operator authority to require merge_owner exact-head approval for the protected subcommand registration file.

    Split

    None. tools/generate-skills.sh is 172 lines (tools/generate-skills.sh:1-172) with one existing Bats suite (tests/tools/generate-skills.bats:1-179); porting generation to Go behind a probe-and-fallback wrapper with Go and Bats tests fits one S pull request as already split from #332 and #588.

    Definition of done

    The definition of done for #807 (Feature). Tick each item and give its evidence. (Ticked boxes below indicate planning coverage in the issue's acceptance criteria and verify plan, not completed implementation 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. — Planning coverage: ## Verify plan maps AC1 through AC10 to concrete verification checks and states under 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 reviewer approved the exact merged head, and the author resolved each finding or filed it as a follow-up. — Planning coverage: ## Verify plan (Definition of done) requires D2 an independent review approves the exact merged head, and every finding has a recorded disposition.
    • D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact head. — Planning coverage: ## Verify plan (AC10 and Definition of done) requires running mise run lint and mise run test on the reviewed head and D3 required checks pass with no gate loosened.
    • 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. — Planning coverage: ## Verify plan (Definition of done) requires 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 The author signed each commit, the squash included. — Planning coverage: ## Verify plan (Definition of done) requires D5 every commit is signed.
    • D6 The docs match the change (README, docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why. — Planning coverage: ## Verify plan (Definition of done) requires D6 the documentation matches, or the description says No doc impact with the reason.
    • D7 The commits and the squash have Assisted-by trailers, and the pull request has the provenance:agent-assisted label. — Planning coverage: ## Verify plan (Definition of done) requires D7 every commit carries an Assisted-by trailer and the pull request carries the provenance:agent-assisted label.
    • D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit. — Planning coverage: ## Size scopes the work to S. One pull request and ## Verify plan (Definition of done) requires D8 the change stays within the pull request size limits.
    • 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. — Planning coverage: ## Scope and non-goals tracks fallback retirement under D9 after N-2 evidence permits it, and ## Verify plan (Definition of done) requires 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 the issue through a closing reference and the board shows Done. The final Status: comment names the merge commit. — Planning coverage: ## Verify plan (Definition of done) requires 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.

    Round: 1 of 3

  3. tbhb-agent commented on Oct 26, 2026

    @tbhb-agent
    ContributorAuthor

    Refinement: READY — Skill generation Go port criteria, probe-and-fallback compatibility, non-blocking dependencies, and verification plan are complete
    Model: gemini-4-argon-high

    Checked

    • Live issue tbhb-dev/devtools#807 (1 comment from Round 1 refinement) and all referenced issues: #332, #588, #717, #328, #330, #542, #333, #544, #382, #545, #546, #359, and #753.
    • Problem statement and baseline commit a0347a715fab580ece45edfcb3d13509620459a8 on origin/main:
      • tools/generate-skills.sh:74 (render manifest @include expansion), tools/generate-skills.sh:92 (build output assembly), tools/generate-skills.sh:123 (listing mode and digest enumeration), and tools/generate-skills.sh:134 (skill loop for check and write modes).
    • Acceptance criteria (AC1–AC10):
      • Each criterion is observable, testable, single-outcome, and free of prescribed implementation detail, with non-functional coverage for read-only --check invariance (AC6), N-2 backward compatibility (AC9, matching docs/compatibility.md:3-6), and cross-platform parity across Linux, macOS, and Windows (AC10).
    • Scope and non-goals citations on origin/main:
      • tools/generate-skills.sh:47 (CLI flag parsing and exit 2 usage), tools/generate-skills.sh:74 (include and unterminated final-line loop), tools/generate-skills.sh:96 (shared and <harness> tree copy and collision check), tools/generate-skills.sh:111 (conditional lib.sh, gates.py, agent.sh, signing.sh, and worktree-ready.sh copy), tools/generate-skills.sh:121 (tokens.json exclusion in listing), tools/generate-skills.sh:144 (--check comparison, not-generated and generated-drift reporting, and TOTAL summary), and tools/generate-skills.sh:161 (stale-file removal preserving tokens.json).
      • mise.toml:462 (generate-skills), mise.toml:466 (regen-generated order: generate-skills, skill-tokens, apm install), and mise.toml:470 (check-generated-skills).
      • Adjacent tooling boundaries: tools/check-versions.sh:4 (already calls agentrepo check-versions), tools/lint-draft.sh:94 (#717), tools/test-guards.sh:91 (#328), and tools/fuzz.sh:4, tools/go-ldflags.sh:4, and tools/lint-types.sh:4 (#330).
    • Dependencies and Operator authority:
      • Both Round 1 gaps are resolved in the live body: ## Dependencies now records None blocking, explaining that the probe-and-fallback wrapper (AC7–AC9) lets this split land without waiting for #542, #333 → #544 → #382 → #545 → #546, or #359 (with fallback removal deferred to a D9 follow-up after #333, #544, and the N-2 window), and that skill generation is repository build tooling rather than merge, commit, or PR enforcement so it does not wait for #753.
      • ## Operator authority (None.) agrees with .agents/agent-skills.conf:4-9: tools/generate-skills.sh, tests/tools/generate-skills.bats, and Go packages on origin/main are outside golden_paths, constitutional_paths, and owner_paths, while preserving existing entry points and byte-identical generated skill outputs avoids touching gate_config_paths or protected generated skill scripts.
    • Verify plan citations on origin/main:
      • tests/tools/generate-skills.bats:31, tests/tools/generate-skills.bats:49, tests/tools/generate-skills.bats:55, tests/tools/generate-skills.bats:64, tests/tools/generate-skills.bats:75, tests/tools/generate-skills.bats:85, tests/tools/generate-skills.bats:103, tests/tools/generate-skills.bats:114, tests/tools/generate-skills.bats:120, tests/tools/generate-skills.bats:127, tests/tools/generate-skills.bats:134, tests/tools/generate-skills.bats:151, tests/tools/generate-skills.bats:164, and tests/tools/generate-skills.bats:172.

    Gaps

    None

    Split

    None. tools/generate-skills.sh is 172 lines (tools/generate-skills.sh:1-172) with one existing Bats suite (tests/tools/generate-skills.bats:1-179); porting generation to Go behind a probe-and-fallback wrapper with Go and Bats tests fits one S pull request as already split from #332 and #588.

    Definition of done

    The definition of done for #807 (Feature). Tick each item and give its evidence. (Ticked boxes below indicate planning coverage in the issue's acceptance criteria and verify plan, 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. — Planning coverage: ## Verify plan maps AC1 through AC10 to concrete verification checks and states under 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 reviewer approved the exact merged head, and the author resolved each finding or filed it as a follow-up. — Planning coverage: ## Verify plan (Definition of done) requires D2 an independent review approves the exact merged head, and every finding has a recorded disposition.
    • D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact head. — Planning coverage: ## Verify plan (AC10 and Definition of done) requires running mise run lint and mise run test on the reviewed head and D3 required checks pass with no gate loosened.
    • 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. — Planning coverage: ## Verify plan (Definition of done) requires 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 The author signed each commit, the squash included. — Planning coverage: ## Verify plan (Definition of done) requires D5 every commit is signed.
    • D6 The docs match the change (README, docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why. — Planning coverage: ## Verify plan (Definition of done) requires D6 the documentation matches, or the description says No doc impact with the reason.
    • D7 The commits and the squash have Assisted-by trailers, and the pull request has the provenance:agent-assisted label. — Planning coverage: ## Verify plan (Definition of done) requires D7 every commit carries an Assisted-by trailer and the pull request carries the provenance:agent-assisted label.
    • D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit. — Planning coverage: ## Size scopes the work to S. One pull request and ## Verify plan (Definition of done) requires D8 the change stays within the pull request size limits.
    • 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. — Planning coverage: ## Scope and non-goals and ## Dependencies track fallback retirement under D9 after #333, #544, and the N-2 window, and ## Verify plan (Definition of done) requires 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 the issue through a closing reference and the board shows Done. The final Status: comment names the merge commit. — Planning coverage: ## Verify plan (Definition of done) requires 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.

    Round: 1 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

    change:featPR adds or changes a feature (Conventional Commit type feat)refinement:readyPassed the Definition of Ready; may be picked up

    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