allod secret: answer the cross-family review #198

Merged
vnprc merged 1 commit from agent/secret-review-fixes into master 2026-09-11 02:39:03 +01:00
Member

Seven fixes to allod secret from a cross-family review of allod/tools#195 after it merged, none of which changes what a successful landing writes. The command now refuses a checkout whose default branch cannot be resolved instead of guessing master and risking a push to main; refuses when git status itself fails; restores and unstages both files when git add or git commit fails, and says so plainly if that restoration itself fails; flips the pending entry correctly when a comment quotes the field or a string contains a brace; evaluates a checkout whose path contains a space; and compiles again on the BSD family, which the moved terminal code had broken for every untagged build. On Nexus nothing changes until the allod-tools pin moves. What proves it: all four tag builds pass with six new tests, a darwin cross-vet passes, and the tagged binary was smoke-tested against a real secrets clone under a path with a space. If it is wrong, revert; the previous behavior differs only in the failure paths named above.

Refs allod/tools#182. Reviews: gpt-5.6-sol and gpt-5.6-terra via pi, thinking high, each reading the merged diff of #195 cold. codex refused both models with a reconnect error, so the runner changed and the model ids did not. Between them nine findings; seven reproduced against the source and are fixed here. Two were discarded: one claimed git add or commit failures leave files modified, which is the same defect as another finding and is fixed; one claimed the moved code changed site config behavior, which the unchanged site tests refute.

Risk

R2. Every change is on a refusal or rollback path; the happy path's writes, recipients, and commit contents are untouched, and the existing tests for them pass unchanged.

Validation

  • nix flake check on this branch as git+file://<worktree>: all checks passed.
  • New tests: default branch unknown is refused; a failing pre-commit hook leaves a clean, unstaged tree for both create and rekey; the flip survives a commented duplicate and a brace inside a string.
  • CGO_ENABLED=0 GOOS=darwin go vet -tags site,secret ./cmd/allod passes; before this change syscall.TCGETS was undefined there.
  • Smoke against a clone of the secrets template under .../secret landing/ with real nix eval and real age -d: an active entry is refused as not pending, rekey reaches the decrypt call, and unsetting origin/HEAD produces the new refusal with its remote set-head hint.
Seven fixes to `allod secret` from a cross-family review of allod/tools#195 after it merged, none of which changes what a successful landing writes. The command now refuses a checkout whose default branch cannot be resolved instead of guessing `master` and risking a push to `main`; refuses when `git status` itself fails; restores and unstages both files when `git add` or `git commit` fails, and says so plainly if that restoration itself fails; flips the pending entry correctly when a comment quotes the field or a string contains a brace; evaluates a checkout whose path contains a space; and compiles again on the BSD family, which the moved terminal code had broken for every untagged build. On Nexus nothing changes until the `allod-tools` pin moves. What proves it: all four tag builds pass with six new tests, a darwin cross-vet passes, and the tagged binary was smoke-tested against a real secrets clone under a path with a space. If it is wrong, revert; the previous behavior differs only in the failure paths named above. Refs allod/tools#182. Reviews: gpt-5.6-sol and gpt-5.6-terra via pi, thinking high, each reading the merged diff of #195 cold. codex refused both models with a reconnect error, so the runner changed and the model ids did not. Between them nine findings; seven reproduced against the source and are fixed here. Two were discarded: one claimed `git add` or commit failures leave files modified, which is the same defect as another finding and is fixed; one claimed the moved code changed `site config` behavior, which the unchanged site tests refute. ## Risk R2. Every change is on a refusal or rollback path; the happy path's writes, recipients, and commit contents are untouched, and the existing tests for them pass unchanged. ## Validation - `nix flake check` on this branch as `git+file://<worktree>`: all checks passed. - New tests: default branch unknown is refused; a failing pre-commit hook leaves a clean, unstaged tree for both `create` and `rekey`; the flip survives a commented duplicate and a brace inside a string. - `CGO_ENABLED=0 GOOS=darwin go vet -tags site,secret ./cmd/allod` passes; before this change `syscall.TCGETS` was undefined there. - Smoke against a clone of the secrets template under `.../secret landing/` with real `nix eval` and real `age -d`: an active entry is refused as not pending, `rekey` reaches the decrypt call, and unsetting `origin/HEAD` produces the new refusal with its `remote set-head` hint.
Refuse a checkout whose origin/HEAD is unset instead of guessing master; refuse when git status itself fails; restore and unstage both files when git add or commit fails; report a restore that itself failed instead of claiming a clean tree; skip comment lines and braces inside strings when flipping the pending entry; run every nix invocation from inside the checkout as path:. so a directory with a space in its name evaluates; and split the termios ioctl constants per platform so the untagged build compiles on the BSD family again.

Refs allod/tools#182
vnprc approved these changes 2026-09-11 02:38:57 +01:00
vnprc merged commit 409ac40960 into master 2026-09-11 02:39:03 +01:00
vnprc deleted branch agent/secret-review-fixes 2026-09-11 02:39:03 +01:00
Sign in to join this conversation.
No description provided.