Treat CodeRabbit login timeout as fatal auth (#43)#45
Conversation
There was a problem hiding this comment.
Sorry @pandalump, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Summary
WalkthroughExtend ODW authentication classifiers to recognise CodeRabbit and Codex login-timeout and authentication-completion messages as fatal auth failures. Add assessment, classifier, and host-review tests covering these mappings while preserving benign timeout classification. ChangesODW authentication classification
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (18 passed)
📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.3)workflows/df12-build-odw.jsFile contains syntax errors that prevent linting: Line 3866: Illegal return statement outside of a function Comment |
CodeRabbit's non-interactive login can abandon with "Automatic login timed out. Use the printed fallback URL to finish authentication." This message previously fell through to a deferred 'error' outcome, letting the review stage continue past an unrecoverable authentication failure. Broaden authFailureDetail with two tightly-scoped, login/authentication- anchored patterns so the message classifies as 'auth', routing it through the existing fatal-auth halt. Generic "timeout" prose is left unmatched. Cover the new behaviour with a positive and a negative classifier row in the faults module tests, an outcome-level test in the host-review module tests, and mirrored rows in the compiled-bundle assessment suite. Regenerate the committed workflows/df12-build-odw.js artefact. Closes #43
8356768 to
2c13ba0
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c13ba0d0a
ℹ️ 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".
| /\blogin tim(?:ed out|eout)\b/i, | ||
| /\bfinish authentication\b/i, |
There was a problem hiding this comment.
Constrain the login-timeout match to CodeRabbit errors
When an implementation legitimately reports something like “Raised the login timeout to 30 seconds” or “Finish authentication before redirect”, either new regex matches. Because this shared matcher is also applied to implementation summaries and open issues by implementationAuthFailureDetail, run-task.ts then returns fatal-auth, halting the workflow instead of integrating the completed task. Match the complete CodeRabbit abandonment message, or require CodeRabbit error context, rather than treating these standalone phrases as authentication failures.
Useful? React with 👍 / 👎.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@tests/modules/faults.test.ts`:
- Line 37: Update the fault test table in tests/modules/faults.test.ts to add a
separate `login timeout` input row expecting the authentication category and
retain the existing `timed out` case with its current message expectation,
covering both production alternatives.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3212666e-ddbf-423e-8d34-e81c14b52336
📒 Files selected for processing (5)
src/workflows/df12-build-odw/faults.tstests/df12-build-odw-assessment.test.mjstests/modules/faults.test.tstests/modules/host-review.test.tsworkflows/df12-build-odw.js
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/nixie(auto-detected)
| ['coderabbit login hint', 'Run `coderabbit auth login` to authenticate', 'auth'], | ||
| ['logged-in false JSON', '{"loggedIn": false}', 'auth'], | ||
| ['token expired', 'the token expired yesterday', 'auth'], | ||
| ['coderabbit login timeout', 'Automatic login timed out. Use the printed fallback URL to finish authentication.', 'auth'], |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Cover the literal login timeout branch.
Add a separate table row using login timeout; the current case exercises only timed out, leaving the second production alternative unverified.
- ['coderabbit login timeout', 'Automatic login timed out. Use the printed fallback URL to finish authentication.', 'auth'],
+ ['coderabbit login timeout', 'login timeout', 'auth'],
+ ['coderabbit login timed out', 'Automatic login timed out. Use the printed fallback URL to finish authentication.', 'auth'],🤖 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 `@tests/modules/faults.test.ts` at line 37, Update the fault test table in
tests/modules/faults.test.ts to add a separate `login timeout` input row
expecting the authentication category and retain the existing `timed out` case
with its current message expectation, covering both production alternatives.
Summary
CodeRabbit's non-interactive login can abandon with the message "Automatic
login timed out. Use the printed fallback URL to finish authentication." Until
now this fell through
authFailureDetailto a deferred'error'outcome,letting the review stage continue past an unrecoverable authentication
failure instead of halting.
This change broadens
authFailureDetailinsrc/workflows/df12-build-odw/faults.tswith two tightly-scoped regexesanchored on the login/authentication context (
login timed out/login timeoutandfinish authentication), so the message classifies as'auth'and rides the existing
'auth'→ fatal-auth halt path. No new outcomes,branches, or signatures are introduced. Generic "timeout" prose remains
unmatched.
Tests
tests/modules/faults.test.ts: positive classifier row (coderabbit login timeout) and a negativebenign timeout proserow.tests/modules/host-review.test.ts: outcome-level test asserting alogin-timeout error event classifies as
'auth'.tests/df12-build-odw-assessment.test.mjs: mirroredauthFailureDetailsurface assertion and
classifyCoderabbitOutcomerow for the compiledbundle.
The generated
workflows/df12-build-odw.jsartefact was regenerated viamake workflow-buildand committed alongside the source and tests.Commit gates (format, lint, typecheck, module and behavioural tests,
freshness) are pending orchestrator validation.
Closes #43