Skip to content

drain(#133 follow-up): bash quoting (SC2086) + status-banner truth-update#427

Merged
AceHack merged 2 commits intomainfrom
drain/133-followup-bash-quoting-status-banner
Apr 25, 2026
Merged

drain(#133 follow-up): bash quoting (SC2086) + status-banner truth-update#427
AceHack merged 2 commits intomainfrom
drain/133-followup-bash-quoting-status-banner

Conversation

@AceHack
Copy link
Copy Markdown
Member

@AceHack AceHack commented Apr 25, 2026

Summary

Codex post-merge review on PR #133 surfaced 5 P1+P2 findings on `docs/research/secret-handoff-protocol-options-2026-04-22.md`. Addressed in one follow-up PR.

Finding Severity Fix
Unquoted command substitution in macOS Keychain launcher P2 `export VAR="$(...)"`
Unquoted command substitution in Linux libsecret launcher P2 `export VAR="$(...)"`
Unquoted command substitution in 1Password launcher P2 `export VAR="$(...)"`
Status banner says "Not a BACKLOG row yet" but row exists P1 Updated banner
Transcript path Tier 5 thread P1 Reply only — path is correct (`~/.claude/projects//.jsonl` is the Claude Code transcript format)

Discipline

Per Otto-279, this is a research/history surface so research-doc names stay (Otto / Aaron in the file are intentional).

Per Otto-229, post-merge corrections to merged content go in a NEW follow-up PR (this one), not in-place edits.

Test plan

  • CI markdownlint passes
  • CI shellcheck passes (the new `"$(...)"` quoting is exactly what shellcheck SC2086 wants)
  • Auto-merge armed; merges on green

Copilot AI review requested due to automatic review settings April 25, 2026 04:22
@AceHack AceHack enabled auto-merge (squash) April 25, 2026 04:22
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Follow-up to PR #133 updating a research doc to align the status banner with current BACKLOG state and to tighten shell snippet quoting to avoid word-splitting/globbing issues flagged by shellcheck.

Changes:

  • Updated the status banner to reflect that a related docs/BACKLOG.md row now exists and that this doc remains research/design input.
  • Added quoting around $(...) command substitutions in the macOS Keychain, Linux libsecret, and 1Password op launcher examples.

Comment thread docs/research/secret-handoff-protocol-options-2026-04-22.md Outdated
Comment thread docs/research/secret-handoff-protocol-options-2026-04-22.md Outdated
AceHack added 2 commits April 25, 2026 00:32
Five Codex post-merge findings on secret-handoff protocol:

P2 ×3 (lines 121, 144, 171) — Unquoted command substitution:
`export VAR=$(...)` was unquoted in all three tier launchers
(macOS Keychain, Linux libsecret, 1Password). Quoted to
`export VAR="$(...)"` per shellcheck SC2086 — handles
whitespace / glob chars / newlines in the retrieved value.

P1 (line 6) — Status banner stale:
Banner said 'Not a BACKLOG row yet' but a BACKLOG row landed
later (docs/BACKLOG.md line 1478). Updated to reflect that
the BACKLOG row exists and points at this doc; the doc
itself is design-input research, the BACKLOG row tracks
implementation work.
… unquoted $(...)

Codex P2 ×2 caught: SC2086 is for unquoted variable expansions
($var); SC2046 is the rule for unquoted command substitution
($(...)). The two earlier 'per shellcheck SC2086' comments
referenced the wrong rule ID. Updated to phrase the rationale
in terms of the actual concern (whitespace/glob/newline
handling on the retrieved value) and reference SC2046 as the
related rule.
@AceHack AceHack force-pushed the drain/133-followup-bash-quoting-status-banner branch from e41df8e to b840d8c Compare April 25, 2026 04:33
@AceHack AceHack merged commit 3425943 into main Apr 25, 2026
13 checks passed
@AceHack AceHack deleted the drain/133-followup-bash-quoting-status-banner branch April 25, 2026 04:35
AceHack added a commit that referenced this pull request Apr 25, 2026
…rain-log

Multiple Codex/Copilot threads on #444 caught:

- L16: '3 were Otto-279' → '2 were Otto-279' (matches body's
  Threads C1-C2 = 2 OTTO-279 SURFACE-CLASS).
- L22: 'Outcome distribution: 4 OTTO-279' → '2 OTTO-279 + 2 dups'
  (matches L161 final-resolution math: 4 + 5 + 2 + 2 dups = 13).
- L56: Thread A3 'Copilot P1 ×2' → 'Copilot P1 ×3' (3 thread IDs
  listed: ejy1 + eenN + eenr).
- L87: non-portable `grep -i "actionlint\|shellcheck"` → portable
  `grep -iE "actionlint|shellcheck"` (BSD/macOS grep doesn't
  support `\|` BRE alternation; the `-E` extended-regex form is
  POSIX-portable). Captured the rationale inline so the verification
  command actually works on macOS.

Same count-vs-list cardinality pattern (Class B in PR #465 doc-lint
suite BACKLOG row) — third drain-log of mine to exhibit it (after
#195 and #231). The shellcheck-rule-precision class also surfaces
via the `\|` portability finding (related to SC2086-vs-SC2046 from
#427 drain-log).
AceHack added a commit that referenced this pull request Apr 25, 2026
…arch) (#444)

* hygiene(#268): pr-preservation drain-log for #377 (setup-tooling research)

Otto-268 backfill: drain-log for PR #377 covering 13 threads — notable
for high stale-resolved density (38%, 5 of 13) where the doc was
authored against a future-state of main that adjacent PRs landed
during the review window.

Per Otto-250 training-signal discipline. Pattern observations capture
four load-bearing patterns:
1. High stale-resolved density (38%) when research doc forward-
   authors against future state of main; adjacent PRs landing
   produces natural drift.
2. "CLAUDE.md-level rule" cite shape is undisciplined — Otto-NNN IDs
   live in memory files; CLAUDE.md has the rule shapes. Fix template
   for any factory-rule cross-reference.
3. Runner-matrix vs current-truth drift is recurring; research docs
   need explicit "post-#NNN landing" annotations.
4. Otto-114 forward-mirror landing is a high-leverage substrate
   improvement — converts memory-file dangling-citation findings from
   re-fix-required to verify-and-resolve.

* drain(#444 follow-up): correct Otto-248 memory file path in #377 drain-log

Codex P1 caught that the cited memory file path in #377's drain-log
()
doesn't exist; actual file is the longer
.

This was a fix-induced citation error inherited from #377's research
doc (which used the same wrong abbreviated path). Both #377 and
#444 needed correction — landed paired (#377 force-pushed earlier
this tick, #444 corrected here). The drain-log inherited the wrong
citation from the research doc it was logging.

* drain(#444 follow-up): fix count mismatches + portable grep in #377 drain-log

Multiple Codex/Copilot threads on #444 caught:

- L16: '3 were Otto-279' → '2 were Otto-279' (matches body's
  Threads C1-C2 = 2 OTTO-279 SURFACE-CLASS).
- L22: 'Outcome distribution: 4 OTTO-279' → '2 OTTO-279 + 2 dups'
  (matches L161 final-resolution math: 4 + 5 + 2 + 2 dups = 13).
- L56: Thread A3 'Copilot P1 ×2' → 'Copilot P1 ×3' (3 thread IDs
  listed: ejy1 + eenN + eenr).
- L87: non-portable `grep -i "actionlint\|shellcheck"` → portable
  `grep -iE "actionlint|shellcheck"` (BSD/macOS grep doesn't
  support `\|` BRE alternation; the `-E` extended-regex form is
  POSIX-portable). Captured the rationale inline so the verification
  command actually works on macOS.

Same count-vs-list cardinality pattern (Class B in PR #465 doc-lint
suite BACKLOG row) — third drain-log of mine to exhibit it (after
#195 and #231). The shellcheck-rule-precision class also surfaces
via the `\|` portability finding (related to SC2086-vs-SC2046 from
#427 drain-log).

* hygiene(#444): reconcile 377 drain-log outcome distribution math

Codex P2 + Copilot both caught: header said '4 FIX + 2 dups' but
Section A enumerates 6 FIX thread-IDs (A1×1 + A2×2 + A3×3) and
Section B enumerates 5 STALE thread-IDs (B5 explicit dup of B3).
Header didn't match the per-section enumeration end-to-end; intro
prose ('3 were real-fix factual corrections' + '2 were combined')
disagreed with the header in turn.

Pick a single counting rule (by thread-ID) and apply it
consistently:
- 6 FIX (3 unique findings, 3 duplicate reviewer threads on the
  same fixes — combined into one fix commit c8d91b5)
- 5 STALE-RESOLVED-BY-REALITY (4 unique + 1 dup B5≡B3)
- 2 OTTO-279
- = 13 thread-IDs covering 9 unique findings

Fix header + intro prose + final-resolution all to match this
single rule. The 'unique findings' count (9) is preserved in
parentheses for cross-reference.
AceHack added a commit that referenced this pull request Apr 25, 2026
…449)

* hygiene(#268+): pr-preservation drain-log for #427 (#133 follow-up)

Otto-268 follow-on: drain-log for the post-merge cascade PR #427
following parent #133 (research: secret-handoff protocol options).

Per Otto-250 training-signal discipline. Captures two specific
fixes from the cascade wave:

1. **Shellcheck SC2086 → SC2046 correction**: prior rationale cited
   the wrong shellcheck rule. SC2046 covers unquoted command
   substitution `$(...)`; SC2086 covers unquoted variable expansion
   `$var`. Pre-commit-lint candidate: regex check on shellcheck
   SC-NNNN claims against the actual rule applying to cited code
   shape.

2. **Status-banner truth-update**: doc-claim-staleness during review
   window, same class as #135 DORA canonical definitions and #231
   Wave-4 version-currency reclassifications.

Pattern observation: drain follow-ups for substantive PRs are
themselves often small + targeted; substantive technical content
gets first-wave attention, small cleanups land as separate
follow-ups when they don't gate merge.

* hygiene(#449): reflow maintainer-asleep across line break

Copilot P2 catch: previous wrap split "maintainer-asleep" mid-token
as "maintainer-" / "asleep" which renders with extra space ("maintainer-
asleep") in Markdown. Reflow so the hyphenated compound stays on a
single line.

Class A pattern (inline-code-span / hyphen line-wrap) per
docs/pr-preservation/_patterns.md.
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.

2 participants