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
21 changes: 21 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,27 @@ the delete. Run one pass at a time: the slot name has no invocation-unique compo
two concurrent calls on one slot race. Passes are sequential by construction, so that is
a stated limitation, not a guarded one.

**Optional companions, from field practice.** Two files may sit beside a findings file.
Both are advisory human notes: neither is ever the findings file, neither participates in
pass validation, and either may be deleted or rebuilt. The findings file plus its
terminator remain the only hard requirement, and a zero-finding pass needs no companion.

- `<slot>-dispositions.md` — one line per finding: verdict + reason. It makes a dismissal
durable, so "we looked at that and why" outlives the session rather than the chat.
- a **cycle-stable** resume note when a cycle is interrupted — `gate-a-spec-resume.md`,
`gate-a-plan-resume.md`, `gate-b-resume.md`. Cycle-stable, not pass-named: a note keyed
to the interrupted pass number is exactly the file a resuming agent will not look for
once the counter moves or an incomplete pass is discounted. Gate A runs separate spec
and plan loops, so those are two cycles; Gate B is one cycle with one note even under
`reviewType: full`, because the per-branch findings files race only since Codex's two
reviewers write them — the resume note is written by the outer agent, sequentially, and
splitting it would create two records able to disagree about one shared recovery budget.
Whoever runs the cycle writes it when useful, replaces it as the cycle moves, and
deletes it once the cycle closes. Nothing depends on it existing.

(Field practice, infinite-portfolio-canvas: 7 dispositions files and a Gate-A resume note
had been invented per-session there before the protocol knew about them.)

**Accept a pass only when** the file exists and is readable; its last line is exactly
`END OF FINDINGS (<n> total)`; it contains exactly `<n>` finding lines *and nothing
else* (or the single line `NO FINDINGS` when `<n>` is 0 — "n valid lines somewhere in
Expand Down
1 change: 1 addition & 0 deletions docs/hardening-log.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,4 @@ escape `\|`, one line), `source` (gate-a|gate-b|bot|manual),
| 2026-07-25 | unverified-enforcement-claim | docs/sparring-briefing.md asserted "This is a prompt artifact and follows docs/prompt-standards.md" while giving its target model as "any capable chat model" — checklist item 1 requires a NAMED executing model plus the author having checked that model's prompting page, and a model class satisfies neither half (there is no single page to check), so the document contradicted its own conformance claim | bot | major | pending | todos.md § Tooling revalidation, "Prompt-standards conformance checker — resolves two `pending` ledger rows (2026-07-25)". THIRD occurrence, after 2026-07-18 `P std` (prompt-standards item 11) and 2026-07-19 `1 prose` (the "never describe what a gate proves" rule, whose own ref states it is human-run, not CI-run, and "raises the floor; it does not close the class"). The class has now cycled P std → 1 prose → recurrence without ever reaching a deterministic rung, and this instance IS tool-decidable — a file claiming to follow prompt-standards must carry a `Target model:` line — which is why the escalation is mechanical rather than a fourth prose rule. The FIX landed in 4f31df2 (PR #12); the HARDENING is what is pending, deferred to the canvas-findings round on the same scope reasoning as the sibling row. Resolve by appending a rung-2 row referencing this one, never by editing it |
| 2026-07-26 | unverified-enforcement-claim | third occurrence, resolved: a prompt asserting it follows docs/prompt-standards.md while naming no executing model — the PR #12 sparring-briefing said `Target model: any capable chat model` | bot | major | 2 lint | scripts/check-invariants.sh check 4a + fixtures in .test.sh. **Resolves the 2026-07-25 `pending` row** (same fingerprint, appended not edited). GUARDED SPELLING, exactly: in a `*.md` file containing the literal `prompt artifact and follows`, the column-zero `^Target model:` line must appear exactly once, its value must BEGIN with a tested token (`Claude\|Codex\|GPT`, portable ERE boundary — token-anywhere is not enough, since the real defect line contained "Claude" as provenance), and it must name exactly one DISTINCT token. STILL INSTRUCTION-BACKED: a bare `Target model: Claude` naming no execution surface passes; a value whose prose is wrong in a way no token test sees passes; a column-zero declaration inside a fenced block counts (no fence awareness in POSIX grep); an unknown future model fails until the token list is extended. BLIND SPOT, stated because it is structural: this ledger is EXCLUDED from the check it records — a ledger that quotes defects self-rejects the checks that detect them, and scanning it would fail the repo forever on rows like this one. OVER-ESCALATION WARNING: `harden-finding` compares fingerprints only, so a later in-class defect OUTSIDE the guarded spelling will be proposed for a stronger rung than anything justifies; that is a proposal a human validates, and this row names the guard so they can judge it. The skill's scope-blind recurrence rule is parked in todos.md |
| 2026-07-26 | docs-drift | third occurrence, resolved: a prose count contradicting the checklist it counts — PR #12's `docs/prompt-standards.md` said "all ten items" while its own checklist ran 1-12 | bot | minor | 2 lint | scripts/check-invariants.sh check 4b + fixtures in .test.sh. **Resolves the 2026-07-25 `pending` row** (same fingerprint, appended not edited). GUARDED SPELLING, exactly: two-stage claim recognition over `*.md` — stage 1 matches `all <digits\|one..twenty>( checklist)? items` with outer word boundaries, stage 2 requires canonical decimal, so `all 012 items` fires as malformed rather than being invisible; every recognised claim must equal N, where N comes from exactly one `^## Checklist([[:space:]].*)?$` heading whose body holds labels forming the contiguous canonical sequence 1..N, in BOTH docs/prompt-standards.md and the workflow-init template, which must agree. Word forms are in scope BECAUSE the motivating occurrence was one — a digit-only check would have sailed past the very defect this row claims to harden. STILL INSTRUCTION-BACKED: counts above twenty in word form, ordinals, hyphenated compounds (`all twenty-one items`), split-line claims, and any count of something other than this checklist. Same ledger blind spot and over-escalation warning as the row above |
| 2026-07-26 | session-bound-context-not-durable | Gate A/B dispositions and interrupted-cycle state lived only in chat history, so a dismissal's reasoning and a cycle's resume point died with the session; the field had independently invented per-pass companion files (7 dispositions files and a Gate-A resume note in infinite-portfolio-canvas) because the shipped protocol knew only about the findings file | manual | minor | P std | CLAUDE.md §5 "Optional companions, from field practice" + the same block in the workflow-init inline template: `<slot>-dispositions.md` and a cycle-stable resume note (`gate-a-spec-resume.md`, `gate-a-plan-resume.md`, `gate-b-resume.md`). MECHANISM, stated exactly: this is a paragraph of prompt text and **nothing enforces it** — the companions are optional, no step requires one to exist, none participates in pass validation, and a cycle that writes none behaves exactly as before. It recommends durable notes; it does not make context durable. Reading `P std` as more than that would be `unverified-enforcement-claim`, the class resolved at rung 2 in #13. BLIND SPOT inherited from #13: checks 4a/4b do not scan docs/hardening-log.md, so nothing mechanical reads this row either. Gate A rejected `truncated-tool-output-read-as-complete` for this: nothing is truncated, the content was never written |
10 changes: 10 additions & 0 deletions docs/hardening-taxonomy.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,16 @@ the synonyms a future reader might search for instead.
(Defined here retroactively: the 2026-07-20 ledger row used this class before any
definition existed. Recorded now so the recurrence grep has something to land on.)

- `session-bound-context-not-durable` — reasoning that has to outlive a session is left in
chat history, so it is gone at the next session, model change or tool switch. Aliases:
lost rationale, why-did-we-dismiss-that, undocumented decision, no resume state,
context died with the session, "it was in the chat".

**Not `docs-drift`.** Drift is two artifacts *disagreeing*; this is one artifact never
existing. The fix differs too: drift is repaired by updating whichever is stale, this by
writing the thing down at all. Also not `truncated-tool-output-read-as-complete` —
nothing here is truncated or misread as complete; the content was simply never durable.

**Promotion candidate.** These classes are stack-neutral, not project vocabulary, so
they belong in the `harden-finding` base list rather than here. They live here because
the skill says to mint into this file (the plugin ships the base classes, the project
Expand Down
68 changes: 63 additions & 5 deletions docs/pr-review-bots.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,79 @@ uses hangs the loop, and treating a channel as context silently drops real findi

| Bot | Enabled | Where findings appear | Notes (plan/tier limits, completion signal, quirks) |
|---|---|---|---|
| CodeRabbit | yes | **inline** | Observed on PR #1 (plan: Pro Plus, profile CHILL): posts real inline review comments on the diff, each with severity and a committable suggestion, plus a walkthrough summary comment. Read the inline comments — the walkthrough is not a findings source. |
| CodeRabbit | yes | **inline** | Observed on PR #1 (plan: Pro Plus, profile CHILL): posts real inline review comments on the diff, each with severity and a committable suggestion, plus a walkthrough summary comment. Read the inline comments — the walkthrough is not a findings source. **Its status check can pass while the comment says "Review rate limited" — see the completion-signal note: a green check does not prove the final head was reviewed.** |
| Greptile | yes | **summary always; inline usually** | Four PRs observed (#1, #2, #4, #5): a PR-level **summary comment every time**, with findings sometimes only inside it under "Comments Outside Diff". **Inline** comments on #2 (1), #4 (2), #5 (1) but **none on #1** — so inline is usual, not guaranteed. Read both channels; the summary is the one that has never been missing. **Completion signal: none you can block on.** `gh pr checks` displayed a "Greptile Review" entry for #4 and #5, but the check-runs and statuses APIs return no Greptile entry for any of those heads — the two tools disagree, so neither proves it has finished. Posts within ~4–11 min. |
| Cursor Bugbot | no | n/a | Comments only to say it is disabled for this account. Ignore. |

**Routing — these lists are authoritative.**

- **Wait for (block on it):** CodeRabbit. It has a status check, so `gh pr checks`
going non-pending is proof it finished.
- **Wait for (block on it):** CodeRabbit. It has a status check, so `gh pr checks` going
non-pending is proof the *check* finished — which is what you block on. That is **not**
the same as proof the final head was reviewed; verify that separately before merging
(below).
- **Process opportunistically (never block):** Greptile. Read whatever it has posted
when the CodeRabbit-gated pass begins, in both channels. If it posts later, handle it
as a follow-up.
- **Ignore:** Cursor Bugbot — disabled for this account, and it says so itself.

**Completion signal, per bot.** CodeRabbit posts a status, so `gh pr checks` shows it
and you can block on it.
**Completion signal, per bot.** CodeRabbit posts a status, so `gh pr checks` shows it and
you can block on it. **Two different things, and conflating them merges unreviewed heads.**

- *The check stopped pending* — the blocking signal. Block on this.
- *The final head was reviewed* — a separate verification. Observed on #12 and #13: the
check passed while the issue comment read "Review rate limited", and on #13 the only
CodeRabbit **review record** carried `commit_id` `eed589c` while the merged head was
`92de0d2`. The head that merged was never reviewed, and the green check said nothing
about it.

Verify the second before merging — a deterministic boolean, so it can gate rather than be
eyeballed:

```sh
head=$(gh pr view <n> --json headRefOid --jq .headRefOid) # the LIVE head, not local HEAD
gh api --paginate repos/<owner>/<repo>/pulls/<n>/reviews | jq -s "
[ .[][]
| select(.user.login==\"coderabbitai[bot]\")
| select(.commit_id==\"$head\")
| select((.body // \"\") | test(\"rate limit\"; \"i\") | not)
| select(.state==\"COMMENTED\" or .state==\"APPROVED\" or .state==\"CHANGES_REQUESTED\")
] | length" | grep -qv '^0$'
Comment on lines +41 to +49

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "File excerpt:"
sed -n '1,90p' docs/pr-review-bots.md | cat -n

echo
echo "Search for exact tool usage in docs/pr-review-bots.md:"
rg -n '\b(gh|jq|grep)\b' docs/pr-review-bots.md || true

echo
echo "Search pinning examples in docs/scripts:"
git ls-files '*.{md,sh,json,yml,yaml}' | xargs rg -n '\b(gh|jq|grep)\b' | head -200

Repository: dsnger/dev-workflow-kit

Length of output: 40198


🌐 Web query:

docs/prompt-standards.md CodeRabbit checks required

💡 Result:

There is no standard requirement or built-in check named docs/prompt-standards.md in CodeRabbit [1][2]. CodeRabbit employs a system of "Code Guidelines" to enforce coding standards, which automatically detects and applies existing configuration files used by other AI agents [3][4]. The default list of files monitored for these guidelines includes [1]: -.cursorrules -.github/copilot-instructions.md - CLAUDE.md - GEMINI.md -.cursor/rules/ -.windsurfrules -.clinerules/ -.rules/* - AGENT.md - AGENTS.md - REVIEW.md If you have your coding standards stored in a file named docs/prompt-standards.md, CodeRabbit will not detect it automatically by default [1][3]. To use this file, you must explicitly add it to your configuration in your .coderabbit.yaml file using the knowledge_base.code_guidelines.filePatterns setting [3]: yaml knowledge_base: code_guidelines: filePatterns: - "docs/prompt-standards.md" Once configured, CodeRabbit will pick up the rules defined in that file and apply them as review criteria [3][4]. Note that adding guideline files to path_instructions is incorrect, as that treats the file as source code to be reviewed rather than as a source of rules [3].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Find prompt-standards:"
git ls-files | rg '(^|/)docs/prompt-standards\.md$|prompt-standards' || true

if [ -f docs/prompt-standards.md ]; then
  echo
  echo "First 220 lines of docs/prompt-standards.md:"
  sed -n '1,220p' docs/prompt-standards.md | cat -n
fi

echo
echo "Determine whether docs/pr-review-bots.md is a skill/command/agent/hook message/scaffold:"
git ls-files | rg '^plugins/dev-workflow/(skills|commands|hooks|.*templates.*)|(^|/)\.mcp\.json$|(^|/)docs/prompt-standards\.md$|(^|/)AGENTS\.md$' | sed -n '1,220p'

Repository: dsnger/dev-workflow-kit

Length of output: 10748


Pin or avoid unpinned runtime tools in the merge-gate command.

docs/pr-review-bots.md is Markdown, but the documented pipeline still depends on unsigned/pinned tool versions (gh, jq, grep). Treat this as documented executable tooling: either remove the shell example or document where a repository-pinned/runtime-locked toolchain is provisioned and use it for the merge-gate check.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/pr-review-bots.md` around lines 41 - 49, Update the merge-gate command
example around the `gh`/`jq`/`grep` pipeline to avoid relying on unpinned
runtime tools: either remove the shell example or document and invoke a
repository-pinned or runtime-locked toolchain provisioned for this check. Ensure
the documented command uses that locked tooling for the review filtering logic.

Source: Coding guidelines

```

**`--slurp` cannot be used here, and an earlier draft of this file said it could.**
`gh api --slurp` is rejected outright when combined with `--jq`
("the `--slurp` option is not supported with `--jq` or `--template`"), so that command
failed with a usage error rather than returning a boolean. Pipe the paginated raw pages
to `jq -s` instead — `--paginate` emits one JSON array per page, `-s` wraps them, which is
why the filter iterates `.[][]`. **Verified against #13, and the two stages report differently — say which you mean:** the
`jq` stage prints the count, `0` for the merged head `92de0d2` and `1` for `eed589c`, the
commit actually reviewed; the full pipeline prints nothing and communicates through its
**exit status**, `1` for the merged head and `0` for the reviewed commit. That inversion
is deliberate — exit 0 means "a qualifying review exists", so the pipeline can gate a
merge directly. Documenting
the unrun form broke this repo's own rule against documenting a command nobody ran.

**Four observations now, and the fourth is the one that mattered.** #12 and #13 both
*merged* heads that were never reviewed — the miss was only found afterwards. On **#14**
the check said `pass` while the comment said "Review rate limited", and this query
returned `0` for head `787dd9a` **before** the merge: the re-trigger produced nothing, and
the PR merged on an explicit human decision with the exception recorded, the unreviewed
delta being a one-word prose correction the reviewer had itself requested. That is the
intended shape — the check is a signal you block on, this query is what tells you whether
a review actually happened, and when they disagree a human decides.

`--paginate` matters: without it only page one is read, so a qualifying review can sit on
page two and be read as absent. `jq -s` is what slurps the pages — `gh api --slurp` cannot
do it here, being rejected outright when combined with `--jq`. `DISMISSED` is excluded — a dismissed review is not
a review of that head. If no qualifying record exists, re-trigger once; if it is still
absent, **merge only on an explicit human decision**, recording that the head went
unreviewed.

What was actually measured, stated exactly: the rate-limit warning appeared in the **issue
comment**, while the review record was an earlier completed review of an earlier commit.
So the demonstrated failure is a *missing* review for the final head — which the
`commit_id` comparison catches. No rate-limited *review record* has been observed; the
body filter is bounded defensive filtering, not a check against something seen.

**Greptile has no signal you can block on**, across four PRs: `gh pr checks` displayed a
"Greptile Review" entry for #4 and #5, while the check-runs and statuses APIs return no
Expand Down
Loading