Skip to content

Repository navigation

feat(apm): move hook reconciliation to Go #800

New issue
New issue
Open
Feature
Open
feat(apm): move hook reconciliation to Go#800
Feature
Labels
change:featPR adds or changes a feature (Conventional Commit type feat)refinement:readyPassed the Definition of Ready; may be picked up

Description

@tbhb-agent
tbhb-agent
opened on Oct 26, 2026

Problem

Part of #588. Split APM hook reconciliation out of #330 as one reviewable PR. The repository task and published consumer helper currently repeat the same cleanup before APM installation; moving this behavior to Go must preserve settings outside that cleanup.

Evidence on origin/main at a0347a715fab580ece45edfcb3d13509620459a8: the published helper is .devtools/scripts/apm-sync.sh:1, the consumer task and subsequent audit are .devtools/tasks/apm.toml:2, and the repository task and post-audit are mise.toml:827. Behavioral evidence is tests/tasks/adoption.bats:75, tests/tasks/adoption.bats:110, and tests/tools/masked-status.bats:28.

Acceptance criteria

  1. AC1: Reconciliation removes only the existing cleanup keys: hooks from Claude settings, apm from agent hooks, and hooks from Codex hooks, preserving all sibling settings.
  2. AC2: A settings document left empty by cleanup is removed, while an absent settings document remains absent until installation needs it.
  3. AC3: The three legacy APM hook sidecars are absent before installation starts.
  4. AC4: Each existing APM sync task performs installation once and audits a successful deployment through its existing audit interface.
  5. AC5: A failed settings operation, installation, or audit fails the invoking task without running later stages or concealing the failure status.
  6. AC6: Non-functional: repeated reconciliation with unchanged inputs produces the same effective settings and deployment outcome as the frozen shell.
  7. AC7: Every existing consumer-facing sync entry point ends as a thin shell wrapper that probes the Go subcommand and falls back to the frozen shell when that capability is unavailable throughout N-2 compatibility.
  8. AC8: Non-functional: supported fixtures preserve status, separate output streams, and final files on Linux, macOS, and Windows.

Scope and non-goals

Include both repository and consumer task behavior, existing helper invocation, all three harness documents and sidecars, unrelated settings, empty documents, absent documents, APM install, and each task's existing audit invocation. The exact current cleanup boundaries are .devtools/scripts/apm-sync.sh:3, .devtools/scripts/apm-sync.sh:9, .devtools/scripts/apm-sync.sh:15, and .devtools/scripts/apm-sync.sh:21. Cover the real local-package adoption case in tests/tasks/adoption.bats:110 without fetching an external package.

Non-goals: changing which hook keys are cleared, introducing new hook ownership rules, replacing APM or the Python audit helper, changing lockfile/deployment policy, rewriting preapproval, or making cleanup/install/audit one rollback transaction. Existing later-stage failures may leave earlier cleanup applied; do not promise restoration the shell does not perform. The unrelated prose adoption and helper-copy assertions in tests/tasks/adoption.bats:37 and tests/tasks/adoption.bats:104 remain unchanged.

Approach

Establish failing reconciliation tests before switching either entry point to Go. Compare both current entry points and the replacement with identical JSON and controlled install/audit outcomes. Preserve the existing consumer task and helper interface. Required thin wrappers probe before touching settings, respect the selected tool version, retain only the frozen N-2 fallback, and do not rerun a failed Go reconciliation through shell.

Dependencies

Size

S. One PR for the duplicated reconciliation behavior, existing task wiring, parity coverage, and compatible dispatch.

Verify plan

Write and observe the named replacement tests fail before changing dispatch or removing shell behavior. Use frozen-shell comparisons as the parity oracle; no live protected action is needed.

  • AC1, AC2: Failing tests first for each harness document with owned keys plus siblings, owned keys only, no owned key, an empty document, and a missing file. Compare semantic preservation and observable file results with the frozen implementation; port tests/tasks/adoption.bats:75.
  • AC3, AC4: Failing tests first with all legacy sidecars and a recorded install/audit trace; assert cleanup precedes exactly one install and each existing task retains its audit. Retain the local-package deploy/audit adoption case from tests/tasks/adoption.bats:110.
  • AC5: Failing tests first for malformed/unreadable settings and failures during cleanup, install, and audit. Preserve the failed-inspection regression in tests/tools/masked-status.bats:28, including plausible partial output, no install after inspection failure, and retained earlier side effects rather than invented rollback.
  • AC6, AC8: Compare frozen shell and replacement status, stdout, stderr, and final files for both entry points, then repeat successful sync to detect accumulating obsolete hooks. Run Go behavior tests on Linux, macOS, and Windows, with shell wiring tests where Bash is available.
  • AC7: Failing wrapper tests first for present, missing, old, and unsupported binaries, consumer-selected version precedence, and no fallback after mutation starts. Run targeted adoption tests, mise run lint and mise run test, and relevant payload checks; retire only migrated Bats assertions.

Definition of done: D1–D10 apply.

Risks

Cleanup currently removes whole named keys; narrowing or broadening that behavior during the port would change hook ownership. A failed install can leave settings cleared. The replacement must preserve unrelated permissions and local hook settings and cannot use a failed Go run as grounds to execute the mutating shell again.

Operator authority

host or harness config change: The migrated command edits the existing harness hook settings covered by .devtools/scripts/apm-sync.sh:3 and mise.toml:833. Operational use retains the existing authorization for APM reconciliation; changing protected repository harness settings requires the existing owner review process. This issue authorizes no expansion of cleanup scope or configuration policy.

Activity

  1. tbhb-agent
    added
    change:featPR adds or changes a feature (Conventional Commit type feat)
    on Oct 26, 2026
  2. github-project-automation
    added this to devtoolson Oct 26, 2026
  3. github-project-automation
    moved this to Backlog in devtoolson Oct 26, 2026
  4. tbhb-agent
    added the
    Feature
    issue type on Oct 26, 2026
  5. tbhb-agent
    added a parent issue on Oct 26, 2026

  6. tbhb-agent commented on Oct 26, 2026

    @tbhb-agent
    tbhb-agent
    on Oct 26, 2026
    ContributorAuthor

    Refinement: READY — #800 clearly specifies requirements, parity and N-2 fallback criteria, verification, and authority for moving APM hook reconciliation to Go
    Model: gemini-4-argon-high

    Checked

    • Live issue tbhb-dev/devtools#800 (title, body, labels, parent #330, and 0 comments) and referenced issues #330, #588, and #333 against origin/main at a0347a715fab580ece45edfcb3d13509620459a8.
    • Problem and baseline citations:
      • .devtools/scripts/apm-sync.sh:1 (22 lines) is the published consumer helper that deletes .hooks from .claude/settings.json (.devtools/scripts/apm-sync.sh:3), .apm from .agents/hooks.json (.devtools/scripts/apm-sync.sh:9), and .hooks from .codex/hooks.json (.devtools/scripts/apm-sync.sh:15), removes empty settings files, deletes .claude/apm-hooks.json, .agents/apm-hooks.json, and .codex/apm-hooks.json (.devtools/scripts/apm-sync.sh:21), and runs apm install (.devtools/scripts/apm-sync.sh:22).
      • .devtools/tasks/apm.toml:2 defines ["devtools:apm-sync"], running bash "$DEVTOOLS_DIR/scripts/apm-sync.sh" (.devtools/tasks/apm.toml:5) followed by uv run --script "$DEVTOOLS_DIR/scripts/apm_audit.py" (.devtools/tasks/apm.toml:6).
      • mise.toml:827 defines [tasks.apm-sync] with depends_post = ["lint-apm"] (mise.toml:829) and duplicates the same cleanup and apm install sequence (mise.toml:833 through mise.toml:852).
      • tests/tasks/adoption.bats:75 tests preserving sibling settings and removing obsolete hooks and sidecars during devtools:apm-sync.
      • tests/tasks/adoption.bats:110 tests real local-package APM deploy and audit in a clean consumer, while tests/tasks/adoption.bats:37 and tests/tasks/adoption.bats:104 cover unrelated prose adoption and helper-copy checks named as non-goals.
      • tests/tools/masked-status.bats:28 tests that jq inspection failures exit with status 23, preserve existing settings files, and prevent apm install in both .devtools/scripts/apm-sync.sh and mise.toml apm-sync.
    • Acceptance criteria (AC1–AC8): Each criterion states one observable, testable outcome free of implementation detail (no prescribed internal Go package layout, file structure, or library choice), including non-functional criteria for idempotence/parity (AC6), N-2 thin-wrapper fallback (AC7, matching docs/compatibility.md:3), and cross-platform stream/status/file parity on Linux, macOS, and Windows (AC8).
    • Scope, non-goals, and size: Scope and non-goals accurately bound the two entry points, three harness settings files, three legacy sidecars, apm install, existing audit wiring, and failure semantics without inventing rollback transactions. Size S fits one PR (21 source lines in .devtools/scripts/apm-sync.sh:1 and 21 source lines in mise.toml:827, matching docs/script-inventory.md:47 and docs/script-inventory.md:215).
    • Dependencies: #330 (parent task-family port; .devtools/tasks/pins.toml:195 calls mise run devtools:apm-sync), #588 (shell-to-Go migration epic), and #333 (release binary assets, mitigated for N-2 consumers by AC7 fallback) are real, open issues with accurately classified relationships.
    • Verify plan: Maps every AC (AC1–AC8) to concrete failing-first tests, parity oracle comparisons against the frozen shell, and existing regression cases (tests/tasks/adoption.bats:75, tests/tasks/adoption.bats:110, tests/tools/masked-status.bats:28).
    • Risks and operator authority: Identifies hook-key ownership, partial-cleanup failure states, and no shell re-execution after a failed Go run. Operator authority accurately reflects .agents/agent-skills.conf:4, .agents/agent-skills.conf:8, and .github/CODEOWNERS:6 (.claude/settings.json is in golden_paths/constitutional_paths/owner_paths, and mise.toml and .devtools/tasks/ are in gate_config_paths).

    Gaps

    None

    Split

    None; size S fits one pull request.

    Definition of done

    The definition of done for #800 (Feature). Tick each item and give its evidence. (Ticked boxes below indicate planning coverage in the issue requirements and verification 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–AC8 to concrete failing-first tests, frozen-shell parity checks, Bats ports (tests/tasks/adoption.bats:75, tests/tasks/adoption.bats:110, tests/tools/masked-status.bats:28), mise run lint, and mise run test, and declares Definition of done: D1–D10 apply.
    • 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 declares Definition of done: D1–D10 apply. and ## Operator authority retains repository review requirements.
    • 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 requires mise run lint, mise run test, cross-platform Go behavior tests (AC8), and payload checks (AC7), and ## Operator authority plus Definition of done: D1–D10 apply. require owner review for protected paths (.agents/agent-skills.conf:4, .agents/agent-skills.conf:8).
    • 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 declares Definition of done: D1–D10 apply. and runs repository lint gates (mise run lint).
    • D5 The author signed each commit, the squash included. — Planning coverage: ## Verify plan declares Definition of done: D1–D10 apply.
    • D6 The docs match the change (README, docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why. — Planning coverage: AC7 and ## Approach preserve the documented consumer task and helper interfaces (README.md:164, .devtools/tasks/apm.toml:2, .devtools/scripts/apm-sync.sh:1), and ## Verify plan declares Definition of done: D1–D10 apply.
    • D7 The commits and the squash have Assisted-by trailers, and the pull request has the provenance:agent-assisted label. — Planning coverage: ## Verify plan declares Definition of done: D1–D10 apply.
    • 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 PR covering .devtools/scripts/apm-sync.sh:1, .devtools/tasks/apm.toml:2, mise.toml:827, and parity tests), and ## Verify plan declares Definition of done: D1–D10 apply.
    • 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: ## Dependencies links #330, #588, and #333, and ## Verify plan declares Definition of done: D1–D10 apply.
    • 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: ## Problem and ## Size scope #800 as one reviewable PR closing the issue, and ## Verify plan declares Definition of done: D1–D10 apply.

    Round: 1 of 3

  7. tbhb-agent
    added
    refinement:readyPassed the Definition of Ready; may be picked up
    on Oct 26, 2026
  8. tbhb-agent
    added 6 commits that reference this issue on Oct 26, 2026

    fix(apm): match the shell when cleanup fails

    6331d0e

    test(apm): check the sync command without an installer

    360044b

    feat(apm): move hook reconciliation to Go

    96e2b4c

    fix(apm): match the shell when cleanup fails

    887a0d8

    test(apm): check the sync command without an installer

    9620ebc

    fix(apm): preserve shell status and blank settings behavior

    0592c4b
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

    Type

    Feature

    Fields

    Priority

    None yet

    Work type

    None yet

    Projects

    • devtools
      Status
      Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions