fix: prevent duplicate MFA notifications across concurrent sessions#14
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughIntroduces a cross-process locking mechanism for auto-login using timestamp-based lockfile coordination, replacing the previous filesystem cooldown approach. Updates auto-login initiation in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #14 +/- ##
===========================================
- Coverage 100.00% 97.53% -2.47%
===========================================
Files 2 2
Lines 133 162 +29
Branches 31 37 +6
===========================================
+ Hits 133 158 +25
- Misses 0 4 +4
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@aws-auth-refresh.mjs`:
- Around line 39-48: The wait loop currently only checks getRemaining(config)
which can be null if metadata is missing, causing a timeout even after another
session refreshed credentials; update the loop in aws-auth-refresh.mjs (where
getRemaining, formatTime, execSync, process.exit are used) to perform an STS
fallback when getRemaining(config) is falsy: call AWS STS GetCallerIdentity
(using the existing AWS SDK import or `@aws-sdk/client-sts`) to verify whether
current credentials are valid, and if that call succeeds treat it as a fresh
refresh (log using the same message with formatTime and call process.exit(0));
ensure STS errors are caught/ignored so the loop continues sleeping via
execSync("sleep 3") until timeout.
- Line 47: The code calls execSync("sleep 3") (line uses execSync("sleep 3"))
which is platform-dependent and should be replaced with a Node.js native delay;
change the surrounding function (where execSync is invoked) to be async and
replace the sleep call with an await new Promise(resolve => setTimeout(resolve,
3000)) (or an equivalent native timer) instead of any Atomics.wait busy-wait
approach, ensuring the function signature and callers are updated to handle the
async/await change.
In `@lib.mjs`:
- Around line 117-126: The tryAcquireAutoLoginLock function can fail when the
parent directory of LOCK_FILE doesn't exist; before calling writeFileSync(..., {
flag: "wx" }) ensure the lock directory exists by creating the parent directory
(use the dirname of LOCK_FILE) with recursive=true. Update
tryAcquireAutoLoginLock to create the directory prior to the writeFileSync call
so writeFileSync won't throw ENOENT when the ~/.config/cc-aws-keepalive
directory is missing.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 765cf64c-418f-45a3-8752-c1a7650ad5af
📒 Files selected for processing (4)
.gitignoreaws-auth-refresh.mjsaws-cred-check.mjslib.mjs
Add atomic cross-process lock so only one auto-login runs system-wide. Multiple CC sessions hitting 403 simultaneously no longer each spawn their own expect script — the first acquires the lock, others either skip (proactive) or poll waiting for fresh creds (reactive).
- Ensure state directory exists before lock creation (ENOENT fix)
- Replace execSync("sleep 3") with Atomics.wait (cross-platform)
- Add STS fallback in wait loop when getRemaining returns null
GeiserX
force-pushed
the
fix/cross-process-lock
branch
from
430e5e8 to
e5d6049
Compare
Add 11 new tests covering: - tryAcquireAutoLoginLock (fresh, held, stale) - releaseAutoLoginLock (exists, missing) - sleepSync timing - loadConfig validation branches (unknown keys, numeric coercion, non-numeric string, non-string type, autoLoginMinutes warning)
GeiserX
force-pushed
the
fix/cross-process-lock
branch
from
e5d6049 to
57c0aa3
Compare
1 participant
Summary
~/.config/cc-aws-keepalive/.auto-login.lock) usingO_CREAT|O_EXCLso only one auto-login runs system-wideaws-cred-check.mjs): if lock held, silently skips — another session is already handling itaws-auth-refresh.mjs): if lock held, polls every 3s waiting for the other session's login to finish, then piggybacks on the refreshed credscoverage/to.gitignoreProblem
Multiple CC sessions hit Bedrock 403 simultaneously → each independently spawned the expect script → multiple MFA notifications (6-7 overnight with just 2 sessions open).
Test plan
npm test)Summary by CodeRabbit
Release Notes
New Features
Chores