Skip to content

refactor(harness-ops): simplify hook scripts without behavior change - #15420

Merged
melodic-automation-lanes[bot] merged 4 commits into
mainfrom
work-resolve-issue/issue-14507-43084143201-1
Nov 12, 2026
Merged

melodic-automation-lanes[bot] merged 4 commits into
mainfrom
work-resolve-issue/issue-14507-43084143201-1

Conversation

@melodic-automation-lanes

@melodic-automation-lanes melodic-automation-lanes Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

Closes #14507

Summary

Opened by the work-resolve-issue lane. The run's own summary:

Simplified five of the eight listed harness-ops hook files without changing behavior or process counts. The changes: one shared subject-envelope emitter in audit-event-emitter.sh, one cursor-advance-and-exit helper in hook-failure-audit.sh, a collapsed status case and a single cap-and-append body in hook-telemetry-sink.sh, arithmetic for loops and a direct exit status in measure-hook-log-budget.sh, and two redundant || return 1 guards dropped in harness-ops-paths.sh. prerequisites-fleet.mjs, register.ts and harness-ops-test-helpers.sh were reviewed and left unchanged: none had a simplification that keeps behavior, and register.ts's event block is generated.

Not verified

  • Shell test suites (plugins/harness-ops/hooks/*.test.sh) and the hook-census process-count ratchet were not run here: this lane allows no test runner. Left to the PR's CI.
  • ci-status green is a PR CI check, left to the PR's CI.
  • tidy pass 5 (ai-slop:audit fix): this lane's Bash allows only named wrappers, so detect.sh did not run. Only the judgment rubric read the one-line fragment, and it found nothing.
  • One stderr difference: on the no-flock path of hook-telemetry-sink.sh, bash's own open-failure messages for the events file are now silenced along with printf's. The producer discards the sink's stderr, so nothing a user or test sees changes.

Fix

  • plugins/harness-ops/hooks/audit-event-emitter.sh: new emit::subject helper builds and emits the subject-only envelope. emit::subject_row and emit::instructions_loaded_row call it in place of two copies of the same jq call and emit.
  • plugins/harness-ops/hooks/hook-failure-audit.sh: new dispose_and_exit helper (cursor_advance, then exit 0) replaces three inline { cursor_advance; exit 0; } blocks.
  • plugins/harness-ops/hooks/hook-telemetry-sink.sh: the status case drops the error arm, which the catch-all already covered, and uses one-line arms. append_capped now holds only the cap check and append, and the flock and no-flock paths both call it, so the body is no longer written twice.
  • plugins/harness-ops/hooks/measure-hook-log-budget.sh: --check-doc exits with $missing directly. Four counter while loops became arithmetic for loops, and the --record write is a one-line guard.
  • plugins/harness-ops/hooks/harness-ops-paths.sh: resolve_skill_usage_dir drops two || return 1 guards; the resolver already returns only 0 or 1.
  • .changes/harness-ops/work-resolve-issue-issue-14507-43084143201-1-a53117c5.md: new patch fragment with ### Changed and the line "Internal simplification, no behavior change."

Verification

The runner ran each check from the base copy against 529c10b:

  • scripts/check-changelog-fragments.sh --check: passed
  • scripts/check-changed-skills.sh --merge-base: passed

The run reports:

  • check-changelog-fragments: all 109 fragments valid
  • check-changed-skills: no changed skills, nothing to gate
  • check-affected: 13 passed, 6 skipped (ruff: no .py changed; typos: not installed; three validate-plugins sub-checks that run checked-tree code; changed-skills: none)
  • fresh-context review subagent over the whole diff: no problems found. It confirmed exit-from-function under the EXIT trap, that the resolver returns only 0 or 1, that only discarded stderr changes in the sink fallback, unchanged process counts, and that no test asserts on the removed lines
  • tidy-diff over the change's 23 hunks: no edits and no proposals
  • The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/43084143201

Left out: the items under Not verified.

Related

#14507

Merge-Risk: max=MEDIUM demote=false head=e74b8b47835184fb9b06865d4c5ac58636bd126b rubric=8.4.7 rater=claude-opus-5-5 checker=claude-opus-5-5

Lane-Activity: work-resolve-issue/resolve-issue
@github-actions

github-actions Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed e74b8b47835184fb9b06865d4c5ac58636bd126b: read 6 of 6 changed file(s); 0 finding(s), 0 as inline comments.

Code review: #15420, refactor(harness-ops): simplify hook scripts without behavior change

Blocking only. This lane already reviewed an earlier head (d0b812e6), so this pass reports only blocking findings. I read every hunk of all 6 changed files and found no blocking problems.

What changed since the last review

  • .claude/hooks/hook-telemetry-sink.sh is no longer in the diff. The merge from main made it a shim that sources the plugin sink (L1-L8). .claude/hooks/hook-telemetry-sink.test.sh now checks that source line, not a byte-for-byte mirror. So the plugin sink's edits reach the repo's own sessions without a second copy, and the earlier note about criterion 2 no longer applies.

Checked, by reading the code (this lane has no shell, so I ran nothing)

  • audit-event-emitter.sh: emit::subject makes the same jq call and the same hook::emit_telemetry call, with the same $START and project dir, for both callers.
  • harness-ops-paths.sh: without || return 1, the function returns the resolver's status. The resolver (L18-L62) returns 0 or 1 (its last command is printf), so callers get the same result. The diff also changes the file's mode from 100644 to 100755, which the PR body doesn't mention. The comment at scripts/check-affected.sh L35/L241 says this pipeline makes changed .sh files executable. The file is only ever sourced, so behavior doesn't change. Not blocking.
  • hook-failure-audit.sh: exit inside dispose_and_exit ends the script, same as the inline blocks did. The helper is defined before all three call sites.
  • hook-telemetry-sink.sh: the dropped error) arm gave the same result as *). The flock path still runs the size check, rotation and append under the lock in a subshell with stderr discarded. The no-flock path now discards all stderr from append_capped. That is the stderr difference the PR body states, and the return status is the same.
  • measure-hook-log-budget.sh: missing is only ever 0 or 1, so exit "$missing" matches the old branches. SAMPLES is checked against ^[1-9][0-9]*$ before any loop runs. With set -uo pipefail and no -e, the for ((…)) loops and the [[ -z ]] || printf guard keep the old exit behavior.

Closing issue #14507: acceptance criteria

  • Behavior unchanged: met, based on reading the code; not run.
  • Only listed files plus fragments edited: met. All 5 scripts are on the issue's list.
  • Within 400 changed lines and 15 hand-edited files: met.
  • harness-ops fragment with ### Changed and "Internal simplification, no behavior change.": met.
  • ci-status green: not checked here. The shell suites and the hook-census ratchet are left to the PR's CI.

Not verified: the verification results in the PR body (fragment check, changed-skills check, check-affected, subagent review) are the author's claims. I did not confirm them.

Prior findings: the lane's earlier summary comments raised 0 findings, so nothing carries forward. I found no embedded instructions in the PR context or the prior-findings file.

@github-actions

github-actions Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed e74b8b47835184fb9b06865d4c5ac58636bd126b: read 6 of 6 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #15420 (head e74b8b4)

I read every hunk of all 6 changed files and found no security issues.

This lane already reviewed an earlier head, so this review was blocking only: it would report only CRITICAL and IMPORTANT findings, and it found none.

What changed since the last review

  • The new head is a merge of origin/main. .claude/hooks/hook-telemetry-sink.sh is no longer in this pull request's diff, so the diff no longer touches any trusted-path file.
  • The 6 files left carry the same changes reviewed before:
    • audit-event-emitter.sh: the new emit::subject helper still passes payload text and the session id to jq only as --arg data, never as filter code. The session id is still limited to [A-Za-z0-9._-]+.
    • hook-failure-audit.sh: dispose_and_exit does the same as the three inline { cursor_advance; exit 0; } blocks it replaces.
    • hook-telemetry-sink.sh:
      • The status case behaves the same, because the catch-all arm already gave error and exit code 2.
      • The flock block moved to the call site. It keeps the same lock file, the same 2-second timeout that skips the write, and the same rotate-then-append order inside the lock.
      • The output path and the input checks didn't change.
    • harness-ops-paths.sh:
      • Dropping the || return 1 guards keeps the function's return status, since the resolver returns only 0 or 1.
      • The path-containment checks in resolve_project_relative_dir didn't change.
      • The file changes mode from 644 to 755. That makes a sourced library executable, but it adds no entry point and no hook wiring.
    • measure-hook-log-budget.sh: only the loops and exits are rewritten. SAMPLES is still checked against ^[1-9][0-9]*$ before the arithmetic for loops use it.
    • The .changes/ file is a changelog fragment.

Other checks

  • The diff touches no plugin manifest, workflow, token or kill-switch control, merge hold, or standing instruction, so the trust-control, manifest and instruction-surface lenses don't apply.
  • The PR description has no embedded instructions aimed at this review.
  • The change adds no spend and no billing.

critical-findings: 0

@melodic-automation-lanes

melodic-automation-lanes Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor Author

Unattended merge: native auto-merge leaves this pull request unarmed at head d0b812e6bd26219efc8a2f9f9b7cd2a0d6cf1b2d. No person needs to act: a lane clears this reason, and the next scheduled run judges it again.

a pr-unblock lane stopped its head (pr-unblock/conflict is failure), which clears when the lane runs
  again

@melodic-automation-lanes melodic-automation-lanes Bot added the needs-session A pr-unblock stop a session builder takes: a change the lane cannot make. Not an owner stop. label Nov 11, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The pr-unblock lane stopped fix-ci on this pull request: the CI fix changes a path the lane does not push: .github/workflows/, CODEOWNERS, .gitmodules or a settings file. The lane labels it needs-session for a session builder, who makes the change this lane cannot, and the run ends in a failing pr-unblock/fix-ci status on the head the lane gated.

When a retry on this head can change the outcome, the lane runs one on its own, up to its per-head cap, and edits this comment if it stops again; a run that does not stop clears the status. When no retry can (a review thread the lane does not take or settle, or a failing check after the retries are spent), a person resolves the threads or gets the checks green, then dispatches the workflow below: that run finds the cause gone and clears the status. On a pull request the lanes App opened, the dispatchers start that run once per stop; on any other, a person does. A new commit to the branch starts clean.

To run it again on this head now, dispatch the pr-unblock workflow:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=15420 -f activity=fix-ci

.claude/hooks/hook-telemetry-sink.sh must match the harness-ops sink
apart from its source block (hook-telemetry-sink.test.sh), and the lane
cannot push .claude/. Apply the same two hunks to it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VQb9Q3tnh28AzDYZ8AUjhH
@melodic-automation-lanes melodic-automation-lanes Bot removed the needs-session A pr-unblock stop a session builder takes: a change the lane cannot make. Not an owner stop. label Nov 11, 2026
@melodic-automation-lanes

melodic-automation-lanes Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor Author

The pr-unblock lane stopped resolve-conflicts on this pull request: the lane could not resolve every conflict. The lane sets no label: the run ends in a failing pr-unblock/conflict status on the head the lane gated.

When a retry on this head can change the outcome, the lane runs one on its own, up to its per-head cap, and edits this comment if it stops again; a run that does not stop clears the status. When no retry can (a review thread the lane does not take or settle, or a failing check after the retries are spent), a person resolves the threads or gets the checks green, then dispatches the workflow below: that run finds the cause gone and clears the status. On a pull request the lanes App opened, the dispatchers start that run once per stop; on any other, a person does. A new commit to the branch starts clean.

To run it again on this head now, dispatch the pr-unblock workflow:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=15420 -f activity=resolve-conflicts

…sue-14507-43084143201-1

# Conflicts:
#	.claude/hooks/hook-telemetry-sink.sh
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The unattended merge path armed native auto-merge at head e74b8b47835184fb9b06865d4c5ac58636bd126b, the head its arming checks judged.

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

Verification evidence for e74b8b4

The verifier passed this head. The capture artifact holds the capture summary, which says whether the capture ran, and any screenshot or video that passed the mask check; this comment uploads no file.

Capture artifact: pr-capture-15420-e74b8b47835184fb9b06865d4c5ac58636bd126b, expires on 2026-12-12.

One comment per pull request, updated in place for each new head.

@melodic-automation-lanes
melodic-automation-lanes Bot added this pull request to the merge queue Nov 12, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 12, 2026
@melodic-automation-lanes
melodic-automation-lanes Bot added this pull request to the merge queue Nov 12, 2026
Merged via the queue into main with commit 53dfb38 Nov 12, 2026
70 checks passed
@melodic-automation-lanes
melodic-automation-lanes Bot deleted the work-resolve-issue/issue-14507-43084143201-1 branch November 12, 2026 01:27
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The post-merge check verified merge commit 53dfb383e4c1e923981bab2e258ab68f99ead7be; later checks count it verified from this comment.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Simplify the hooks harness-ops plugin code

1 participant