Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,21 @@
# The umbrella also re-covers git-hooks/lib/*.sh already caught by *.sh; harmless.
git-hooks/** text eol=lf

# BUG-039: the other extensionless-but-text groups in the tree, same reasoning
# as git-hooks/** above. `dotf init`'s templates are go:embed'd into the CLI
# binary at build time, so a CRLF checkout here means every repo `dotf init`
# scaffolds inherits a CRLF Makefile/gitignore/claude-md on a Windows build.
# Golden-corpus fixtures under tests/golden/ are compared byte-for-byte against
# script output, so a CRLF checkout would also break the comparison itself, not
# just the shebang class of failure. A regression test (tests/gitattributes-
# eol.bats) asserts every tracked extensionless text file resolves an explicit
# eol, so the next one added outside these groups fails loudly instead of
# silently depending on which OS someone happens to be checking out on.
cli/internal/initrepo/templates/** text eol=lf
tests/golden/** text eol=lf
ssh/config text eol=lf
LICENSE text eol=lf

# Windows shells: native tooling expects CRLF.
*.ps1 text eol=crlf
*.psm1 text eol=crlf
Expand Down
7 changes: 6 additions & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,12 @@ repos:

- id: sdd-spec-gate
name: SDD spec-gate (Tier 4)
entry: ./scripts/check-spec-gate.sh
# BUG-061: routed through the prepush adapter, not check-spec-gate.sh
# directly -- the adapter resolves this branch's live PR (labels/body/
# author) via `gh` when one exists, so archive-on-merge is credited
# locally instead of only after push. Falls through to the gate with no
# PR context (today's behaviour) when there is none yet.
entry: ./scripts/spec-gate-prepush.sh
args: ['--base-ref', 'origin/main', '--head-ref', 'HEAD']
language: script
pass_filenames: false
Expand Down
24 changes: 24 additions & 0 deletions docs/lessons.md
Original file line number Diff line number Diff line change
Expand Up @@ -2235,6 +2235,30 @@ The blast radius was also wider than the one function: because `compile-harness.

**Tags**: `git`, `github`, `workflow`, `worktree`

### [2026-08-12] A "multi-call binary" bug report can name the wrong mechanism — verify the dispatch, not just the symptom

**Context**: BUG-054, fixing `tests/install-dotf.bats`'s busy-binary fixture so the ETXTBSY swap path it claims to exercise is actually reached. The filed issue diagnosed the root cause precisely — `sleep` copied to a file named `dotf` exits immediately instead of sleeping on a multi-call coreutils build — and proposed a fix: `exec -a sleep "$0" 30` to hand the copy the argv[0] the dispatcher expects while the file on disk keeps the name `install_dotf` needs to swap.

**Problem**: the proposed fix does not work on this machine, and the reason is that "multi-call binary" names a shape, not a single mechanism. GNU coreutils' traditional multi-call dispatch reads argv[0], which is exactly what `exec -a` controls — but this system's `sleep` is `uutils coreutils` (the Rust reimplementation), confirmed by `sleep --version`. Direct testing (`/proc/$PID/cmdline` right after `exec -a sleep ... &`) showed the override reaching the process correctly, yet the copy still refused with `unknown program 'dotf'`. Invoking the SAME bytes at their original resolved path dispatched correctly regardless of argv[0]; invoking the copy at a different path failed regardless of argv[0]. That is only consistent with dispatch keyed on the executable's own resolved path (`current_exe()`/`/proc/self/exe`), not on argv[0] at all — the opposite axis from what the issue assumed and what `exec -a` can influence.

**Solution**: stop trying to satisfy whichever dispatch mechanism a given coreutils build uses, and stand in a binary that has no multi-call dispatch to satisfy in the first place. A copy of `bash`, driven by `"$DEST/dotf" -c 's=$SECONDS; while (( SECONDS - s < 30 )); do :; done'`, busy-spins using only shell builtins — no forked or exec'd child, so the copied ELF's own text image stays open for the whole duration regardless of what its filename is or which coreutils flavor is on the host. Verified before wiring into the test: copying bash to a renamed file and running it directly reproduced ETXTBSY on a second `cp` onto it while busy, confirming the mechanism independent of any dispatch behavior.

**Rule**: a bug report's stated root cause is a hypothesis until it reproduces on the machine actually running the fix, even when the diagnosis reads as obviously correct and even when it comes with a plausible-looking patch. "Multi-call binary" covers at least two different dispatch strategies (argv[0]-keyed, as GNU coreutils and busybox use; resolved-path-keyed, as observed here in uutils coreutils) that look identical from a bug report's black-box symptom but demand opposite fixes. When a fixture needs to hold a *real* executable text image open/busy without depending on a specific tool's argv0-vs-path dispatch behavior, prefer a binary with no multi-call ambiguity at all (a shell, driven by a builtin-only blocking loop) over trying to satisfy whichever mechanism the host happens to implement.

**Tags**: `testing`, `bash`, `coreutils`, `debugging`

### [2026-08-12] "Weaker locally, CI catches it" is not safe when local and CI share the same script

**Context**: BUG-061, fixing a spec-gate false negative where a PR that correctly archived its spec in the same change could not be pushed locally — `check-spec-gate.sh`'s archive-on-merge credit only fires when `SDD_PR_BODY` names a closing keyword, and that variable is empty on every local pre-push run by design. The filed issue offered three fix options; option 2 read "credit an archive move unconditionally in the pre-push tier... weaker, but the #397 protection against a gratuitous archive-move dodging the gate still holds in CI."

**Problem**: option 2 was implemented as stated — locally (empty `SDD_PR_BODY`), any spec that genuinely transitioned from active-at-base to archived-at-head was credited toward the LOC-threshold's "spec folder touched" requirement, without needing a closing-issue link. Two existing regression tests (`tests/check-spec-gate.bats`, the #397 pair) immediately went red: a PR bundling 60 LOC of unrelated production code with a genuine archive of an unrelated spec now passed locally, exactly the "gratuitous archive-move dodging the gate" #397 already closed. The flaw in the reasoning: "CI catches it" only holds when local and CI run *different* logic. Here they run the *same* `check-spec-gate.sh`, gated only by whether `SDD_PR_BODY` happens to be set — so a check made unconditionally permissive "when there's no PR body" is exactly as permissive in CI whenever CI itself has no PR body to supply (or, more subtly, it normalizes local runs to a genuinely weaker invariant than the one the script's own tests pin, which is a contradiction the tests exist to catch). A diff-only heuristic cannot distinguish #854's legitimate author (archiving the spec they are actually implementing) from #397's attacker (archiving something unrelated to bulk-legitimize other code) — their diffs are structurally identical, and no amount of "was this transition genuine" narrowing closes that gap without the PR-body linkage the local run doesn't have.

**Solution**: discard the local-credit approach entirely and implement option 1 instead — `scripts/spec-gate-prepush.sh`, a wrapper mirroring the existing CI adapter (`spec-gate-pr.sh`) that resolves the current branch's live PR via `gh pr view` (no token needed locally; the developer's own `gh auth`) and forwards real `SDD_LABELS`/`SDD_PR_BODY`/`SDD_PR_AUTHOR` to the unmodified gate. Unlike its CI sibling, it falls THROUGH to running the gate with no PR context on any resolution failure (no `gh`, no `jq`, unauthenticated, no PR open yet) rather than failing closed — "no PR context" is the ordinary local baseline here, not a stale-data hazard to guard against. `check-spec-gate.sh` itself needed no change beyond a message pointing at the wrapper and the manual `SDD_PR_BODY=...` override.

**Rule**: when a fix option is phrased as "loosen X here, Y elsewhere still catches it," check first whether "elsewhere" is actually a different code path or just the same code path under different environment variables — if a script is shared between an advisory local tier and an authoritative CI tier, any relaxation gated only on which env vars happen to be set is live everywhere those vars happen to be unset, not only in the tier the fix was aimed at. The repo's own existing regression tests caught this on the first test run after implementing the "obvious" reading of the option — proof that re-running the FULL adjacent test suite (not just new tests for the change at hand) before trusting an implementation is itself the guard.

**Tags**: `testing`, `shell`, `sdd`, `ci`

### [2026-08-12] "Looks like a known bug class" is a hypothesis, not a finding — reproduce before you fix

**Context**: HARNESS-070 (deploy convergence, #843/#869/#828). The session's brief carried live evidence: `dotf doctor` flagging 4 deployed skills (`computer-use`, `find-skills`, `orca-cli`, `orchestration`) as symlinks, labeled as a "BUG-100" regression — the historical, closed issue #100 about this repo's own deploy strategy fighting `agy`'s filesystem layout.
Expand Down
14 changes: 12 additions & 2 deletions scripts/check-spec-gate.sh
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,12 @@ BASE_REF=""
HEAD_REF=""
EXPLAIN=0
# Path to the open-issue feed for the advisory adjacency report (HARNESS-063).
# Empty on every local run: the fetch needs a token, so it lives in the workflow
# and this script stays offline for the pre-push hook (#854).
# Empty on every local run: fetching every open issue needs a token, so this
# feed is populated by the workflow only -- this script itself never fetches
# anything. (Distinct from BUG-061/#854's PR-body/labels/author resolution:
# that is a single PR read, needs no token, and is handled by the separate
# scripts/spec-gate-prepush.sh wrapper on the local pre-push tier -- see its
# header for why it lives outside this script rather than inside it.)
ADJACENCY_ISSUES=""

usage() {
Expand Down Expand Up @@ -715,6 +719,12 @@ cat >&2 <<EOF
(b) Add the "skip-sdd" label to the PR AND a non-empty
"## SDD skip rationale" section in the PR body.

If this already archived a spec to satisfy archive-on-merge but still
failed here: open the PR first (scripts/spec-gate-prepush.sh, wired to
this hook, resolves its labels/body/author live via \`gh\` once one
exists). Before a PR exists there is nothing to resolve; SDD_PR_BODY can
be set by hand for a one-off check: SDD_PR_BODY='Closes #N' $0 ...

Reference: AGENTS.md "Discipline Gate (NON-NEGOTIABLE)" section.
EOF
exit 1
9 changes: 6 additions & 3 deletions scripts/spec-gate-pr.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,12 @@
#
# It lives here rather than inline in the workflow for the reason recorded in
# tests/bitacora-reconcile.bats: logic inside a `run:` block is unreachable by
# tests and went red-and-silent twice (BUG-063). The token still lives in the
# workflow — check-spec-gate.sh stays offline for the pre-push hook (#854), and
# so only this CI-only wrapper needs one.
# tests and went red-and-silent twice (BUG-063). The token stays in the
# workflow: this adapter needs GH_TOKEN to read a PR it does not own (CI's
# actor). scripts/spec-gate-prepush.sh (BUG-061/#854) is the local pre-push
# equivalent and needs none, reading a PR under the developer's own `gh auth`
# instead — see its header for why its failure mode is deliberately the
# opposite of this one's fail-closed behaviour below.
#
# Usage:
# spec-gate-pr.sh --pr N [args forwarded verbatim to check-spec-gate.sh]
Expand Down
74 changes: 74 additions & 0 deletions scripts/spec-gate-prepush.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
#!/usr/bin/env bash

# spec-gate-prepush.sh: local pre-push adapter that resolves LIVE PR metadata,
# when there is a PR to resolve, before running check-spec-gate.sh.
#
# BUG-061: check-spec-gate.sh's archive-on-merge credit
# (_mandated_archive_ids) only fires when SDD_PR_BODY names a closing keyword.
# Locally that variable is empty by design ("check-spec-gate.sh stays offline"
# — see its ADJACENCY_ISSUES comment), so a PR that correctly archives its spec
# in the same change could not be pushed: the LOC gate saw no active-spec touch
# and rejected a change the author had done exactly right. The documented
# escape (`SDD_PR_BODY=... git push`) is not one anybody would guess, so the
# discoverable "fix" was --no-verify.
#
# This mirrors scripts/spec-gate-pr.sh's shape but not its failure mode: that
# adapter fails CLOSED on a bad live read because in CI the alternative is a
# stale event-payload replay (BUG-066) — deciding on wrong data is worse than
# refusing to decide. Here there is no stale fallback to guard against: "no PR
# context" is simply the ordinary state before a PR exists (first push of a
# branch), so any failure to resolve one — no `gh`, no `jq`, unauthenticated,
# no PR open yet for this branch — falls THROUGH to running check-spec-gate.sh
# with nothing set, i.e. today's exact behaviour. Once a PR exists for the
# branch, local pre-push becomes as strict as CI: archive-on-merge and the
# label-gated skips now apply locally too, not only after push.
#
# No token required: `gh` uses the developer's own local auth, unlike
# spec-gate-pr.sh's CI run, which is the one that needs GH_TOKEN.
#
# Usage:
# spec-gate-prepush.sh [args forwarded verbatim to check-spec-gate.sh]

set -euo pipefail

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" && pwd)"

usage() {
cat <<'EOF'
Usage: spec-gate-prepush.sh [args forwarded to check-spec-gate.sh]

Resolves the current branch's PR labels/body/author live via `gh`, when one
exists, then forwards every argument to check-spec-gate.sh. Falls through to
running the gate with no PR context (today's local behaviour) when `gh`/`jq`
are unavailable, the developer is unauthenticated, or no PR is open yet for
this branch — never fails on that account. e.g.
spec-gate-prepush.sh --base-ref origin/main --head-ref HEAD
EOF
}

case "${1:-}" in
-h|--help) usage; exit 0 ;;
esac

_run_gate() {
exec "$SCRIPT_DIR/check-spec-gate.sh" "$@"
}

command -v gh >/dev/null 2>&1 || _run_gate "$@"
command -v jq >/dev/null 2>&1 || _run_gate "$@"

if ! meta=$(gh pr view --json labels,body,author 2>/dev/null); then
# No PR for this branch yet, or the developer is unauthenticated. Both are
# the ordinary local baseline, not an error this wrapper reports on.
_run_gate "$@"
fi

# `// ""` on both scalars: gh returns JSON null for an empty body, and the
# literal string "null" reaching the gate would match headings and label
# substrings that were never there.
SDD_LABELS=$(printf '%s' "$meta" | jq -r '[.labels[].name] | join(",")')
SDD_PR_BODY=$(printf '%s' "$meta" | jq -r '.body // ""')
SDD_PR_AUTHOR=$(printf '%s' "$meta" | jq -r '.author.login // ""')
export SDD_LABELS SDD_PR_BODY SDD_PR_AUTHOR

_run_gate "$@"
81 changes: 81 additions & 0 deletions tests/gitattributes-eol.bats
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
#!/usr/bin/env bats
# BUG-039: git-hooks/** got an explicit `text eol=lf` rule after a CRLF
# checkout disabled the GUARD-001 dispatcher on Windows (BUG-068) -- the shebang
# became "#!/usr/bin/env bash\r" and every hook died "No such file or
# directory". That fix covered one group of extensionless files. This is the
# CLASS-LEVEL guard: every tracked extensionless text file, repo-wide, must
# resolve an explicit eol, so the next one added outside an already-covered
# group (cli/internal/initrepo/templates/**, tests/golden/**, ssh/config,
# LICENSE) fails loudly here instead of depending on which OS happens to check
# it out.

setup() {
REPO="$BATS_TEST_DIRNAME/.."
}

# Tracked files whose basename has no '.' -- the shape .gitattributes' *.sh/
# *.bash/*.bats extension-keyed rules cannot match by construction, and the
# reason every file in this class needs an explicit rule of its own.
_extensionless_tracked_files() {
git -C "$REPO" ls-files | awk -F/ '{n=$NF; if (n !~ /\./) print $0}'
}

@test "every tracked extensionless file has an explicit eol (or is declared binary)" {
cd "$REPO" || return 1
local files
files=$(_extensionless_tracked_files)
# A fixture-drift guard on the guard itself: if this ever finds NOTHING,
# the class the test protects has vanished from the repo and the test
# would pass vacuously -- the exact failure mode this whole PR is about.
[ -n "$files" ] || {
printf 'No tracked extensionless files found. Either the repo layout\n' >&2
printf 'changed (update this test) or something upstream is broken.\n' >&2
return 1
}

# One process for the whole set (not one `check-attr` per file): --stdin
# emits "<path>: <attr>: <value>" per attribute per path, two lines per
# file for the two attributes requested here.
local -A eol_of text_of
local line path rest attr val
while IFS= read -r line; do
path="${line%%: *}"
rest="${line#*: }"
attr="${rest%%: *}"
val="${rest#*: }"
case "$attr" in
eol) eol_of["$path"]="$val" ;;
text) text_of["$path"]="$val" ;;
esac
done < <(printf '%s\n' "$files" | git check-attr --stdin text eol)

# "unspecified" eol is only acceptable when text itself is explicitly
# unset (`binary`) -- a deliberate escape hatch for a future extensionless
# fixture that genuinely is binary, not a way to silence this guard for a
# text file someone forgot to cover.
local unresolved=() p
for p in "${!eol_of[@]}"; do
if [ "${eol_of[$p]}" = "unspecified" ] && [ "${text_of[$p]:-}" != "unset" ]; then
unresolved+=("$p")
fi
done

if [ ${#unresolved[@]} -ne 0 ]; then
printf 'Tracked extensionless files with no explicit eol -- a Windows\n' >&2
printf 'checkout CRLFs their shebang or fixture comparison (BUG-039/BUG-068):\n' >&2
printf '%s\n' "${unresolved[@]}" | sort | while IFS= read -r f; do printf ' %s\n' "$f" >&2; done
printf '\nAdd an explicit `text eol=lf` (or `binary`) rule in .gitattributes.\n' >&2
return 1
fi
}

@test "the five GUARD-001 hook dispatchers resolve eol=lf specifically" {
# AC1: distinct from the class guard above -- this pins the exact value,
# not just "something explicit", for the dispatchers whose CRLF breakage
# (BUG-068) is what motivated the class guard in the first place.
local f
for f in pre-commit pre-push commit-msg post-checkout prepare-commit-msg; do
run git -C "$REPO" check-attr eol -- "git-hooks/$f"
[[ "$output" == *": eol: lf" ]]
done
}
19 changes: 17 additions & 2 deletions tests/install-dotf.bats
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,26 @@ EOF
mkdir -p "$DEST"
# A shell script never triggers ETXTBSY (the kernel does not hold it as an
# executable text image), so stand in a real ELF and keep it running.
cp "$(command -v sleep)" "$DEST/dotf"
# `sleep` itself does NOT work for this on every coreutils build (BUG-054):
# on a multi-call `coreutils` binary the applet dispatch can key off the
# RESOLVED EXECUTABLE PATH rather than argv[0] (observed with uutils
# coreutils -- `exec -a sleep` changes argv[0] but the copy still dispatches
# by its own path and refuses "unknown program"), so no argv[0] trick
# recovers it once the file has been copied to a path named "dotf". A copy
# of `bash` sidesteps the whole dispatch question: it is never a multi-call
# binary, and a builtin busy-loop (no forked/exec'd child) keeps THIS COPY's
# own executable text image open for the swap to contend with.
cp "$(command -v bash)" "$DEST/dotf"
chmod 0755 "$DEST/dotf"
"$DEST/dotf" 30 &
"$DEST/dotf" -c 's=$SECONDS; while (( SECONDS - s < 30 )); do :; done' &
BUSY_PID=$!

# A fixture that failed to hold the binary busy must fail loudly here, at
# setup, instead of silently degrading the rest of the test into a check of
# the ordinary replace-a-file path (BUG-054).
sleep 0.2
kill -0 "$BUSY_PID"

run install_dotf "$VERSION" "$DEST" "$BASE"
[ "$status" -eq 0 ]

Expand Down
Loading
Loading