chore: harden hook scripts, add Windows/shellcheck/actionlint CI gates - #108
Merged
Conversation
Hooks (fail-open contract made uniform): - safety-check.sh, audit-trail.sh, slop-detect.sh: add set -euo pipefail with guarded jq extractions, matching rtk-rewrite.sh convention - post-validate.sh, subagent-stop.sh: guard previously-unguarded jq calls so malformed stdin degrades cleanly instead of killing the hook - slop-detect.sh: fix latent '|| echo 0' double-output bug (grep -c already prints 0 on no-match) that broke integer comparisons - audit-trail.sh: tolerate log-write failures (read-only fs) silently - stop-gate.sh: remove unused HAS_CONFIG (SC2034) CI: - new windows-latest job: build+test src engine and mcpb/launcher on PR (previously Windows breakage only surfaced at release time) - new lint-shell job: shellcheck hooks/*.sh + wrappers - new lint-actions job: actionlint via the author's docker image - new .github/dependabot.yml: weekly gomod (src, mcpb/launcher) + github-actions update groups Go (swallowed errors): - engine.go: surface GetSteps failure from final-report path unless a step error already takes precedence - status.go: cost-query failure renders 'unknown' like showAllSessions - workflow.go: check agent/budget flag lookup errors Verified: go build/vet clean, 441 tests pass, shellcheck clean, hooks_test.sh 75/75.
The first run of build-and-test-windows surfaced two pre-existing issues the gate exists to catch: - mcp tests leaked the server's open devkit.db handle; Windows cannot delete a TempDir containing an open file. setupTestServer and TestNewServer now t.Cleanup(srv.Close) (LIFO — runs before TempDir removal). - TestDBDirectoryPermissions and TestSessionFileMode assert Unix permission bits, which os.Stat does not report on Windows — skip there with rationale. actionlint flagged two pre-existing SC2235 subshell-style nits in release.yml's version-compare condition — converted to brace groups, semantically identical.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hardening pass from the hook/CI review, plus mechanical riders. No behavior changes intended for the happy path — every change is either a robustness guard, a new CI gate, or surfacing a previously swallowed error.
Hooks — uniform fail-open contract
safety-check.sh,audit-trail.sh,slop-detect.shgainset -euo pipefailwith guardedjqextractions (the conventionrtk-rewrite.sh/security-patterns.shalready document).post-validate.sh,subagent-stop.shhad the inverse bug: strict mode but unguardedjq— malformed stdin killed the hook instead of failing open. Now guarded.slop-detect.sh: fixed latent|| echo 0bug —grep -calready prints0on no-match before exiting 1, so the guard produced0\n0and broke the integer comparisons.audit-trail.sh: log-write failures (read-only fs, permissions) no longer error every Bash call.stop-gate.sh: removed unusedHAS_CONFIG(SC2034) so the new shellcheck gate starts green.Deliberately NOT changed: the
*matcher ondevkit-guard.shinhooks.json. guard.go is default-deny under hard enforcement — narrowing the matcher would let unlisted tools (WebFetch, NotebookEdit, MCP tools) bypass workflow fencing.CI
build-and-test-windows: builds + testssrc/andmcpb/launchernatively onwindows-latestper PR. Previously nothing validated Windows compiles before the release pipeline.lint-shell: shellcheck overhooks/*.sh+ wrappers (fresh-install job only covered the wrappers).lint-actions: actionlint viarhysd/actionlintdocker image (tool author's image, no third-party wrapper action)..github/dependabot.yml: weekly grouped updates for bothgo.mods and github-actions.Go — swallowed errors
engine.go:GetStepsfailure on the final-report path now surfaces (unless a step error already takes precedence).status.go: cost-query failure rendersunknown, matchingshowAllSessionsstyle.workflow.go:agent/budgetflag lookup errors are checked (turns a future flag rename into a loud failure instead of a silent zero-value).Verification
go build ./...+go vet ./...clean; 441 tests pass in 6 packagesshellcheck -S warning hooks/*.sh bin/devkit bin/mcpb-build mcpb/server/devkitcleanhooks/hooks_test.sh: 75/75 pass