Repository navigation
feat(dev-workflow): add prmode parameter and reduce stop hook noise - #235
Merged
Merged
Conversation
Add --prmode create/update to complete-task with validation: - create fails if PR exists, update fails if no PR - update requires --feedback-items-resolved and --feedback-items-remaining - remaining > 0 rejected to avoid expensive round-trip costs - --reject-review-feedback restricted to update mode only Reduce stop hook output from 20 lines to 1-line file reference. Add Stop Checklist and Complete Task Modes sections to task-workflow.md. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Tests already run before complete-task, during pre-commit hook, and on PR CI checks. Running them in verify-build is redundant. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Deploying living-architecture with
|
| Latest commit: |
c064e4a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://130da7ea.living-architecture.pages.dev |
| Branch Preview URL: | https://polish-complete-task.living-architecture.pages.dev |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. 📝 WalkthroughWalkthroughAdds explicit PR submission modes ("create" and "update") to the complete-task workflow, extends the complete-task context/schema with prMode and feedback counts, routes CI watching through a new ghCli.watchCI, refactors submit-pull-request to resolve/create PRs by mode, and simplifies Stop handler messaging and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as Client CLI
participant CT as CompleteTask Command
participant GH as GitHub REST Client
participant GHCLI as gh CLI Wrapper
participant CI as CI System
CLI->>CT: invoke complete-task --prmode=create|update + flags
CT->>GH: check/get existing PR (if update)
alt create mode
CT->>GH: create PR (title/body)
GH-->>CT: PR info
else update mode
GH-->>CT: existing PR info
end
CT->>GHCLI: watchCI(prNumber)
GHCLI->>CI: poll/check PR checks
CI-->>GHCLI: checks status (pass/fail)
GHCLI-->>CT: CIResult {failed, output}
CT->>CLI: report outcome / next steps
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.claude/commands/complete-task.md:
- Around line 29-35: Update the verification bullet in the complete-task command
description to reflect that verify-build no longer runs tests: change the line
that currently reads "Runs local verification (lint, typecheck, test)" to say
"Runs local verification (lint, typecheck)" and ensure any surrounding text
mentioning verify-build or local verification is consistent with this change
(look for the phrase "verify-build" and the bullet under the 10-minute timeout
header to locate the content).
In `@docs/workflow/task-workflow.md`:
- Around line 194-200: The docs state that Create Mode (`--prmode create`)
"Requires `--pr-title`" too strictly; update the wording around the `--prmode
create` section (the example for pnpm nx run dev-workflow:complete-task --
--prmode create --pr-title ...) to clarify that `--pr-title` is required only
when the task/PR is not backed by an existing issue or task, and that for
issue-backed tasks PR title/details are derived automatically and `--pr-title`
is optional; keep the example but add a note explaining the conditional
requirement.
In `@tools/dev-workflow/features/complete-task/commands/complete-task.spec.ts`:
- Around line 275-285: The test "context builder rejects non-integer
feedback-items-remaining" mismatches its setup: mockCli.requireArg currently
returns an invalid value for '--feedback-items-resolved' instead of
'--feedback-items-remaining'; update the mockCli.requireArg implementation used
in this test (the block around executeCompleteTask()/getContextBuilder()) to
return 'abc' for the '--feedback-items-remaining' flag (or alternatively rename
the test to reference '--feedback-items-resolved') so the input and test name
align; ensure you reference the same call sites (mockCli.requireArg,
executeCompleteTask, getContextBuilder) when making the change.
In `@tools/dev-workflow/features/complete-task/commands/complete-task.ts`:
- Around line 33-48: Replace the manual validation in parsePRMode and
parseNumberArg with Zod safeParse usage: export and import the existing
prModeSchema from task-to-complete.ts and use
prModeSchema.safeParse(cli.requireArg('--prmode')) inside parsePRMode to
validate/throw WorkflowError on parse.error; create and export a Zod schema for
non-negative integers (e.g., nonNegIntSchema) and use
nonNegIntSchema.safeParse(cli.requireArg(flag)) inside parseNumberArg to
validate/throw WorkflowError on parse.error, removing the Number.isInteger/ < 0
manual checks.
…k watching Custom Octokit polling only checked Check Runs API, missing commit statuses (used by CodeRabbit). New approach uses gh CLI which handles both. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Update mode reuses existing PR — no title/body needed. Co-Authored-By: Claude Opus 4.5 <[email protected]>
…tion gh CLI exits with error when checks haven't registered yet after push. Retry up to 6 times with 10s delay. Also makes PR title/body optional in update mode since existing PR doesn't need them. Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@tools/dev-workflow/platform/infra/external-clients/gh-cli.spec.ts`:
- Around line 73-83: The test for ghCli.watchCI doesn't assert that the retry
loop ran the expected number of times; update the spec to assert
mockExecFileSync was called exactly MAX_RETRIES times (or 6 if MAX_RETRIES
constant is 6) after the failure case. Locate the test using mockExecFileSync
and ghCli.watchCI and add an expectation verifying
mockExecFileSync.mock.calls.length (or toHaveBeenCalledTimes) equals the retry
constant (MAX_RETRIES) so regressions in the retry logic are caught.
In `@tools/dev-workflow/platform/infra/external-clients/gh-cli.ts`:
- Around line 3-6: Export the CIResult interface so consumers can import the
return type of the public watchCI function; locate the CIResult declaration in
gh-cli.ts and add an export to it (i.e., change/interface name CIResult to
exported CIResult) so callers of ghCli.watchCI can reference the type, ensuring
any other public types used by watchCI are also exported if needed.
- Update verify bullet to reflect lint+typecheck only (no test) - Clarify --pr-title conditional requirement in docs - Fix test data mismatch for feedback-items-remaining - Use Zod safeParse for prMode and number validation - Add retry count assertion to CI check tests - Export CIResult interface for consumers Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tools/dev-workflow/features/complete-task/commands/complete-task.ts`:
- Around line 133-138: The check in resolveSkipReview currently compares
cli.parseArg('--prmode') as a raw string; replace that with a call to
parsePRMode() so the value is validated by the Zod schema. Specifically, inside
resolveSkipReview call parsePRMode() to get prMode, and if hasFlag && prMode !==
'update' throw the existing WorkflowError('--reject-review-feedback can only be
used with --prmode update'); ensure parsePRMode is imported/available in the
file and let any validation errors propagate as before.
When mergeableState is not 'clean', fetches check runs and includes failedChecks array showing which checks failed or are pending. Prevents misdiagnosing 'blocked' state as human review requirement. Co-Authored-By: Claude Opus 4.5 <[email protected]>
No consumers yet — re-export when needed. Co-Authored-By: Claude Opus 4.5 <[email protected]>
…ation Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Summary by CodeRabbit
New Features
Documentation
Changes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.