Repository navigation
ums: capture GIA learnings — heredoc hook, chapter wiring, empty promises - #2664
Conversation
…ises - Security hook: don't heuristic-strip heredoc bodies; keep full scan - Quarto new chapter: format block, navbar, index, WORDLIST + lychee - Inline R: formatC with format="d" to avoid scientific notation - No empty promises: don't use 'will' for future work without mechanism; daytb covers GIA checkpoint
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@claude review |
|
👀 Picked up by workflow run #33296302728. Setup runs first; Claude itself responds after that. |
This comment has been minimized.
This comment has been minimized.
|
@claude review |
|
👀 Picked up by workflow run #33296453778. Setup runs first; Claude itself responds after that. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
78640ac to
d537ac9
Compare
|
@claude review |
|
@claude review |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@claude review |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@claude review |
Could not push the changesPush rejected: the branch moved on the remote while this run was working. The push was rejected as a non-fast-forward: the branch gained commits on This usually means another session, another workflow run, or a person The commits below were made during this run but are not on the branch -- they exist only on the runner, which is discarded when the job ends. git outputPatch -- recover with `git am`(truncated at 53672 bytes; the full patch is in the workflow run) From 4d4dfc1acd11bba3b8c7b1c465c6d3829a5189ab Mon Sep 17 00:00:00 2001
From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com>
Date: Sun, 30 Aug 2026 06:23:40 +0000
Subject: [PATCH 1/5] fix(memories): undo unnecessary whole-file reflow from
prior fix
The previous commit (78640ac) addressed review Finding 1 (false daytb/mwc
claim) and Finding 2 (Do/Don't pairing) correctly, but also ran a
semantic-line-break reformatter across the ENTIRE contents of both files
instead of scoping it to the actually-new/changed lines. That reflowed
hundreds of untouched, pre-existing lines (shared/coding/ascii-punctuation-in-source.md's
documented anti-pattern: 'Fixing one flagged glyph is a one-line edit;
fixing it with a whole-file replace is a different, much larger change'),
inflating preferences.md from 1195 to 1680 lines and debugging.md from
1202 to 1312 -- both past the memory-file-size CI gate (1250 lines),
which is why validate was failing on this branch.
Reset both files to origin/main and re-applied only the legitimate
changes: the corrected 'No empty promises' bullet (false claim removed,
restructured as Do/Don't) in preferences.md, and the three wai-session
learnings appended to debugging.md, each split at sentence/clause
boundaries per the check-new-line-breaks gate's own reported findings
(confirmed against the actual CI failure log for the new-line-breaks job
on commit 49a91ca, rather than guessing at what needed wrapping).
---
memories/debugging.md | 207 +++-------
memories/preferences.md | 873 +++++++++-------------------------------
2 files changed, 265 insertions(+), 815 deletions(-)
diff --git a/memories/debugging.md b/memories/debugging.md
index 235a377..d4140d7 100644
--- a/memories/debugging.md
+++ b/memories/debugging.md
@@ -2,9 +2,7 @@
## Heredocs in chained terminal commands are unreliable
-Multi-line heredoc-style commands in chained terminal commands get garbled or
-silently fail. Always write multi-line content to a temp file first, then
-reference it:
+Multi-line heredoc-style commands in chained terminal commands get garbled or silently fail. Always write multi-line content to a temp file first, then reference it:
```sh
cat > /tmp/msg.txt << 'EOF'
@@ -14,23 +12,17 @@ EOF
git commit -F /tmp/msg.txt
```
-Never inline heredocs in chained commands. Applies to git commit messages, MR
-descriptions, and any other multi-line content passed to CLI tools. (Learned
-during HACtions MR !37.)
+Never inline heredocs in chained commands. Applies to git commit messages, MR descriptions, and any other multi-line content passed to CLI tools. (Learned during HACtions MR !37.)
## Markdown line-by-line processors: every inner loop needs its own fence tracker
-When writing a script that reformats Markdown line by line, the outer loop
-usually
+When writing a script that reformats Markdown line by line, the outer loop usually
tracks `in_code_block` to pass fence bodies verbatim. But inner collection loops
-(bullet continuation, blockquote accumulation) often collect lines first and
-process
-later — they need their **own** fence-state tracker, or fenced code blocks
-inside
+(bullet continuation, blockquote accumulation) often collect lines first and process
+later — they need their **own** fence-state tracker, or fenced code blocks inside
bullets/blockquotes get stripped, joined, and reflowed as prose.
-Pattern: any loop that accumulates lines before processing should break (or
-track
+Pattern: any loop that accumulates lines before processing should break (or track
`in_inner_code`) when it hits a ```` ``` ```` line.
**Test matrix before shipping a Markdown reformatter:**
@@ -41,17 +33,14 @@ track
Hit on Morrison-Lab/ai-config#265: the semantic-line-breaks script lacked fence
tracking in the bullet continuation loop and the blockquote collection loop.
-Both took two rounds of review to fully fix (single-line fence caught in round
-1,
+Both took two rounds of review to fully fix (single-line fence caught in round 1,
multi-line body in blockquotes caught in round 2).
-A separate edge case also surfaced in round 3: `_flush_bq_prose` silently
-dropped
+A separate edge case also surfaced in round 3: `_flush_bq_prose` silently dropped
lines when `split_sentences` returned `[]` (empty blockquote text) and multiple
lines had accumulated — unrelated to fence state, but found in the same cycle.
## Testing CSS/JS-dependent web features — use a REAL browser, not a DOM stub
-- A hand-rolled DOM shim (or jsdom-style unit test) can PASS while the feature
-is
+- A hand-rolled DOM shim (or jsdom-style unit test) can PASS while the feature is
visibly broken, because it doesn't apply the page's CSS or run the framework's
own scripts. On rme#929 a DOM-stub test of a mobile TOC passed, but in a real
browser the menu never opened: a framework CSS rule (`nav[role=doc-toc]{display:none}`)
@@ -97,16 +86,14 @@ is
trigger table, and read its date rather than assuming either answer.
- Poll until a review note appears that references your latest commit SHA.
- Wait ~30-60s, then check. If nothing after ~2min, check pipeline status.
-- The iterate skill now has explicit polling instructions for both GitHub and
-GitLab.
+- The iterate skill now has explicit polling instructions for both GitHub and GitLab.
## Confirm a CI failure is pre-existing (not your diff) via workflow-run history on main
- Before waving off a red CI check as "unrelated, pre-existing" (e.g. to defer
fixing it as out of scope for the current PR), verify it, don't assume —
check that same workflow's run history filtered to `main`, not just that the
failure looks unrelated to the files in your diff.
-- `mcp__github__actions_list` (`list_workflow_runs`, `resource_id:
-"<file>.yml"`,
+- `mcp__github__actions_list` (`list_workflow_runs`, `resource_id: "<file>.yml"`,
`workflow_runs_filter: {"branch": "main"}`) returns that job's own run
history; sort by `run_number` descending and read the most recent entries'
`conclusion`. If it's already `failure` on `main`'s current HEAD commit (and
@@ -142,66 +129,43 @@ GitLab.
## macOS sed vs GNU sed syntax differences
- `sed '1{/pattern/d}'` works on GNU sed (Linux CI) but FAILS on macOS BSD sed
- Portable form: `sed -e '1{' -e '/pattern/d' -e '}'` (separate -e expressions)
-- Always test sed commands locally before committing if they use address+command
-blocks
-- The CI runs on Linux (GNU sed) but local dev is macOS (BSD sed) — use portable
-form
+- Always test sed commands locally before committing if they use address+command blocks
+- The CI runs on Linux (GNU sed) but local dev is macOS (BSD sed) — use portable form
## bash "syntax error: unexpected end of file" at last line
- Almost always an unclosed construct (`if`/`fi`, quote, `$(`).
-- BUT a sneaky cause is **CRLF line endings**: `\r` makes bash read
-`then\r`/`fi\r`
+- BUT a sneaky cause is **CRLF line endings**: `\r` makes bash read `then\r`/`fi\r`
as non-keywords, so the `if` never closes -> EOF error.
- Diagnose: `sed -n 'A,Bp' file | od -c | head` and look for `\r \n`.
- Fix: `perl -i -pe 's/\r\n/\n/g' file`, then re-run `bash -n file`.
- Prevent: add `.gitattributes` with `*.sh text eol=lf`.
## Programmatic comment edits leave punctuation/grammar artifacts
-Recurring across the sparta scrub PRs (Lacaedemon/sparta#150,
-Lacaedemon/sparta#152)
-— removing inline content from comments (issue refs like `(#138)`,
-parentheticals,
-clauses) via sed/scripted passes repeatedly broke the surrounding prose. The
-reviewer
-(and Copilot) flagged ~6 of these. After any scripted comment edit, **re-audit
-the
+Recurring across the sparta scrub PRs (Lacaedemon/sparta#150, Lacaedemon/sparta#152)
+— removing inline content from comments (issue refs like `(#138)`, parentheticals,
+clauses) via sed/scripted passes repeatedly broke the surrounding prose. The reviewer
+(and Copilot) flagged ~6 of these. After any scripted comment edit, **re-audit the
touched lines** before pushing:
-- **Mid-sentence parenthetical removed → orphaned comma/period on the wrapped
-continuation line.** When the ref opened a continuation line — `# ...launched
-from the map` / `# (#122), before...` — stripping `(#122)` leaves `# ,
-before...`. Fix: move the comma/period up to the end of the prior line (`#
-...launched from the map,` / `# before...`).
-- **Line-leading `(#NN).` removed → comment marker + bare punctuation.** `##
-(#82/#84).` opening a continuation line → `##.`. Fix: end the previous line's
-sentence and drop the marker-orphan.
-- **Trailing clause/ref removed → dangling text.** `# ... see issue #61.` → `#
-... see issue.` (referent gone). Fix: drop the now-meaningless phrase.
-- **Repeated word exposed.** `keyed off the uid (#50): keyed off
-get_instance_id()` → `...uid: keyed off...`. Fix: reword.
-- Audit greps (ERE — `grep -E`): `grep -rnE "^[[:space:]]*#+[[:space:]]*[.,;:]"`
-(orphaned leading punctuation),
+- **Mid-sentence parenthetical removed → orphaned comma/period on the wrapped continuation line.** When the ref opened a continuation line — `# ...launched from the map` / `# (#122), before...` — stripping `(#122)` leaves `# , before...`. Fix: move the comma/period up to the end of the prior line (`# ...launched from the map,` / `# before...`).
+- **Line-leading `(#NN).` removed → comment marker + bare punctuation.** `## (#82/#84).` opening a continuation line → `##.`. Fix: end the previous line's sentence and drop the marker-orphan.
+- **Trailing clause/ref removed → dangling text.** `# ... see issue #61.` → `# ... see issue.` (referent gone). Fix: drop the now-meaningless phrase.
+- **Repeated word exposed.** `keyed off the uid (#50): keyed off get_instance_id()` → `...uid: keyed off...`. Fix: reword.
+- Audit greps (ERE — `grep -E`): `grep -rnE "^[[:space:]]*#+[[:space:]]*[.,;:]"` (orphaned leading punctuation),
and scan for `see issue\.`, `, #[0-9]+,`, double spaces, broken section-header dashes.
-- The blanket strip patterns that work cleanly (with `sed -E` / `sed -r` — they
-need
+- The blanket strip patterns that work cleanly (with `sed -E` / `sed -r` — they need
ERE for the `+` quantifier; in ERE, `\(` and `\)` match literal parens, not groups):
`s/ \(#[0-9]+\)//g` (inline), `s/^# #[0-9]+: /# /` (prefix — `^`-anchored, so no `g`), `s/, #[0-9]+,/,/g` — but
the line-leading and sentence-internal cases need hand edits, not sed.
## Merging main into a sibling PR can silently clobber an un-customized file
-When PR-A merges and you sync sibling PR-B (which touches the same files), a
-file
-that B never customized takes main's (A's) version **with no merge conflict** —
-so
-it can end up with content describing A's change, not B's. Hit on
-Lacaedemon/sparta#152:
-the `demos/demo.json` reason silently became Lacaedemon/sparta#143's diplomacy
-text.
+When PR-A merges and you sync sibling PR-B (which touches the same files), a file
+that B never customized takes main's (A's) version **with no merge conflict** — so
+it can end up with content describing A's change, not B's. Hit on Lacaedemon/sparta#152:
+the `demos/demo.json` reason silently became Lacaedemon/sparta#143's diplomacy text.
After such a
-merge, don't just resolve the marked conflicts — **diff the whole merge result
-vs
+merge, don't just resolve the marked conflicts — **diff the whole merge result vs
the PR's intent** and check files that merged "cleanly" but belong to this PR
-(demo manifests, PR-specific metadata) still say the right thing. Also re-run
-the
+(demo manifests, PR-specific metadata) still say the right thing. Also re-run the
PR's own invariant (here: the ref-scrub) over files main re-touched, since A may
have re-introduced exactly what B removed.
@@ -211,9 +175,7 @@ When two sibling PRs both add the same new file(s), syncing one branch after the
other merges can produce `CONFLICT (add/add)` across many paths. Fast, safe
pattern:
-- Compare `git show ":2:<file>"` vs `git show ":3:<file>"` for **every
-conflicted file** (quote the whole revision argument — bash parses a bare `<`
-mid-word as input redirection, breaking the command).
+- Compare `git show ":2:<file>"` vs `git show ":3:<file>"` for **every conflicted file** (quote the whole revision argument — bash parses a bare `<` mid-word as input redirection, breaking the command).
Representative files can establish a likely pattern, but they do not replace
the per-file check (especially where late review fixes landed).
- Keep the side with the newer baseline (often incoming `main`), then re-apply
@@ -224,12 +186,10 @@ mid-word as input redirection, breaking the command).
## A parallel session can force-push your PR branch out from under you
On Lacaedemon/sparta#150 another driver (a second `@claude` task, or GitHub's
-"Update with rebase") force-pushed the PR branch three times, each time
-replacing
+"Update with rebase") force-pushed the PR branch three times, each time replacing
my sync-merge commit with a rebase that dropped my conflict resolutions and
reverted fixes. Defenses:
-- **Before pushing to a shared PR branch, `git fetch` and check that
-`origin/<branch>`
+- **Before pushing to a shared PR branch, `git fetch` and check that `origin/<branch>`
hasn't moved since your last push** — don't assume your last push is still HEAD.
`git log --oneline "HEAD..origin/<branch>"`: non-empty means a parallel session pushed
past you. (This handles unpushed local commits, where a bare `rev-parse HEAD` vs
@@ -300,18 +260,15 @@ plain moves, reform moves, form-up drags, etc. — even if the PR's focus was on
specific case. Reviewers flag missing coverage for implicit paths.
Pattern: after placing a change in a shared branch, enumerate the full set of
-command types that reach it, and add at least one test per type beyond the
-primary
-case. Name each test to make the path explicit (e.g.
-`test_form_up_pre_faces_march_direction`).
+command types that reach it, and add at least one test per type beyond the primary
+case. Name each test to make the path explicit (e.g. `test_form_up_pre_faces_march_direction`).
Hit on Lacaedemon/sparta#352: the pre-facing block ran for form-up drags too,
but no test covered that path until the reviewer flagged it.
## Appending to skill/memory files: grep for duplicate sections first
Before adding a new `##` section to an existing skill or memory file, grep the
-file for the section heading. It's easy to append a section that already exists
-—
+file for the section heading. It's easy to append a section that already exists —
the scout-peers duplicate `## Relationship to other skills` bug (ai-config#132)
happened because an existing section was missed and a duplicate was appended at
the end. Run `grep -n "^## " -- "<file>"` before appending.
@@ -382,19 +339,16 @@ returned nothing and the citation was one step from being reported back to
its author as dangling.
The sentence was present verbatim the whole time.)
-**Third occurrence, 2026-08-20** (after ai-config#771), twice, and with two
-different phrases rather than one phrase in two files.
+**Third occurrence, 2026-08-20** (after ai-config#771), twice, and with two different phrases rather than one phrase in two files.
`never auto-reviews a PR on push` gave `grep` 0 and a `\s+` regex 1 at
`7f14442f:memories/debugging.md`;
-`at the end of this file` gave `grep` 0 at
-`a578a050:memories/claude-review-dispatch.md`, split across lines 32-33 there.
+`at the end of this file` gave `grep` 0 at `a578a050:memories/claude-review-dispatch.md`, split across lines 32-33 there.
Cite the blob rather than the path, since both wordings were fixed the same day.
The remedy above was already written and not reached for, so what recurs is
recall rather than knowledge, and this machine's BSD `grep` also rejects `-P`.
## Writing robust bash scripts (recurring review findings)
-Lessons the reviewer flagged across the `session-lock` PR
-(Morrison-Lab/ai-config#38) —
+Lessons the reviewer flagged across the `session-lock` PR (Morrison-Lab/ai-config#38) —
pre-empt these when authoring shell, especially under `set -euo pipefail`:
- **`mktemp` + rename: add a cleanup trap.** A process killed between `mktemp`
and the `mv` orphans temp files forever. Pattern: `tmp=$(mktemp -- "<dir>"/.tmp.XXXXXX);
@@ -521,8 +475,7 @@ Error in chartr("<U+2019>", "'", as.character(add_words)) :
invalid input multibyte string 5
```
-`<U+2019>` is `chartr()`'s **search** argument ---
-`hunspell:::dictionary_load()`
+`<U+2019>` is `chartr()`'s **search** argument --- `hunspell:::dictionary_load()`
normalizing curly apostrophes, hard-coded in the package --- rendered as an
escape only because a non-UTF-8 locale cannot print it.
The real bad input is element `5` of `add_words`, an accented author name in
@@ -558,8 +511,7 @@ attributed `dictionary_load()` to `spelling` rather than `hunspell` --- the same
misreading one level down, caught by the next round.)
## Verifying R-package tests: install + testthat, never `source()` the R files
-Hit on ucdavis/ettbc#14. The env had no `devtools`/`renv`, so I "verified" the
-new
+Hit on ucdavis/ettbc#14. The env had no `devtools`/`renv`, so I "verified" the new
tests by `sys.source()`-ing every `R/*.R` file and re-running the assertions by
hand. They passed — but CI's `R CMD check` failed with
`could not find function "run_augment_one"`. Two lessons:
@@ -645,8 +597,7 @@ days across several later merges, because no PR ever ran the full multi-format
website render that collides.
When you fix such a post-merge-only failure, don't stop at the fix — add a
-**cheap static check that runs on `pull_request`** so the bug class can't
-regress
+**cheap static check that runs on `pull_request`** so the bug class can't regress
unnoticed.
It needn't reproduce the whole heavy job; a few seconds of parsing
that asserts the invariant is enough.
@@ -659,8 +610,7 @@ bad input) and enforcement (the PR guard) are complementary — ship both.
## An Actions job that fails in ~1s with NO logs = a concurrency-group self-collision
When a GitHub Actions job reports `failure` almost instantly and has no logs at
all, don't hunt for a step that failed -- no step ran.
-The signature is distinctive: the run's `created_at` and `updated_at` are 1
-second apart, the
+The signature is distinctive: the run's `created_at` and `updated_at` are 1 second apart, the
job's `completed_at` is stamped *before* its `started_at`, `get_job_logs`
404s on the log download, and the check run's `output.title`/`summary`/`text`
are all empty (so there's no annotation to read either).
@@ -672,14 +622,11 @@ Per the Actions docs
"there can be at most one running job or workflow in a concurrency group at
any time" -- the workflow run already holds the group, so its own job can
never acquire it and is failed immediately.
-That sentence is verbatim from
-`data/reusables/actions/actions-group-concurrency.md`
+That sentence is verbatim from `data/reusables/actions/actions-group-concurrency.md`
in `github/docs`, the fragment both concurrency pages include.
-It sits in that fragment's `{% ifversion actions-nga %}` branch; the `{% else
-%}`
+It sits in that fragment's `{% ifversion actions-nga %}` branch; the `{% else %}`
branch reads "at most one running and one pending job" instead, which supports
-the same deadlock, since only the at-most-one-*running* half is load-bearing
-here.
+the same deadlock, since only the at-most-one-*running* half is load-bearing here.
Nothing in the docs calls this deadlock out explicitly,
so it has to be recognized from the signature.
@@ -1256,57 +1203,35 @@ artifact-download advice presupposes `@v2`.)
## Security hook: don't heuristic-strip heredoc bodies to silence a false positive
-A PreToolUse hook that blocks a git exec-vector flag (`--upload-pack`, `-O`) via
-tokenized scanning had a false positive: `cat <<'EOF'` prose mentioning `git
-grep -O` was flagged. Three successive attempts to silence it by stripping
-heredoc bodies each introduced a new full bypass:
-
-1. Truncate at first `<<` → bypass for any command *after* a heredoc (`cat
-<<'EOF' … EOF; git grep -lO…`).
-2. Skip every heredoc body to its terminator → bypass for interpreter heredocs
-(`bash <<EOF … git grep -lO… EOF`, `ssh … <<'EOF' …`).
-3. Allow-list `cat`/`tee` as "safe" consumers by single-token lookback → bypass
-for `cat <<'EOF' | bash` pipes.
-
-Each fix patched the demonstrated case without addressing the underlying design:
-whether a heredoc body executes depends on the full pipeline shape, not one
-adjacent token. The robust fix (per three consecutive `Needs more work` reviews)
-is to not special-case heredocs at all and scan the full token stream, accepting
-the narrow false positive (blocking benign prose) as the security-conservative
-trade-off over a false negative that lets the one vector the hook exists to
-catch (`-lO` bundling, which has no other defense layer in
-`.claude/settings.json`) through. The value-flag walk
-(`-e`/`-f`/`-m`/`-A`/`-B`/`-C` consumes the rest of the token) is the only
-heredoc-adjacent fix that is genuinely safe and should be kept.
-
-**Test matrix for this hook that would have caught each round before review:**
-`git grep -eOK` (no flag), `git grep -A2 -B2 -nO` (flag), `cat <<'EOF'` prose
-(conservative flag is okay), `cat <<'EOF' … EOF; git grep -lO…` (must flag),
-`bash <<EOF … git grep -lO… EOF` (must flag), `cat <<'EOF' | bash … git grep
--lO…` (must flag), `echo $((1<<2)) && git grep -lO…` (must flag).
+A PreToolUse hook that blocks a git exec-vector flag (`--upload-pack`, `-O`) via tokenized scanning had a false positive: `cat <<'EOF'` prose mentioning `git grep -O` was flagged.
+Three successive attempts to silence it by stripping heredoc bodies each introduced a new full bypass:
+
+1. Truncate at first `<<` → bypass for any command *after* a heredoc (`cat <<'EOF' … EOF; git grep -lO…`).
+2. Skip every heredoc body to its terminator → bypass for interpreter heredocs (`bash <<EOF … git grep -lO… EOF`, `ssh … <<'EOF' …`).
+3. Allow-list `cat`/`tee` as "safe" consumers by single-token lookback → bypass for `cat <<'EOF' | bash` pipes.
+
+Each fix patched the demonstrated case without addressing the underlying design: whether a heredoc body executes depends on the full pipeline shape, not one adjacent token.
+The robust fix (per three consecutive `Needs more work` reviews) is to not special-case heredocs at all and scan the full token stream, accepting the narrow false positive (blocking benign prose) as the security-conservative trade-off over a false negative that lets the one vector the hook exists to catch (`-lO` bundling, which has no other defense layer in `.claude/settings.json`) through.
+The value-flag walk (`-e`/`-f`/`-m`/`-A`/`-B`/`-C` consumes the rest of the token) is the only heredoc-adjacent fix that is genuinely safe and should be kept.
+
+**Test matrix for this hook that would have caught each round before review:** `git grep -eOK` (no flag), `git grep -A2 -B2 -nO` (flag), `cat <<'EOF'` prose (conservative flag is okay), `cat <<'EOF' … EOF; git grep -lO…` (must flag), `bash <<EOF … git grep -lO… EOF` (must flag), `cat <<'EOF' | bash … git grep -lO…` (must flag), `echo $((1<<2)) && git grep -lO…` (must flag).
(Morrison-Lab/wai#125, PR #137, 2026-08-29, three review rounds.)
## Quarto chapter frontmatter and site wiring
-Adding a new top-level chapter (`chapters/*.qmd`) requires four mechanical
-steps, and missing any one is flagged as blocking:
+Adding a new top-level chapter (`chapters/*.qmd`) requires four mechanical steps, and missing any one is flagged as blocking:
-1. YAML frontmatter `format:` block
-(`html`/`revealjs`→`<slug>-slides.html`/`pdf`→`<slug>-handout.pdf`/`docx`→`<slug>.docx`)
---- without it the revealjs output collides on the literal `{stem}-slides.html`
-path.
+1. YAML frontmatter `format:` block (`html`/`revealjs`→`<slug>-slides.html`/`pdf`→`<slug>-handout.pdf`/`docx`→`<slug>.docx`) --- without it the revealjs output collides on the literal `{stem}-slides.html` path.
2. Navbar entry in `_quarto-website.yml` `navbar.Chapters.menu`.
3. Homepage bullet in `index.qmd` `## Chapters`.
-4. WORDLIST entries for new proper nouns in `inst/WORDLIST` (add, don't reword
-to dodge).
+4. WORDLIST entries for new proper nouns in `inst/WORDLIST` (add, don't reword to dodge).
-Lychee link check: authenticated MCP endpoints quoted as prose (`https://mcp.granola.ai/mcp`, `https://api.githubcopilot.com/mcp`) are not browsable pages; exclude them in `lychee.toml` rather than treating the check failure as a broken link (wai#133).
+Lychee link check: authenticated MCP endpoints quoted as prose (`https://mcp.granola.ai/mcp`, `https://api.githubcopilot.com/mcp`) are not browsable pages;
+exclude them in `lychee.toml` rather than treating the check failure as a broken link (wai#133).
## Inline R: avoid scientific notation with `formatC(..., format="d", big.mark=",")`
-`format(200000, big.mark=",")` renders as `2e+05` under default `scipen`. Use
-`formatC(..., format="d", big.mark=",")` (or `prettyNum(..., scientific=FALSE)`)
-so derived figures like `192,000` render with commas in Quarto inline R. Hit on
-wai#128 (byok ITPM/budget figures).
-
+`format(200000, big.mark=",")` renders as `2e+05` under default `scipen`.
+Use `formatC(..., format="d", big.mark=",")` (or `prettyNum(..., scientific=FALSE)`) so derived figures like `192,000` render with commas in Quarto inline R.
+Hit on wai#128 (byok ITPM/budget figures).
diff --git a/memories/preferences.md b/memories/preferences.md
index 067284e..a9cbe5d 100644
--- a/memories/preferences.md
+++ b/memories/preferences.md
@@ -9,8 +9,7 @@
If state is MERGED, trigger post-merge instead of reporting CI details. (Learned on ucdavis/bcs#266.)
Same principle for tool availability: before telling a user a capability doesn't exist in the current session (e.g. "no `subscribe_pr_activity` tool here"), run a live check (`ToolSearch`, or the equivalent discovery mechanism) rather than reciting what a memory entry or a prior session documented --- a local CLI session's tool roster isn't fixed, and reciting stale documentation as current fact is the exact failure this rule exists to prevent. (Sparta gii-ffdb93 session, 2026-07-14: initially told the user no GitHub MCP server was available in local sessions based on documented prior-session behavior, without running `ToolSearch` first.
The user's pushback "can't you use the GitHub mcp server?" was the correct challenge, and a live check would have shown the tool was in fact reachable --- that check should have been run before stating unavailability as fact, not after being questioned.)
-- **ARDI Loop Foreground Verification & Monitor Timers**: Run `python3
-scripts/check-pr-fully-clean.py <pr>` synchronously in the foreground turn;
+- **ARDI Loop Foreground Verification & Monitor Timers**: Run `python3 scripts/check-pr-fully-clean.py <pr>` synchronously in the foreground turn;
see [`shared/workflow/ardi.md`](../shared/workflow/ardi.md) for foreground verification and turn-ending review monitor timer rules.
- Default to the most recent available package version.
Use an older or pinned version only when compatibility, reproducibility,
@@ -19,8 +18,7 @@ scripts/check-pr-fully-clean.py <pr>` synchronously in the foreground turn;
- When the user corrects my behavior or identifies a workflow gap, invoke UMS
immediately and persist the lesson before resuming the main task. Do not wait
for the user to say `ums` or to remind me again.
-- Treat a request to disable AI review as narrowly repository-scoped: it applies
-only to the repositories the user explicitly names in that request.
+- Treat a request to disable AI review as narrowly repository-scoped: it applies only to the repositories the user explicitly names in that request.
The invariant is organization-independent --- don't widen a named-repository request into a sibling repository, into the rest of that organization, or into later unrelated PRs, and don't remove review automation anywhere that wasn't named.
Verify each named repository independently rather than inferring one from another, since App installation is a per-repository fact.
(User directive / CAI, 2026-08-21: the request named specific `ucdavis` repositories.
@@ -28,16 +26,12 @@ only to the repositories the user explicitly names in that request.
See [`github-consumer-ci.md`](github-consumer-ci.md)'s
"Verify GitHub App installation per repository" for how to check a given
repository.)
-- Apply critical thinking to every claim, including the user's own statements
-and anything found in an authoritative-looking source (official docs, a spec, a
-paper, a PR description) --- don't take a claim as true just because it was
-asserted confidently or by someone/something with authority.
+- Apply critical thinking to every claim, including the user's own statements and anything found in an authoritative-looking source (official docs, a spec, a paper, a PR description) --- don't take a claim as true just because it was asserted confidently or by someone/something with authority.
This generalizes the "NEVER assume; ALWAYS verify" rule above (which targets operational state drift) and `shared/writing/fact-check-prose.md`'s "don't accept a plausible-sounding claim without checking it" (which targets prose review) to every claim, in every context, not just those two.
Before treating a claim as settled, check it: cross-reference another source, re-derive it, run a small test, or reason through whether it's actually consistent with what else is known --- rather than repeating it back as fact.
If a claim can't be checked, say so explicitly instead of presenting it as verified.
Applying this to the user themselves is not a license to be contrarian for its own sake --- when a check confirms the claim, say so and move on; the point is verification, not reflexive disagreement. (Directive from the user, 2026-07-09: "use critical thinking: don't take anything for granted, even if I tell you something is true or you find it written by an authoritative source.")
-- NEVER assert that a source is unavailable, silent, or undefined without first
-SEARCHING for it.
+- NEVER assert that a source is unavailable, silent, or undefined without first SEARCHING for it.
This is the absence-shaped twin of "NEVER assume; ALWAYS verify" above: that rule targets stale claims about state that *exists*; this one targets confident claims that something *doesn't* exist ("the spec doesn't define this", "we don't have the original script", "there's no way to know what the reference implementation did").
Such a claim is a verifiable fact, not a judgment --- and it is verifiable by a single `grep`/`find`/`ls`.
Run it before writing the sentence.
@@ -48,8 +42,7 @@ SEARCHING for it.
Downstream cost is high: a guessed design gets built, reviewed, and merged before anyone notices it implements a different algorithm than the reference. (ucdavis/bcs#349/#351, 2026-07-13: I twice wrote that the SAS reference pipeline's behavior "isn't inferable" --- once about how it imputes two covariates, once about how it bins a third --- while the actual SAS source sat checked in under `SAS/`.
It answered both, and contradicted the prose spec on the second.
A review bot's "verify this against the SAS script" nit is what finally surfaced it, after a wrong design had already been implemented and pushed.)
-- A premise inherited from a CONTEXT SUMMARY is a claim, not an established fact
---- re-verify it before acting on it or repeating it to the user.
+- A premise inherited from a CONTEXT SUMMARY is a claim, not an established fact --- re-verify it before acting on it or repeating it to the user.
This is the third twin of "NEVER assume; ALWAYS verify" above: that rule targets state that drifted since you last looked, and the bullet above it targets confident claims that something *doesn't* exist.
This one targets claims that arrived in your context from an auto-compaction summary, a `handoff` note, or a `checkpoint` --- written in your own voice, about work you did.
That provenance is what makes it dangerous.
@@ -60,32 +53,19 @@ SEARCHING for it.
It may still hold --- retract the method, re-verify, and say both. (ucdavis/bcs, 2026-07-25/26: a compaction summary said PR #422 was "complete except the dedup decision" when the dedup commit was already on the branch and four review findings sat unaddressed, and that two `references.bib` author lists had been "verified against the PDF title pages using `pdftotext`" when `pdftotext` is not installed there.
Both were repeated to the user across several hours --- the first caught only when the user asked whether that PR was still being driven, the second only when a fresh extraction attempt failed.
Re-verified with `pypdf`, the author-list corrections themselves held up.)
-- "For example", "e.g.", "such as", and "in cases like X" all introduce an
-illustration, not an exhaustive list --- the guidance generalizes to any
-relevantly similar case, NOT only the case named.
+- "For example", "e.g.", "such as", and "in cases like X" all introduce an illustration, not an exhaustive list --- the guidance generalizes to any relevantly similar case, NOT only the case named.
When a rule, instruction, or memory illustrates its point with an example, apply the underlying principle broadly rather than pattern-matching on the literal example.
This matters for sweeps: when auditing PRs/repos/skills for compliance with a rule that was illustrated with an example, check for the general pattern the example illustrates, not just literal recurrences of that example.
-- NEVER fabricate anything, under any circumstances --- always PRODUCE IT FOR
-REAL.
+- NEVER fabricate anything, under any circumstances --- always PRODUCE IT FOR REAL.
Demos/recordings must be captured from the actual system through the real code path (not hand-authored data dressed up as a recording); results/metrics must come from actually running the thing; screenshots must be of real state.
If it can't be produced for real yet, do the work to make it real (build the harness, drive the real pipeline) --- do NOT fall back to "disclosing a limitation" or writing excuses into a skip-demo-recording config (e.g. a manifest field like `"skip": true`) to avoid recording a real gameplay demo; always produce real demo recordings for user-visible changes. (Directive from the user, 2026-07-22: "stop making excuses for avoiding demos.")
-- When "restoring" or reconstructing a full file's content (e.g. re-typing a
-file you fetched earlier in the conversation, or rebuilding it from memory after
-catching a truncation bug), don't trust your own transcription --- diff the
-pushed result against the actual source (`git diff <base> <head> -- <path>` ---
-two-dot, not three-dot, so it diffs against the branch tip rather than the
-merge-base --- or re-fetch and compare) before claiming it's a faithful
-restoration.
+- When "restoring" or reconstructing a full file's content (e.g. re-typing a file you fetched earlier in the conversation, or rebuilding it from memory after catching a truncation bug), don't trust your own transcription --- diff the pushed result against the actual source (`git diff <base> <head> -- <path>` --- two-dot, not three-dot, so it diffs against the branch tip rather than the merge-base --- or re-fetch and compare) before claiming it's a faithful restoration.
A plausible-sounding but invented bullet/section can slip in even when you intend to copy real content verbatim, and it reads exactly like a genuine hallucination to a reviewer (same failure mode as fabricating a demo --- just a different repo). (Learned on gha#155: while fixing a CHANGELOG truncation bug, the "restored" content itself included an invented changelog entry --- a `test-coverage` Python-support bullet describing an input/step that never existed in the repo --- caught only by a follow-up review diffing against `origin/main`.)
-- Stress-test edge cases in your OWN new code/tooling yourself, before pushing
---- don't rely on a reviewer to find them for you.
+- Stress-test edge cases in your OWN new code/tooling yourself, before pushing --- don't rely on a reviewer to find them for you.
A functional smoke test that only exercises the happy path (does the tool produce the right answer on one example) is not the same as thinking through what a careful reviewer would immediately probe: subdirectories/nesting if the code walks a path, the empty/zero/null case, the "already ran once" case if the code has any memoized/cached state, the cost/performance profile of the unconditional path.
Passing a self-review checklist against stated conventions (the existing "run the applicable review skills against your own diff" habit) is necessary but not sufficient --- it catches convention violations, not logic gaps a convention checklist was never written to catch.
Before considering a new tool/script/check done, ask explicitly: "what's the edge case a skeptical reviewer would try first?", then actually try it, rather than shipping the happy-path version and waiting to see what review finds. (Directive from the user, 2026-07-14, sparta gii-ffdb93 session: after a locally-built coverage tool shipped with a happy-path-only smoke test, review caught a design/perf issue --- the expensive step ran unconditionally even when the diff had nothing to check --- that a two-minute "what if the diff touches nothing?" self-check would have caught before pushing.)
-- Check test coverage locally before pushing, whenever the repo has a way to
-reproduce a coverage/patch-coverage gate (a local script, a coverage tool, or
-re-running the instrumented suite) --- don't rely on a CI round trip to discover
-a shortfall.
+- Check test coverage locally before pushing, whenever the repo has a way to reproduce a coverage/patch-coverage gate (a local script, a coverage tool, or re-running the instrumented suite) --- don't rely on a CI round trip to discover a shortfall.
When a genuine gap exists, add real tests targeting the specific uncovered lines, not padding aimed at the percentage.
Same pass, opposite direction: while touching a test file, look for redundant tests worth removing or consolidating --- near-duplicate cases that don't each pin something distinct --- and keep only the ones that are meaningful and important.
Growing a suite and trimming it are the same review habit, not two separate ones. (Directive from the user, 2026-07-14, sparta gii-ffdb93 session --- led to the local `patch_coverage` tool in sparta#852, and to a redundancy pass over the new tests it and sparta#853 added before pushing.)
@@ -94,23 +74,14 @@ a shortfall.
A table is precise but hard to scan for patterns;
a figure shows shape and trend at a glance.
Present both so the reader gets both precision and intuition.
-- ALWAYS record what I learn in memory/AI-instruction notes as I work (standing
-request).
-- When recording a factual claim about tool/workflow behavior (an implementation
-detail or a causal explanation derived from a specific source), cite the source
-inline --- e.g., "(source: gha#70 PR body)" --- so future sessions can calibrate
-trust and verify if needed.
+- ALWAYS record what I learn in memory/AI-instruction notes as I work (standing request).
+- When recording a factual claim about tool/workflow behavior (an implementation detail or a causal explanation derived from a specific source), cite the source inline --- e.g., "(source: gha#70 PR body)" --- so future sessions can calibrate trust and verify if needed.
Directly observed facts need no citation, but explanations inferred from a PR body, commit message, or doc do. (Learned on ai-config#118.)
Citing the source isn't the same as the citation being *accurate* --- before publishing, re-read the source and check the claim doesn't say more than the source actually establishes (a hedged "suggests"/"may" in the source shouldn't become an assertive "traces the root cause to X specifically" in the memory entry), and cross-check the new claim against related existing entries in the same file for consistency. (Learned on ai-config#482: a new bullet overstated what gha#173 had established, contradicting an existing gha#185/#187 bullet a few screens up in the same file --- caught by the PR's own review.
Recurred on ai-config#1779, 2026-08-20: a new bullet recorded a PR-authorization lesson from ucdavis/bcs, phrased as conditional on a repo-level grant.
That conditional phrasing restated --- and inadvertently narrowed --- three already-unconditional "always open the PR after pushing" bullets a few screens up in the same file.
A PR review caught it, and the fix folded the case record into the existing unconditional bullet as a citation rather than keeping a separate conditional one.)
-- Before rebutting --- or accepting --- a review finding that asserts a specific
-technical claim (a predicted CI failure, a language/tool behavior, "this
-pathspec/regex/API doesn't do what you think"), check the actual evidence for
-that exact claim rather than just re-reasoning about the tool's behavior in the
-abstract, and rather than trusting the reviewer's confidence as a proxy for
-correctness.
+- Before rebutting --- or accepting --- a review finding that asserts a specific technical claim (a predicted CI failure, a language/tool behavior, "this pathspec/regex/API doesn't do what you think"), check the actual evidence for that exact claim rather than just re-reasoning about the tool's behavior in the abstract, and rather than trusting the reviewer's confidence as a proxy for correctness.
A plausible-sounding mechanism (e.g. "Rd `\arguments{\item{name}{...}}` labels get spell-checked", or "git pathspec globs don't cross `/` by default") can be wrong for the specific tool/version in use; a real, controlled test against a case that actually distinguishes the claim from its negation is the authoritative signal, not a theory about what the tool probably does --- and this cuts both ways: accepting a false-but-confident finding wastes a fix cycle on a non-bug exactly as much as wrongly rebutting a true one does. (Learned on UCD-SERG/serodynamics#193: rebutted a claude[bot] WORDLIST finding by reading the Spellcheck job's actual log rather than debating the claim in the abstract.
Learned again on sparta#852: a review claimed a git pathspec (`scripts/*.gd`) silently missed subdirectories; an initial "confirmation" test was flawed --- it diffed against a case with no subdirectory files present, so it couldn't have shown the bug either way --- and a rigorous test against a real commit touching `scripts/campaign/*.gd` showed the claim was false.
The reviewer re-raised the same claim (inverted) on the next round, this time with a specific but wrong mechanism ("git uses `wildmatch()` with `WM_PATHNAME` by default"); `gitglossary(7)`'s own "pathspec" definition settles it authoritatively --- the DEFAULT (non-magic) pathspec is explicitly documented as "matched against that pattern using fnmatch(3); in particular, `*` and `?` CAN match directory separators" (example given: `Documentation/*.jpg` matches `Documentation/chapter_1/figure_1.jpg`), which is a DIFFERENT code path from the explicit `:(glob)` magic word (documented separately as using `FNM_PATHNAME`, which does NOT cross `/`) --- the two are easy to conflate but behave oppositely.
@@ -118,43 +89,22 @@ correctness.
Learned again on ai-config#635 (2026-07-22): a Copilot review flagged a documented CI-check-state caveat across three review rounds (5, 7, and 8, with an unrelated finding at round 6 in between), each time with a specific, checkable claim --- first that `gh pr checks`/`get_check_runs` miss raw workflow runs, then that a `gh run list --commit <sha>` fix still misses some trigger types, then that a `--branch <pr-branch>` fix has the same class of gap.
Verifying each claim directly against the PR's own actual runs (not reasoning abstractly) confirmed all three were correct in sequence, while a separate claim in the same PR --- that markdown skill docs are bound by the repo's source-code-only em-dash rule --- checked out FALSE against the rule's own explicit scope and was rebutted.
The review loop only reached zero new comments once every claim got the same live-query treatment, rather than being pattern-matched as "probably right" or "probably just noise" this many rounds in.)
-- **Always query ALL PR comments and review objects across GitHub REST endpoints
-before checking PR status.**
+- **Always query ALL PR comments and review objects across GitHub REST endpoints before checking PR status.**
When reviewing or auditing PR status, NEVER rely on a single endpoint or assume an absence of new comments because a check run completed.
Automated review agent reports (such as `Antigravity Agent Report` or `Claude Code Review`) post issue comments as `github-actions[bot]` or `claude[bot]`.
To ensure 0 unhandled findings, ALWAYS fetch all comments using `gh api repos/{owner}/{repo}/issues/{number}/comments` and all review objects using `gh api repos/{owner}/{repo}/pulls/{number}/reviews`, parse every comment payload, and confirm that all findings have been addressed or rebutted. (Learned on ai-config#1157, 2026-08-05).
-- **Always verify live OS processes (`ps aux`) when checking background task
-state.**
+- **Always verify live OS processes (`ps aux`) when checking background task state.**
`manage_task` lists harness-managed background tasks, but background script executions (such as async python test runners) can persist as live child OS processes.
When checking task state or diagnosing running tasks, run `ps aux | grep ...` to inspect and verify live OS process state before declaring zero tasks running. (User correction, 2026-08-05).
-- **Always create a dedicated `ums-<topic>` branch off default branch (`main`)
-and open a standalone PR for UMS memory passes.**
+- **Always create a dedicated `ums-<topic>` branch off default branch (`main`) and open a standalone PR for UMS memory passes.**
Never fold UMS memory updates into an in-progress feature PR branch or claim UMS is finished without opening a dedicated UMS pull request. (User correction, 2026-08-05).
-- **Always fetch and merge `origin/main` into the UMS branch before opening a
-UMS PR.**
+- **Always fetch and merge `origin/main` into the UMS branch before opening a UMS PR.**
When creating a dedicated `ums-<topic>` branch or preparing a UMS memory pass, always fetch `origin/main` and merge/rebase onto the latest default branch HEAD before opening the PR, ensuring zero initial merge conflicts. (User correction, 2026-08-05).
-- **ALWAYS run UMS IMMEDIATELY upon any user correction, incorrect claim, missed
-item, or scrutiny of the work.**
+- **ALWAYS run UMS IMMEDIATELY upon any user correction, incorrect claim, missed item, or scrutiny of the work.**
The moment the user corrects your behavior, you realize you made an incorrect claim or missed something, you read a review of your work, you receive critical feedback, or a questioned claim ("are you sure about that?") turns out to be wrong, run UMS right then --- do not wait for the task to finish, Address, a clean verdict, a first-person admission, a wrap-up prompt, or permission --- on a dedicated branch per the two bullets above.
This is the memory-file record of the triggers in `CLAUDE.md`'s "Run UMS proactively, as learnings accumulate" section --- a corrected understanding, a false claim about state, and a questioned claim that was wrong all fire immediately, and that section holds the rationale and case records (User directive / CAI, 2026-08-05 and 2026-08-25, [ai-config#2261](https://github.com/Morrison-Lab/ai-config/issues/2261)).
-- **Proactive Immediate Fixes for Self-Acknowledged / Realized Mistakes
-(In-Flight Work & Directives)**: Whenever realizing, discovering, or
-acknowledging a mistake, bug, gap, missed instruction, or oversight in your own
-in-flight work or directive-following (whether self-discovered or pointed out by
-the user), take immediate, proactive corrective action to fix it permanently
-(implement the fix/skill/memory update, commit on a dedicated branch, open a PR,
-request review, and drive to clean) in the exact same turn without waiting for a
-user prompt or follow-up instruction. (For out-of-scope codebase bugs discovered
-incidentally, file a tracking issue per `report-mistakes-proactively` instead).
-(User directive / correction, 2026-08-17.)
-- **Autonomously commit, push, and open PRs for completed changes**: When asked
-to implement, edit, or write up changes in a repository on a worktree/feature
-branch, do not finish the round by leaving modified files sitting uncommitted or
-unpushed in the working directory. Always finish the delivery cycle: stage and
-commit the changes (linking the tracking issue created per issue-first; see
-`shared/workflow/issue-first.md`), push the branch to origin, open a Pull
-Request (if one does not exist), request AI review (`@claude review` / review
-workflow), and drive to clean via ARDI. (User directive / CAI, 2026-08-18.)
+- **Proactive Immediate Fixes for Self-Acknowledged / Realized Mistakes (In-Flight Work & Directives)**: Whenever realizing, discovering, or acknowledging a mistake, bug, gap, missed instruction, or oversight in your own in-flight work or directive-following (whether self-discovered or pointed out by the user), take immediate, proactive corrective action to fix it permanently (implement the fix/skill/memory update, commit on a dedicated branch, open a PR, request review, and drive to clean) in the exact same turn without waiting for a user prompt or follow-up instruction. (For out-of-scope codebase bugs discovered incidentally, file a tracking issue per `report-mistakes-proactively` instead). (User directive / correction, 2026-08-17.)
+- **Autonomously commit, push, and open PRs for completed changes**: When asked to implement, edit, or write up changes in a repository on a worktree/feature branch, do not finish the round by leaving modified files sitting uncommitted or unpushed in the working directory. Always finish the delivery cycle: stage and commit the changes (linking the tracking issue created per issue-first; see `shared/workflow/issue-first.md`), push the branch to origin, open a Pull Request (if one does not exist), request AI review (`@claude review` / review workflow), and drive to clean via ARDI. (User directive / CAI, 2026-08-18.)
Reaffirmed 2026-08-26 as bare "always push and PR" on ai-config#2277 after a turn left four commits ahead of origin and ended with "say if you want those pushed".
- **Do:** push and open/update the PR in the same turn as the commits.
Report the PR URL in the past tense.
@@ -162,13 +112,9 @@ workflow), and drive to clean via ARDI. (User directive / CAI, 2026-08-18.)
park on a client approval-card failure,
or close with an offer to push ---
standing grant already covers push and PR (not merge).
-- When opening a GitHub PR, trigger AI review (`@claude review`) when done
-pushing, and request human review (`<reviewer>`) only after AI review passes
-cleanly or on deadlock (see request-pr-review skill).
+- When opening a GitHub PR, trigger AI review (`@claude review`) when done pushing, and request human review (`<reviewer>`) only after AI review passes cleanly or on deadlock (see request-pr-review skill).
The one exception is `Lacaedemon/sparta`, which never requests human review, on AI review approval or on deadlock escalation alike.
-- **In repos whose review workflow does not auto-trigger on PR activity, ALWAYS
-trigger AI review (`@claude review` / dispatch `claude-review.yml`) when done
-pushing code for the round.**
+- **In repos whose review workflow does not auto-trigger on PR activity, ALWAYS trigger AI review (`@claude review` / dispatch `claude-review.yml`) when done pushing code for the round.**
`ai-config` now auto-reviews ordinary in-repo PR opens and pushes via `pull_request`, so explicit dispatch is the exception rather than the default there.
Keep using the manual path when the automatic one cannot fire or was intentionally bypassed, such as an explicit `@claude review` request, a redispatch after an `@claude` agent push, or a skipped path like a fork PR.
Do not wait to be asked "did you request claude review?",
@@ -181,14 +127,10 @@ pushing code for the round.**
create, push, and open the feature PR first. The PR must expose the exact SHA
that performs the action; opening it afterward turns a costly run into an
unreviewed fait accompli. (User correction, 2026-08-03.)
-- NEVER auto-merge or squash-merge a Pull Request or Merge Request unless the
-user has explicitly granted session permission (e.g. via `/mwc` or `/maw`) or
-explicitly instructed to merge that specific PR (e.g. `/merge-it` or "merge
-this").
+- NEVER auto-merge or squash-merge a Pull Request or Merge Request unless the user has explicitly granted session permission (e.g. via `/mwc` or `/maw`) or explicitly instructed to merge that specific PR (e.g. `/merge-it` or "merge this").
Creating, pushin |
|
💰 Cost: $7.727122999999995 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Do: fetch and follow external repo's CONTRIBUTING.md and .github/pull_request_template.md before opening PR Don't: file with internal/generic template without checking required sections Fixes compliance failure on anomalyco/opencode#46226 where PR body missed required template sections (needs:compliance).
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@claude review |
|
👀 Picked up by workflow run #33298890624. Setup runs first; Claude itself responds after that. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Claude finished review — View run Review: PR #2664 —
|
Captures learnings from wai GIA session 2026-08-29:
Closes nothing — memory update.