Skip to content

feat: begin PR workflow hardening - #308

Merged
flyingrobots merged 8 commits into
mainfrom
feat/pr-workflow-hardening
Mar 25, 2026
Merged

feat: begin PR workflow hardening#308
flyingrobots merged 8 commits into
mainfrom
feat/pr-workflow-hardening

Conversation

@flyingrobots

@flyingrobots flyingrobots commented Mar 25, 2026

Copy link
Copy Markdown
Owner

Starts the PR-workflow improvement cycle with the first concrete slice:

  • add cargo xtask pr-status as the canonical repo-maintenance entrypoint for current-head PR status
  • keep make pr-status as a thin alias to that xtask surface
  • update workflow docs to point contributors at the xtask entrypoint
  • add xtask unit coverage for command construction and explicit script execution

Follow-up slices on this branch will cover the rest of the queued PR-process work.

Test Plan:

  • cargo fmt --all --check
  • cargo test -p xtask
  • bash tests/hooks/test_pr_status.sh
  • cargo xtask pr-status 306

Summary by CodeRabbit

  • New Features

    • Added cargo xtask pr-status command to display PR status with blocker detection
    • Added cargo xtask pr-threads command for managing review threads (list, reply, resolve actions)
    • Added cargo xtask pr-preflight command to verify PR readiness before submission
  • Documentation

    • Updated workflow documentation with new PR-focused commands and preflight verification guidance
  • Chores

    • Added markdownlint development dependency

@coderabbitai

coderabbitai Bot commented Mar 25, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@flyingrobots has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 5 minutes and 59 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2d3b3bdd-708c-4568-b194-1eef8e99992b

📥 Commits

Reviewing files that changed from the base of the PR and between a2ee2f5 and 88cf6d6.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • docs/workflows.md
  • xtask/src/main.rs
📝 Walkthrough

Walkthrough

Migrates PR management tooling from shell scripts to a Rust-based cargo xtask system, introducing three new subcommands (pr-status, pr-threads, pr-preflight) for PR status aggregation, review thread operations, and pre-submission validation.

Changes

Cohort / File(s) Summary
Build & Tooling Configuration
Makefile, package.json
Added pr-threads and pr-preflight phony targets routing to cargo xtask; migrated pr-status from shell script invocation to xtask dispatch. Added markdownlint-cli2 dev dependency.
PR Management - Core Implementation
xtask/src/main.rs
Implemented three new cargo xtask subcommands: pr-status (PR visibility), pr-threads with list/reply/resolve operations (review thread manipulation), and pr-preflight (multi-check validation gate). Extended lint-dead-refs with --file option for targeted markdown linting. Includes substantial unit tests for new helpers and command logic.
PR Management - Shell Enhancement
scripts/pr-status.sh
Added helper functions: bucket_lines (check extraction), join_lines_csv (list formatting), print_blockers (blocker aggregation). Refactored print_group to reuse extraction logic. New blocker detection aggregates unresolved threads, failing/pending checks, non-approved review state, and non-clean merge state.
Testing
tests/hooks/test_pr_status.sh
Added clean fixture and test assertions verifying blocker detection: presence of blockers in success scenario and - none detected message in clean scenario.
Documentation & Guidance
docs/workflows.md, scripts/hooks/README.md
Updated AGENTS.md reference path and expanded xtask entrypoints documentation. Documented new PR-focused commands with usage variants. Added "Pre-PR Preflight" section detailing validation scope (local gate, markdownlint, dead refs, runtime schema, feature checks, shell syntax) and explicit non-guarantees (CI matrix parity, merge readiness). Revised helper script guidance to prefer xtask/make over direct invocation.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant xtask as cargo xtask
    participant GH_CLI as gh cli
    participant GraphQL as GitHub API<br/>(GraphQL)
    
    User->>xtask: pr-threads list [--selector PR#]
    xtask->>GH_CLI: gh api graphql (fetch unresolved threads)
    GH_CLI->>GraphQL: graphql query
    GraphQL-->>GH_CLI: thread list
    GH_CLI-->>xtask: parsed threads
    xtask-->>User: display threads
    
    User->>xtask: pr-threads reply --comment-id X --body "msg"
    xtask->>GH_CLI: gh api (add reply)
    GH_CLI->>GraphQL: mutation
    GraphQL-->>GH_CLI: success
    GH_CLI-->>xtask: ack
    xtask-->>User: reply posted
    
    User->>xtask: pr-threads resolve --thread-ids [IDs] --yes
    xtask->>GH_CLI: gh api graphql (resolve threads)
    GH_CLI->>GraphQL: mutation
    GraphQL-->>GH_CLI: success
    GH_CLI-->>xtask: ack
    xtask-->>User: threads resolved
Loading
sequenceDiagram
    participant User
    participant xtask as cargo xtask<br/>pr-preflight
    participant LocalValidator as Local Validators
    participant ShellCheck as Shell & Lint
    participant SchemaValidator as Schema/Feature
    
    User->>xtask: pr-preflight [--base origin/main] [--full]
    xtask->>xtask: compute changed files (unless --full)
    xtask->>xtask: derive preflight scope & build checks
    
    xtask->>LocalValidator: execute local gate
    LocalValidator-->>xtask: pass/fail
    xtask->>ShellCheck: run markdownlint + dead-refs + bash -n
    ShellCheck-->>xtask: pass/fail
    xtask->>SchemaValidator: validate runtime schema + features
    SchemaValidator-->>xtask: pass/fail
    
    alt All checks pass
        xtask-->>User: success (no blockers)
    else Any check fails
        xtask-->>User: bail with failing labels
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested labels

tooling

Poem

🚀 From shell scripts bound, to Rust they ascend,
PR threads now reply, with logic to mend,
Preflight gates check before merge's big send,
Gates keep the guard—performance's best friend.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 34.38% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: begin PR workflow hardening' directly reflects the main changeset objective: introducing PR-workflow improvements through new xtask entrypoints and documentation updates.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/pr-workflow-hardening

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@flyingrobots
flyingrobots marked this pull request as ready for review March 25, 2026 06:15

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a2ee2f5633

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread xtask/src/main.rs
Comment thread xtask/src/main.rs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/workflows.md`:
- Around line 19-20: The docs/workflows.md text incorrectly points to repo-root
AGENTS.md; update the reference string "AGENTS.md" to the actual path
"docs/archive/AGENTS.md" so the sentence reads e.g. See docs/archive/AGENTS.md
for full details; modify the quoted path in the line containing `AGENTS.md` to
the new path to ensure the link points to the correct file.

In `@Makefile`:
- Around line 34-38: The Makefile targets pr-threads and pr-preflight are
vulnerable to ARGS word-splitting; change the recipe invocations to safely pass
the whole ARGS value (e.g., use `@bash` -lc 'cargo xtask pr-threads $(ARGS)' and
`@bash` -lc 'cargo xtask pr-preflight $(ARGS)' or otherwise ensure the shell
evaluates the embedded quotes) and add a short note in the Makefile comments
mentioning that multi-word --body values are better supplied via --body-file to
avoid quoting pitfalls; update references to the targets pr-threads,
pr-preflight and the variable $(ARGS).

In `@xtask/src/main.rs`:
- Around line 411-425: Track and report partial successes when resolving
multiple threads: maintain a counter (e.g., successes) while iterating over
targets and calling run_gh_capture; on each successful call for a thread (use
thread.thread_id and thread.display_location for logging) increment the counter
and print the per-thread success as now, but if run_gh_capture returns an Err,
print a summary like "Resolved X/Y threads before failure" (using successes and
targets.len()) and propagate the error; alternatively, perform the
run_gh_capture call inside a match/if let to detect Err and log the
partial-summary before returning Err from the function.
- Around line 898-903: The current is_gh_auth_error function overmatches because
lowered.contains("auth") will hit words like "author" or "authentic"; update the
detection to use tighter patterns: remove the generic contains("auth") check and
instead detect whole-word "auth" or specific phrases such as "authentication",
"not logged in", "to authenticate", "you must authenticate", or "authentication
required" (use a regex with word boundaries or split/token checks). Reference
the function is_gh_auth_error and replace the simple substring check with either
a regex (e.g., word-boundary patterns) or explicit phrase matches to avoid false
positives.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c54a1c84-ebf9-4b7f-9235-cbb76540942c

📥 Commits

Reviewing files that changed from the base of the PR and between 729041c and a2ee2f5.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (7)
  • Makefile
  • docs/workflows.md
  • package.json
  • scripts/hooks/README.md
  • scripts/pr-status.sh
  • tests/hooks/test_pr_status.sh
  • xtask/src/main.rs

Comment thread docs/workflows.md Outdated
Comment thread Makefile
Comment thread xtask/src/main.rs Outdated
Comment thread xtask/src/main.rs
@flyingrobots
flyingrobots merged commit e4d8a29 into main Mar 25, 2026
31 checks passed
@flyingrobots
flyingrobots deleted the feat/pr-workflow-hardening branch March 25, 2026 07:38
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