Skip to content

ums: note gh pr update-branch behavior - #2250

Merged
d-morrison merged 5 commits into
mainfrom
ums/gh-pr-update-branch
Aug 26, 2026
Merged

ums: note gh pr update-branch behavior#2250
d-morrison merged 5 commits into
mainfrom
ums/gh-pr-update-branch

Conversation

@d-morrison

Copy link
Copy Markdown
Collaborator

Captured learning during session wrap-up.

@github-actions

This comment has been minimized.

@cursor

cursor Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Adversarial review of bac5fc0a90289721636284ba20eff62fea9b627d (origin/ums/gh-pr-update-branch vs origin/main).

Independent read-only pass. Prior review rounds were not treated as evidence the change is correct.

Summary of Changes

Adds a seven-line section at the end of memories/github.md documenting gh pr update-branch: default behavior merges the base into the PR branch (merge commit), and that update retriggers CI and automated review so the PR is not fully clean until new runs finish.

Single commit bac5fc0a (ums: note gh pr update-branch behavior).

Findings

  1. [Defect] memories/github.md lines 1202–1207 — file length crosses the enforced 1200-line cap.
    origin/main is exactly 1200 lines; the PR branch is 1207 (len(lines) > 1200 in scripts/check-memory-file-size.py).
    scripts/test_check_memory_file_size.py asserts the live tree stays under that default and exits 1 on any oversize file, so this merge would fail CI validation unless the file is split or content is condensed elsewhere first.

  2. [Convention] memories/github.md line 1204 — semantic line break.
    The added paragraph is one 224-character sentence on a single line.
    Repo guidance prefers clause-level breaks in new prose (shared/writing/semantic-line-breaks.md); this is advisory weight, not a CI gate by itself.

  3. [Convention] memories/github.md lines 1202–1207 — no provenance date.
    Adjacent sections often carry (Measured YYYY-MM-DD …) provenance; this block has none.
    shared/writing/timestamp-volatile-claims.md expects a check date on tool-behavior claims verified empirically (e.g. gh pr update-branch --help, checked 2026-08-25).

Fact-check (no findings):

  • gh pr update-branch --help and GitHub CLI manual: default updates with a merge commit; --rebase is the alternative. Matches the section title and body focus on default behavior.
  • GitHub docs: UI “Update branch” uses merge by default; rebase is optional. Consistent.
  • CI/review retrigger on branch update: accurate for typical pull_request synchronize workflows; prose correctly qualifies “pipelines … that run on push.”
  • No em-dash or non-ASCII punctuation issues.
  • No in-file duplication: gh pr update-branch is new; shared/workflow/sync-with-main.md covers local git merge origin/main and the UI button, not this CLI command.

Verdict: Needs more work

Reviewed-Commit: bac5fc0

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

@d-morrison
d-morrison force-pushed the ums/gh-pr-update-branch branch from bac5fc0 to ad4a841 Compare August 25, 2026 21:31
@d-morrison

Copy link
Copy Markdown
Collaborator Author

I have addressed the 3 findings from the adversarial review:

  1. github.md file length is now under the 1200-line limit (split out gh (GitHub CLI) to gh-cli.md).
  2. Applied semantic line breaks to the added paragraph.
  3. Added the provenance date.

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

@github-actions

This comment has been minimized.

@cursor

cursor Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Adversarial review of ad4a841877d693a986c3f1a1ce4c9ce32002841d (origin/ums/gh-pr-update-branch vs origin/main).

Independent read-only pass. The earlier comment on bac5fc0a is superseded; this comment evaluates current HEAD only.

Summary of Changes

Single commit ad4a8418 splits the ## gh (GitHub CLI) block (~615 lines) from memories/github.md into new memories/gh-cli.md (617 lines), trims github.md from 1200 lines to 597, adds a measured gh pr update-branch section, and registers gh-cli.md in memories/MEMORY.md.

Prior findings (1200-line cap, SemBr, missing provenance) are addressed in this tree. gh pr update-branch --help still matches the documented default (merge commit; --rebase optional). Split integrity: 540 lines from main github.md appear verbatim in gh-cli.md; relative links resolve.

Findings

  1. [Convention] memories/github.md lines 587–597 — The gh pr update-branch note stayed at the bottom of github.md after the split routed general gh behavior to gh-cli.md. An agent opening gh-cli.md for gh behavior will not see this note.

  2. [Convention] memories/MEMORY.md line 16 — The github.md row still leads with “The gh CLI” though most gh content now lives in gh-cli.md.

  3. [Edge Case] PR #2254 still carried superseded commit bac5fc0a (the old append-only 1207-line github.md) when last inspected. Merge order between ums: note gh pr update-branch behavior #2250 and chore: purge hardcoded usernames from instructions #2254 needs reconciliation if that ancestor is still on chore: purge hardcoded usernames from instructions #2254.

Verdict: Ready for merge

Convention items are routing polish, not CI or factual blockers.

Reviewed-Commit: ad4a841

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

@cursor

cursor Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

#2304 merged to main at 2b4f5a56 (2026-08-26T16:35:42Z) and this PR is now CONFLICTING.

The collision is memories/github.md: that squash rewrote the satellite-pointer intro to add github-closing-keywords.md. This session will not push to ums/gh-pr-update-branch. Fetch origin/main and keep both the new satellite pointer and this PR's gh pr update-branch notes.

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

@d-morrison

Copy link
Copy Markdown
Collaborator Author

Driving this PR to clean --- please hold off until done.

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

- Move the gh pr update-branch note from github.md to gh-cli.md, where
  general gh CLI content now lives (review finding #1).
- Update MEMORY.md's github.md row to stop leading with "The gh CLI"
  now that most gh content moved to gh-cli.md (review finding #2).
- Merge origin/main, pulling in the CheckRun/StatusContext state
  enumeration that landed there since this branch's base.
- Fix a stray blank line in MEMORY.md's table (introduced by the
  original split commit) that broke the markdown table between the
  gh-cli.md and github-mcp-tools.md rows.
- Drop a redundant duplicate H1/H2 heading pair at the top of
  gh-cli.md, also from the original split commit.

Review finding #3 (PR #2254 carrying a stale ancestor commit) no
longer applies -- #2254's current head is not a descendant of that
commit.
@d-morrison

Copy link
Copy Markdown
Collaborator Author

ARD summary --- round vs Cursor review on ad4a841

  1. [Convention] gh pr update-branch note stayed in github.md after the gh-cli.md split --- Addressed: moved it to gh-cli.md.
  2. [Convention] MEMORY.md's github.md row still led with "The gh CLI" --- Addressed: reworded to note most gh content now lives in gh-cli.md.
  3. [Edge Case] merge-order reconciliation with chore: purge hardcoded usernames from instructions #2254 --- Rebutted/moot: chore: purge hardcoded usernames from instructions #2254's current head (452dca8) is not a descendant of the superseded bac5fc0 commit, so no reconciliation is needed.

Also, while merging main in (this PR was CONFLICTING after #2304 landed):

  • Pulled in the newer CheckRun/StatusContext state-enumeration content that landed in github.md's GraphQL section since this branch's base, folding it into gh-cli.md so it isn't lost.
  • Fixed a stray blank line in MEMORY.md's table (from the original split commit) that broke table rendering between the gh-cli.md and github-mcp-tools.md rows.
  • Dropped a redundant duplicate H1/H2 heading pair at the top of gh-cli.md (same origin).

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

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

memories/gh-cli.md is a brand-new file from git's perspective (github.md
was trimmed, not deleted, so no rename pairing occurs), so the
diff-scoped new-line-breaks check treats all ~620 of its lines as
newly added and re-lints content that was already merged in its prior
location. Reformatted every flagged line (semantic line breaks per
sentence/clause) with content otherwise byte-for-byte unchanged --
verified by stripping all whitespace from both the pre-edit and
post-edit file and diffing (identical) -- and by running the actual
check-new-line-breaks.py script locally against origin/main (clean).

Filed the underlying tooling gap upstream as
Morrison-Lab/gha#684 (file splits will
always trip this diff-scoped check for migrated content; this is a
local workaround, not a fix to the checker).
@d-morrison

Copy link
Copy Markdown
Collaborator Author

ARD summary --- round vs claude-review at 640d270

  1. [Blocking] new-line-breaks check failing on the split content --- Addressed: confirmed the root cause (the reviewer's diagnosis matched mine independently) --- `gh-cli.md` is a brand-new file from git's perspective, so the diff-scoped checker treats all migrated content as newly added. Reformatted every flagged line to satisfy the check, verified against the actual `check-new-line-breaks.py` script run locally (clean) and against a whitespace-stripped diff confirming no content changed, only line breaks. Filed the underlying tooling gap upstream: check-new-line-breaks: flags pre-existing content as new when a PR moves it to a new file gha#684.
  2. Non-blocking observations (CI still in progress at review time, prior findings genuinely addressed) --- no action needed, confirmed correct.

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

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@d-morrison

Copy link
Copy Markdown
Collaborator Author

ARD summary --- round vs claude-review at 75b4f53 (comment 5430574955)

That review's own body says "Needs more work is not accurate to what I found in the diff" -- `check-pr-fully-clean.py`'s pattern match read this as a not-clean verdict (matching the negated phrase), but the actual substance is "no blocking defects found." All checks are now genuinely green (`gh pr checks`: 10/10 pass, including `review / claude-review` and `review / require-review`), which resolves the self-referential confusion the review itself flagged (it ran while its own check was still in-progress).

  1. [Non-blocking] "one non-blocking, pre-existing content-duplication issue... noted above" --- investigated, unable to locate: the posted comment references this without a file/line, and the text describing it isn't present in the delivered body (an apparent gap between the reviewer's internal reasoning and what it actually posted). Searched `memories/gh-cli.md` and `memories/github.md` for duplicated headers/bullets and found none. If a specific instance exists, please re-raise it with a location and I'll address it.

Requesting a fresh review now that CI has actually finished, since no code changed this round (investigation only) and the prior review's own uncertainty was about CI state that has since resolved.

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

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@d-morrison

Copy link
Copy Markdown
Collaborator Author

Done --- PR is free.

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

@d-morrison

Copy link
Copy Markdown
Collaborator Author

Driving this PR to clean --- please hold off on pushing to this branch until I'm done.

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

@github-actions

Copy link
Copy Markdown
Contributor

Claude finished reviewView run


Everything checks out cleanly. Let me finalize.

Review: #2250ums: note gh pr update-branch behavior

This is a fourth review round on this PR. Since the last review (Ready for merge, posted at 2026-08-26T20:23:30Z at HEAD 75b4f53d), the only new commit is 4a969680, a merge of origin/main into the branch.

What I independently verified about that merge commit:

  • The merge is clean and content-preserving. git diff 75b4f53d63 4a96968052 shows the merge pulled in 13 files' worth of unrelated main changes (Cursor Cloud dispatch docs, push/SKILL.md, adversarial-self-review.md, etc.) with no conflict markers left in any file (memories/MEMORY.md, memories/gh-cli.md, memories/github.md all checked directly).
  • No collision between this PR's edits and main's. main's own change to memories/MEMORY.md touched only the cursor.md row; this PR's change touches the github.md/gh-cli.md rows. The merge combined both non-overlapping edits correctly — both rows are present and correct in the merged result.
  • memories/gh-cli.md is byte-identical before and after the merge (diff on the file content across the merge: no output, exit 0) — the merge did not reintroduce the new-line-breaks regression that the prior round's 75b4f53d fix addressed.
  • Links and file size still pass at the current head: scripts/check-links.py reports 2455 links checked, 0 broken; scripts/check-memory-file-size.py reports no file over the 1200-line cap.
  • Live CI status (as of this review): every check that has completed is SUCCESS, including both new-line-breaks runs. mergeStateStatus: BLOCKED reflects that this review job and two validate matrix jobs were still IN_PROGRESS at query time — not a defect, just the expected pre-completion state.

No new findings. The diff-content assessment from the prior round (clean split, no hallucinated APIs, gh pr update-branch claim independently verified against gh help pr update-branch, no broken links, all prior structural findings fixed) still applies — the merge commit introduced nothing that changes that assessment.

Verdict

Ready for merge — no new findings from the merge commit; content preservation, links, and file-size checks all still pass, and the prior round's clean verdict stands.

Reviewed commit: 4a96968

Reviewed commit: 4a96968

@github-actions

Copy link
Copy Markdown
Contributor

💰 Cost: $1.8437 (review) — run

@d-morrison
d-morrison merged commit 26e4661 into main Aug 26, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant