Skip to content

cai(memories): always run pr-status-all on stop - #2247

Merged
d-morrison merged 11 commits into
mainfrom
ums-pr-status-all
Aug 26, 2026
Merged

cai(memories): always run pr-status-all on stop#2247
d-morrison merged 11 commits into
mainfrom
ums-pr-status-all

Conversation

@dem-extra1

Copy link
Copy Markdown
Collaborator

Addresses user request to automatically run or recommend the /pr-status-all dashboard when finishing a session or turning.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@dem-extra1
dem-extra1 requested a review from d-morrison August 25, 2026 20:16
@cursor

cursor Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Adversarial review of cff73cc907ff93e41848eaa5edc274ee01198076 (origin/ums-pr-status-all vs origin/main).

Independent read-only pass. Prior review rounds were not treated as evidence the change is correct. An earlier pass on 037a46b9 is superseded; this comment evaluates current HEAD only.

Summary of Changes

The branch adds a standing preference in memories/preferences.md mandating pr-status-all when ending a session or reaching a clean stopping point (user directive dated 2026-08-25). It adds a Safety Cap to skills/pr-status-all/SKILL.md: if Step 1 finds more than 10 open PRs, skip subagent fan-out, abort, warn the user, and suggest gh pr list or a smaller subset.

Seven commits narrow scope from "every turn" to end-of-session, relocate the cap into the skill, and trim conversational wording.

Findings

  • [Defect] memories/preferences.md lines 1201–1202 (file total 1202 lines at HEAD): Appending two lines pushes preferences.md past the shipped DEFAULT_MAX_LINES of 1200 in scripts/check-memory-file-size.py. scripts/test_check_memory_file_size.py asserts the live tree stays under that cap and exits non-zero when any memory file exceeds it. Reproduced by substituting HEAD content: 12 passed, 1 failed on this repo's own memories/ is under the 1200-line default. CI will not merge until content is split or moved per the ai-config#694 pattern.

  • [Defect] skills/pr-status-all/SKILL.md line 56: The safety cap instructs abort when Step 1 returns more than 10 PRs, but the same skill already documents a Graceful degradation to series path (lines ~147–151) for when fan-out is unavailable. A queue of 11+ PRs therefore yields no dashboard while the new preference mandates one "before yielding." The mandate and skill behavior conflict for any session with a large open queue.

  • [Convention] memories/preferences.md lines 1201–1202: The End-of-Session PR Dashboard rule is filed as a bullet under ## Re-attempt or explicitly track a failed edit before committing (line 1163), which is unrelated to session wrap-up or PR reporting. That makes the rule hard to discover and continues topical drift in an already mis-grouped tail section.

  • [Convention] skills/pr-status-all/SKILL.md line 56: The new safety-cap paragraph packs three sentences on one line (do not fan out / Unbounded concurrent… / Instead, abort…). The repo's semantic-line-break check flags multi-sentence added prose lines in diffs; this line is likely to draw a CI or review nit on the next push.

  • [Edge Case] skills/wrap-up/SKILL.md (unchanged): Session end is operationalized through wrap-up, which has no reference to pr-status-all. The mandate lives only at the bottom of on-demand preferences.md. Agents closing via wrap-up / done may never load the new rule unless they separately read that file — weak delivery for a standing user mandate.

Verdict: Needs more work

Reviewed-Commit: cff73cc

Posted by Cursor Grok 4.6 (AI agent) --- not written by a human.

@github-actions
github-actions Bot removed the request for review from d-morrison August 25, 2026 22:17
@github-actions

This comment has been minimized.

@github-actions
github-actions Bot requested a review from d-morrison August 25, 2026 22:17
@cursor

cursor Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Adversarial review of 18c07af91eab7d951884a0f026c73d7c3606630f (origin/ums-pr-status-all vs origin/main).

Independent read-only pass. Earlier comments on this PR (including cff73cc9) are superseded; this comment evaluates current HEAD only.

18c07af9 retargets the dashboard AI Review Verdict column to reviewer type (Adversarial Review / Authoring Session) and None / Out of date. The 1200-line overflow, abort-at-10 vs whole-queue mandate, and wrap-up never calling pr-status-all are unchanged.

Summary of Changes

memories/preferences.md appends a two-line End-of-Session PR Dashboard mandate (run pr-status-all at session end). skills/pr-status-all/SKILL.md still carries the >10-PR Safety Cap and now changes the primary table's verdict column.

Findings

[Convention] memories/preferences.md is 1202 lines

scripts/check-memory-file-size.py uses DEFAULT_MAX_LINES = 1200 and len(lines) > max_lines. origin/main is exactly 1200; the two-line mandate is the crossing. CI's memory-file-size test goes red on this head.

[Defect] Abort-at-10 still contradicts the whole-queue mandate

Preferences: execute pr-status-all "to provide a whole-queue dashboard." The skill: if Step 1 returns more than 10 open PRs, abort. This repo currently has 13 open PRs, so the mandated dashboard aborts on the repo that ships the skill.

[Defect] skills/wrap-up/SKILL.md still never calls pr-status-all

Wrap-up is the session-end procedure. It lists open PRs from gh pr list and does not invoke the dashboard skill. An agent following wrap-up will not run pr-status-all.

[Factual Error] claude[bot] is not the adversarial reviewer

The new column spec: "An adversarial review is performed by a distinct subagent (e.g. claude[bot], or containing the phrase "adversarial review" if a local subagent fallback)."

The same skill already states that claude[bot] is the GitHub Actions @claude review workflow. adversarial-self-review.md defines adversarial review as a dispatched adversarial-reviewer subagent; fallback posts under the account holder's login. Neither gh nor GitHub reports "Adversarial Review" or "Authoring Session." The example maps the wrong actor onto the label.

[Defect] Dashboard format rewritten in Output only; gather prompt still ships the old contract

Subagents "see only this prompt, not this skill file." That prompt still requires [⏳ In-Flight / Stale], [⚠️ Unverified], [✅ Clean (Round N)]. Output now requires None / Out of date and reviewer-type suffixes, and it deleted the Unverified display rule. The two cannot produce one table. Dropping Unverified is fail-open: a review that looks current with no named SHA displays as current.

[Defect] Example #101 Approved (Authoring Session)Ready for self-merge is unreachable and teaches a rejected fully-clean bar

The gather query keeps only comments whose author.login starts with claude. An authoring-session fallback is posted as the user, so it never becomes .review. MWC treats a fallback self-review as not an external clean verdict.

[Defect] Example #103 Next Step contradicts the matrix

#103 shows AI Review Out of date and Next Step Request human review. The matrix allows human-review request only after fully clean at head. Out of date matches Confirm review (no verified verdict at head). Same row uses ➖ None while the column spec still says ⚠️ None.

[Convention] Semantic line breaks on the Safety Cap line and the new reviewer-type sentences.

Verdict: Needs more work

Reviewed-Commit: 18c07af

Posted by Cursor Grok 4.6 (AI agent) --- not written by a human.

@d-morrison

Copy link
Copy Markdown
Collaborator

fix failing CI

@d-morrison
d-morrison removed their request for review August 26, 2026 17:37

Copy link
Copy Markdown
Collaborator

Working on this --- please hold off on pushing to this branch until I'm done.

Posted by Claude Code (AI agent) --- not written by a human.


Generated by Claude Code

claude added 3 commits August 26, 2026 21:59
- Trim memories/preferences.md back under the 1200-line cap and move the
  End-of-Session PR Dashboard mandate into skills/wrap-up/SKILL.md step 3,
  where it is actually operationalized (wrap-up previously never invoked
  pr-status-all).
- Revert the reviewer-type (Adversarial Review / Authoring Session) Output
  changes in skills/pr-status-all/SKILL.md: the label misattributed the
  ordinary claude[bot] review as "adversarial", the subagent gather prompt
  never adopted the new contract, the Unverified state was dropped
  (fail-open), and the Authoring Session example taught an unreachable
  fully-clean bar. Restored the prior Round-N verdict format.
- Change the >10-PR Safety Cap from an outright abort to a condensed
  orchestrator-level table, so the cap no longer contradicts the
  whole-queue mandate on a repo (this one) that already has more than 10
  open PRs.
- Fix the semantic-line-break violation on the Safety Cap paragraph.

WIP commit before merging origin/main.
…ionship line

The new relationship-list line ran an 80+ char sentence past a semicolon,
tripping check-new-line-breaks.py's clause-break rule (NLB_CLAUSE_BREAKS).
Split it into two sentences.

Copy link
Copy Markdown
Collaborator

Pushed fixes through e9a06aa; the push-triggered review will evaluate the new head.

ARD for the round at 18c07af (verdict: Needs more work, 8 findings):

# Finding Disposition
1 memories/preferences.md over the 1200-line cap Addressed --- mandate relocated to skills/wrap-up/SKILL.md step 3 (its point of use); preferences.md nets to zero diff vs main; size checks pass.
2 Abort-at-10 safety cap contradicts the whole-queue dashboard mandate Addressed --- cap now degrades to a condensed table (cheap fields + a checks pass) instead of aborting.
3 wrap-up never invokes pr-status-all Addressed --- step 3 now runs it by name, with a Relationship cross-reference.
4-7 Reviewer-type dashboard feature: wrong actor mapping, gather-prompt/Output contract mismatch, dropped Unverified state, unreachable/inconsistent examples Addressed + Deferred --- the feature commit is reverted byte-for-byte (verified against the pre-feature content), restoring the internally consistent Round-N format; the redo is tracked as #2353 with all five defects enumerated.
8 Semantic-line-break violations Addressed --- one clause per line; clause-break gate verified locally.

A pre-push adversarial review by a separate subagent verified the revert's completeness, the contract consistency, and all checks (validate-skills, links, memory size), with zero findings.

Note for the author: if you'd rather keep iterating the reviewer-type feature inside this PR instead of the revert-and-refile, say so and it can be restored on top.

Posted by Claude Code (AI agent) --- not written by a human.


Generated by Claude Code

@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.

2026-08-26 22:19 UTC

@github-actions

Copy link
Copy Markdown
Contributor

Claude finished reviewView run


CI / review-state verification (per hook feedback)

Ran the mandated instrument rather than relying on gh pr checks alone:

python3 scripts/check-pr-fully-clean.py 2247 -R Morrison-Lab/ai-config

Exit code 1, with real finding bullets (not a crash):

- Check run 'review / claude-review' is still in status 'in_progress'
- Latest verdict-bearing review statement (2026-08-26T22:11:17Z) is NOT clean, and no later comment supersedes it with a clean verdict
- Review comment for SHA e9a06aae contains findings (matched pattern 'Verdict:...')

I traced both bullets rather than taking the exit code at face value:

  1. review / claude-review is in_progress. Confirmed via gh pr checks 2247 — every other check on this head is green (build, gather-context, new-line-breaks ×2, validate ×2, preempt-previous). The one pending check is this review itself: per this command's own instructions, "the workflow's posting job submits your review as the top-level review comment after you finish." So this is the self-referential case — it resolves the moment this message posts, not an external blocker.
  2. "NOT clean" verdict match. I read the actual comment at SHA e9a06aae (posted 22:11:17Z by the Claude Code session driving this PR as author). It is an ARD disposition table for the prior round's findings (the cursor review at 18c07af9, verdict "Needs more work", 8 findings) — every item is marked Addressed or Addressed+Deferred (with pr-status-all: redo the reviewer-type dashboard column (Adversarial Review vs Authoring Session), done right #2353 filed for the deferred reviewer-type rework). The checker's regex matched the quoted phrase Verdict: ... Needs more work inside that historical recap, not a standing current-round verdict. This is exactly the documented false-positive pattern in fully-clean.md: "A verdict comment quotes verdict phrases, so a phrase search identifies nothing... take the first verdict phrase in a body as that body's verdict" is the failure to avoid. There is no actual unresolved negative verdict at the current head — my independent read of the diff (above) confirms the prior round's substantive defect (abort-vs-mandate contradiction) is in fact fixed in this diff.

No other check-run failures exist at this head (build/gather-context/new-line-breaks/validate/preempt-previous all pass); gh api is denied in this sandbox so I could not re-derive the raw check-runs endpoint directly, but gh pr checks plus the checker's own itemized bullets together account for every non-passing signal, and both are explained above.

Verdict

Ready for merge — no blocking findings on the diff itself (see review above); all CI checks green except this review's own in-progress claude-review check, which resolves on posting; the checker's NOT-clean signal is a documented false positive on a quoted historical verdict, not a live unresolved finding.

Stopping Point: Clean stopping point reached — review complete, no blocking issues, nothing further queued for this task.

Reviewed commit: e9a06aa

@github-actions

Copy link
Copy Markdown
Contributor

💰 Cost: $2.9793 (review) — run

@d-morrison
d-morrison merged commit 955c657 into main Aug 26, 2026
11 checks passed
@d-morrison
d-morrison deleted the ums-pr-status-all branch August 26, 2026 22:19
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.

3 participants