Skip to content

fix(planning): let any session re-pin an interview-defenses digest - #10140

Merged
kyle-sexton merged 3 commits into
mainfrom
fix/planning-agent-repin
Oct 26, 2026
Merged

kyle-sexton merged 3 commits into
mainfrom
fix/planning-agent-repin

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Oct 25, 2026 •

Copy link
Copy Markdown
Member

Refs: #6728
Refs: #7425
Refs: #9864
Refs: #10044
Refs: #10087
Refs: #8756

Summary

A sha256 digest re-pin in plugins/planning/tests/interview-defenses.test.sh needed a person. tests/repin.sh (added in #8220) refused to write unless stdin and stdout were both terminals, and then asked y/N. The suite's header and every failure message said re-pinning was attended-only. So every lane PR that edited guarded interview text stopped for the owner: #10044 today, and #6728, #7425 and #9864 sit in needs-human for this gate alone. The owner removed this stop on 2026-10-25. Only an intent change or a security issue stops an item; the review lanes judge everything else.

This PR removes the gate. Any session, a lane included, may re-pin a digest after reading the diff of the guarded text. The commit records old -> new and why the defense still holds, and the PR review judges the change.

It clears no issue by itself. #6728, #7425 and #9864 still need their own wording edits, but nothing now blocks a lane from making them. #10087 asks for a notice that assumes the attended-only guard stays, so this PR makes it moot. The multi-label half of #8756 still stands.

Fix

  • tests/repin.sh:
    • Drops the terminal check, the y/N prompt, the git diff --stat and pager block, and exit codes 5 and 6.
    • Still edits exactly one sha256 literal and adds the reason above the pin.
    • Prints <label>: <old> -> <new> for the commit message.
  • tests/interview-defenses.test.sh:
    • ATTENDED_ONLY becomes UPDATE_RULE: read the diff, then commit old -> new with the reason.
    • REPIN_HINT drops "in their own terminal (a Claude Code ! line has no terminal)".
    • The header and the update-procedure comment say any session may re-pin after the re-read.
  • tests/repin.test.sh runs every case with no terminal:
    • The pty, decline and half-terminal cases are gone, along with the git fixture they needed.
    • The write cases now expect exit 0.
    • A new case checks the printed label: old -> new line.
  • A changelog fragment for planning (patch).

Option chosen: (a) agents re-pin without a terminal, and every digest stays.

  • I rejected (b), deleting digests that behavioral assertions already cover, because there are none. The suite has no behavioral assertion: there is no model-graded runner (interview-defenses.test.sh:14-21, :65-67).
  • Phrase and whole-line pins cannot see a qualifier added beside a pinned line. Each digest closes that escape, and one escape was shown to work from a single paragraph next to a digest boundary (:42-61). Deleting a digest reopens a hole that has already been shown to work.
  • I also rejected an ApprovalTests-style approved file per section. On the single-source-of-truth (SSOT) test it adds a second full copy of each guarded section. Here the approved artifact is the skill text in git, and the PR diff of SKILL.md or loop.md already shows the change. The digest only forces the change to be acknowledged in the same PR, and the failure names the file and section headings to diff.

Basis:

None of these puts a person in the tool's non-interactive path. The human check is review of the committed diff, which the review lanes do here.

No other plugin carries a copy of this gate. I searched plugins, scripts, .github and docs/conventions for repin, re-pin, attended-only and -t 0 / -t 1 terminal checks. The other "attended only" rules found are different mechanisms: a person's browser driver in testing and app-control, and work-items:ship run.

Verification

  • bash plugins/planning/tests/repin.test.sh: passed 16, failed 0. Against origin/main's gated repin.sh the same suite fails 8 of 16, so the new cases can fail.
  • bash plugins/planning/tests/interview-defenses.test.sh: PASS=172 FAIL=0.
  • shellcheck on all three edited scripts: clean.
  • All 22 plugins/planning/**/*.test.sh suites exited 0. One of them, shared/tests/tracer-live.test.sh, printed SKIP: no cloudflared; surface/surface.test.sh reported SKIP=1.
    • Counted results: align 542/542, tracer 18, surface-journey 249, surface-ui-c 191, surface 241, watch 31, interview-defenses 172, reattach-slice 23, repin 16, standards-binding 9.
    • shared/tests/ui.test.sh ran for more than 10 minutes. Its slowest files were ui_presence.js at 187s and ui_layout.js at 145s.
  • After that run, a follow-up commit fixed repin.sh cutting a label at its first escaped quote (for example the "does NOT do" pin). repin.test.sh now uses a fixture label with escaped quotes; it passes 16/16 and fails 15/16 against the old capture.
  • The branch merges cleanly with origin/main at 2d9ac7f, which includes fix(planning): merge the exported Brief into PLAN.md in place and stop claiming the gate sees post-Confirm edits #10044.

Related

🤖 Generated with Claude Code

kyle-sexton and others added 3 commits October 25, 2026 16:06
repin.sh no longer refuses without a terminal or asks y/N: it rewrites
the one sha256 literal, adds the reason above the pin, and prints
`<label>: <old> -> <new>` for the commit message. The defenses suite's
header and failure messages drop the attended-only rule; the commit
records old -> new and why the defense holds, and PR review judges it,
as approval and snapshot tests are re-approved by the developer and
reviewed in code review. Every digest stays: phrase and whole-line pins
cannot see a qualifier added beside a pinned line.

Refs: #8220, #10044

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
repin.sh cut a label such as `SKILL.md "does NOT do" section ...` at its
first inner quote; the capture now ends at the call line's last quote.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

PR body contract — issue linkage

This PR body conforms to the issue-linkage contract. Nothing to do.

@kyle-sexton
kyle-sexton marked this pull request as ready for review October 26, 2026 02:32
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-26T02:33:26.627988Z 942388a Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

Copy link
Copy Markdown
Contributor

claude-review reviewed 942388a394604191c564c91105ca1fd328e504f5: 0 finding(s), 0 as inline comments.

No findings. I read the full diff and PR context, then checked repin.sh in full and searched the repo for other references.

What I checked

  • repin.sh: The label capture strips backslashes and takes everything up to the line's last quote, so labels with escaped quotes survive. All 34 real pin_* call lines in interview-defenses.test.sh end with the label followed by \, so the capture is correct for each. The <label>: <old> -> <new> line prints before the rerun. The removed exit codes 5 and 6 appear nowhere else in the repo.
  • Stale references: No other script, skill or doc points at repin.sh or the "attended-only" rule. I found no leftover references.
  • Failure messages: UPDATE_RULE and REPIN_HINT are used consistently. The exact-line and boundary failures still omit REPIN_HINT, which matches the comment saying repin.sh has nothing to recompute for them.
  • repin.test.sh: The new label-with-quotes fixture works through the heredoc escaping. After backslashes are stripped, "quoted" matches the expected output line. The unattended run is the only path now, so dropping the pty and decline cases and the git fixture removes nothing that can still fail. Dropping the GIT_INDEX_FILE guard is safe because the test no longer runs any git command.
  • Changelog fragment: bump: patch fits a change to a test helper and its messages.

For a person to know, not a defect: With the terminal and y/N gate gone, a lane can edit guarded interview text and re-pin its digest in the same PR. The digest then only forces the change to be acknowledged, and the PR review is the only check on whether the defense still holds. The PR body says the owner chose this on 2026-10-25, so I did not count it as a finding.

Not verified: I had no shell, so I ran none of the suites. The pass counts in the PR body (repin.test.sh 16/16, interview-defenses.test.sh 172/0, shellcheck clean) are author-claimed and unverified.

Spend and holds: The change spends nothing and sets no hold. Trusted-path globs matched none of the four changed files.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-security-review: 1 inline finding(s) on 942388a394604191c564c91105ca1fd328e504f5; the summary is in this lane's pull request comment.

claude-lane: claude-security-review

Comment thread plugins/planning/tests/interview-defenses.test.sh
@github-actions

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 942388a394604191c564c91105ca1fd328e504f5: 1 finding(s), 1 as inline comments.

Security review of #10140 at 942388a. The diff touches no trusted-path file (no trusted-actors, check-trusted-trigger, CODEOWNERS or .gitmodules), no plugin manifest and no workflow, so the manifest and Actions-hardening items don't apply. The skip gate does not apply either: the diff deletes a standing control, so I read it under the instruction-surface lens and the register at docs/conventions/instruction-exception-register/README.md, which was present.

critical-findings: 0

IMPORTANT: the diff lets an agent re-pin the tripwire over its own edit, and the only stated authority for removing the gate is the PR text. The register's agent-authority class (Gate 0) is the match, by consequence.

  • What the rule prevented. The old header said recomputing a digest "is attended-only: an unattended run that hits a failing digest reports the pin and the git diff, then stops, and never updates it". repin.sh enforced that with a [[ -t 0 && -t 1 ]] check and a y/N prompt (exit 5 or 6). This diff deletes both the rule and the check.
  • What now reaches that action unchecked. A lane that edits a guarded section of the interview SKILL.md, loop.md, an eval case or a fixture can run repin.sh. That rewrites the sha256 and writes its own justification in the same commit. No unchecked step is left once the PR review lane judges the change. The suite exists for exactly this: a single qualifying paragraph beside a pinned line weakens the "auto-guard never folds a user choice" and "lock mode does not fudge a gap" defenses, and the digest was the only thing that forced a human to see it.
  • Nothing replaces it. repin.sh has no replacement control, and I found no CODEOWNERS or other deterministic check on these pins. Outside plugins/planning/tests, only scripts/affected-tests-live.sh and a changelog and docs mention it.
  • Which path matters. A prompt-injected or confused lane can do this just as easily as a well-meaning one. The justification string is free text that nothing validates.
  • The authorization claim cannot be checked. The PR description says the owner removed the stop on 2026-10-25 and that only an intent change or a security issue stops an item. The description is author-written data, and I found no owner decision in the repository (a search for the phrase and date found nothing outside the PR context file). The author is a Claude-generated lane PR, so I took it as a claim and did not treat it as authority.
  • Suggested handling. The owner should confirm the decision on this PR and treat the PR review as the sole check on any future re-pin. If they keep it, consider a deterministic backstop: for example, a CI check that fails a PR when a digest literal in interview-defenses.test.sh changes without a matching Re-pin: line (old -> new) in the commit message, or a CODEOWNERS entry for that file. I rate it IMPORTANT, not CRITICAL: this is a design decision the owner may have made, the compensating control is review of a visible diff, and no remote exploit path exists. It does not set hold.

repin.sh mechanics: no injection found.

  • The label reaches awk through the environment, not -v.
  • The justification is single-line enforced and written through ENVIRON.
  • Only the sha256 literal is substituted.
  • The new digest is matched as [0-9a-f]{64} from the test output.
  • Exit 1 is correctly returned when the post-re-pin run still fails.

Spend: no billed resource or paid setting changes.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 26, 2026
Merged via the queue into main with commit 68d3f2b Oct 26, 2026
86 checks passed
@kyle-sexton
kyle-sexton deleted the fix/planning-agent-repin branch October 26, 2026 03:28
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.

1 participant