Open
Conversation
…elpers-6' into release/mainbound-20260323
…ged-helper' into release/mainbound-20260323 # Conflicts: # index.ts # lib/runtime/account-pool.ts # lib/runtime/account-selection.ts # lib/runtime/manual-oauth-flow.ts # lib/runtime/ui-runtime.ts # lib/storage.ts # lib/storage/named-backups.ts
…g' into release/mainbound-20260323 # Conflicts: # README.md # docs/README.md
…' into release/mainbound-20260323 # Conflicts: # README.md # docs/README.md
…ooks' into release/mainbound-20260323 # Conflicts: # docs/README.md
…lease/mainbound-20260323 # Conflicts: # docs/README.md
…orts-latest' into release/mainbound-20260323
…' into release/mainbound-20260323 # Conflicts: # docs/reference/commands.md # lib/codex-manager.ts
…ease/mainbound-20260323 # Conflicts: # lib/codex-manager.ts
…est' into release/mainbound-20260323
…ard-smoke' into release/mainbound-20260323
…n' into release/mainbound-20260323
…oke' into release/mainbound-20260323
…er-smoke' into release/mainbound-20260323
…-smoke' into release/mainbound-20260323
…-smoke-real' into release/mainbound-20260323 # Conflicts: # test/benchmark-runtime-path-script.test.ts
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Contributor
|
Important Review skippedToo many files! This PR contains 184 files, which is 34 over the limit of 150. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (184)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+95
to
+97
| const newStorage: AccountStorageV3 = { | ||
| version: 3, | ||
| accounts: deduplicatedAccounts, |
There was a problem hiding this comment.
importedCount can go negative with dirty existing storage
const importedCount = deduplicatedAccounts.length - existingAccounts.length;
const skippedCount = params.imported.accounts.length - importedCount;
deduplicateAccounts runs on the full merged array. if existingAccounts itself contains internal duplicates, deduplicatedAccounts.length < existingAccounts.length, making importedCount negative and skippedCount > params.imported.accounts.length. in practice storage is always written post-dedup, but this silently produces wrong counts for the caller if the invariant is violated.
consider clamping to zero or asserting the invariant:
const importedCount = Math.max(
0,
deduplicatedAccounts.length - existingAccounts.length,
);Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/storage/import-export.ts
Line: 95-97
Comment:
**`importedCount` can go negative with dirty existing storage**
```
const importedCount = deduplicatedAccounts.length - existingAccounts.length;
const skippedCount = params.imported.accounts.length - importedCount;
```
`deduplicateAccounts` runs on the full merged array. if `existingAccounts` itself contains internal duplicates, `deduplicatedAccounts.length < existingAccounts.length`, making `importedCount` negative and `skippedCount > params.imported.accounts.length`. in practice storage is always written post-dedup, but this silently produces wrong counts for the caller if the invariant is violated.
consider clamping to zero or asserting the invariant:
```typescript
const importedCount = Math.max(
0,
deduplicatedAccounts.length - existingAccounts.length,
);
```
How can I resolve this? If you propose a fix, please make it concise.
…' into release/mainbound-20260323 * origin/plan/02-cli-help-and-login-split: test: cover live lease failures test: cover auth command error paths Serialize live auth-best probe writes test: cover concurrent auth best live refresh writes fix: clamp restored auth family indices test: cover extracted auth command modules refactor: split auth cli help and command flows # Conflicts: # lib/codex-manager.ts # test/documentation.test.ts
…elease/mainbound-20260323 * origin/plan/04-cli-repair-split: fix: guard repair command persistence drift fix: avoid stale flagged restore writes fix: align repair command review followups guard doctor sync after transaction abort fix direct repair command review regressions fix: preserve manage selection and flagged repair state fix(cli): snapshot doctor file existence checks fix doctor auto-fix transaction flow fix(cli):close-repair-review-gaps fix(storage): harden flagged transaction rollback test(cli): add direct repair command coverage fix(cli): close remaining repair review gaps fix(cli): close remaining repair review gaps fix(cli):transact-verify-flagged-persistence fix(cli): persist repair commands through storage tx refactor: split repair and doctor cli flows refactor: split forecast and report cli flows # Conflicts: # lib/codex-manager.ts # lib/storage.ts # test/codex-manager-cli.test.ts
Comment on lines
+281
to
+288
| max: 24 * 60 * 60_000, | ||
| step: 60_000, | ||
| unit: "ms", | ||
| }, | ||
| ]; | ||
|
|
||
| export const BACKEND_CATEGORY_OPTIONS: BackendCategoryOption[] = [ | ||
| { |
There was a problem hiding this comment.
proactiveRefreshGuardian and proactiveRefreshIntervalMs unreachable in category UI
proactiveRefreshGuardian is in BACKEND_TOGGLE_OPTIONS and proactiveRefreshIntervalMs is in BACKEND_NUMBER_OPTIONS, but neither is included in any BACKEND_CATEGORY_OPTIONS category. users navigating the backend settings UI will never see these two controls — the token refresh guard toggle and its scan interval are silently inaccessible.
they both logically belong in the "refresh-recovery" category alongside proactiveRefreshBufferMs and tokenRefreshSkewMs. the fix is to add "proactiveRefreshGuardian" to toggleKeys and "proactiveRefreshIntervalMs" to numberKeys for that category.
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/codex-manager/backend-settings-schema.ts
Line: 281-288
Comment:
**`proactiveRefreshGuardian` and `proactiveRefreshIntervalMs` unreachable in category UI**
`proactiveRefreshGuardian` is in `BACKEND_TOGGLE_OPTIONS` and `proactiveRefreshIntervalMs` is in `BACKEND_NUMBER_OPTIONS`, but neither is included in any `BACKEND_CATEGORY_OPTIONS` category. users navigating the backend settings UI will never see these two controls — the token refresh guard toggle and its scan interval are silently inaccessible.
they both logically belong in the `"refresh-recovery"` category alongside `proactiveRefreshBufferMs` and `tokenRefreshSkewMs`. the fix is to add `"proactiveRefreshGuardian"` to `toggleKeys` and `"proactiveRefreshIntervalMs"` to `numberKeys` for that category.
How can I resolve this? If you propose a fix, please make it concise.
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.
Summary
Merge the current
main-target PR set through a single verified release branch.Included PRs: #140 #141 #149 #263 #298 #299 #300 #302 #303 #304 #306 #307 #308 #309
Additional release fixes
forecastcommand testssessionAffinityMaxEntriesEACCESas retryable for queued settings writesLocal validation
npm run clean:repo:checknpm run typechecknpm run buildnpm run lintnpm test -- test/storage.test.ts test/named-backup-export.test.ts test/storage-recovery-paths.test.ts test/backup-metadata-builder.test.tsnpx vitest run --maxWorkers=1 --silent --shard=1/4npx vitest run --maxWorkers=1 --silent --shard=2/4npx vitest run --maxWorkers=1 --silent --shard=1/8npx vitest run --maxWorkers=1 --silent --shard=2/8npx vitest run --maxWorkers=1 --silent --shard=3/8npx vitest run --maxWorkers=1 --silent --shard=4/8npx vitest run --maxWorkers=1 --silent --shard=5/8Known local blocker
main.note: greptile review for oc-chatgpt-multi-auth. cite files like
lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.Greptile Summary
this release PR merges 14 main-target PRs — the bulk is a large codex-manager refactor extracting auth commands, settings panels, and repair/forecast commands into dedicated modules under
lib/codex-manager/, alongside a set of targeted fixes: corrected backup metadatalatestValidPathpriority (discovered backups now beat WAL even when WAL is newer),EACCESadded to retryable settings write codes, optionalloadDashboardDisplaySettingsdep in forecast command tests, switch command docs parity, and storage regressions for named-backup snapshot isolation and flagged transaction rollback.proactiveRefreshGuardiantoggle andproactiveRefreshIntervalMsnumber setting are defined inBACKEND_TOGGLE_OPTIONS/BACKEND_NUMBER_OPTIONSbut omitted from everyBACKEND_CATEGORY_OPTIONScategory — these controls are silently unreachable in the backend settings UI; they belong in"refresh-recovery"settings-write-queue.ts: per-path write queue serialization andEACCESaddition look correct; the TOCTOU-aware mtime logging innamed-backups.tsis good practicebackup-metadata-builder.tspriority fix is well-tested intest/backup-metadata-builder.test.tsnamed-backups.tsreaddircatch only silencesENOENT; on windows a transientEACCESon the backup root would surface as an unhandled throwswitch.tspath, not the old monolithiccodex-manager.tsvi.doMock+vi.resetModulescorrectly to verify stale transaction snapshots are ignored when the path key doesn't matchConfidence Score: 4/5
Important Files Changed
proactiveRefreshGuardiantoggle andproactiveRefreshIntervalMsnumber are missing from allBACKEND_CATEGORY_OPTIONSentries, making them unreachable in the UIFlowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[buildBackupMetadata] --> B[getAccountsBackupRecoveryCandidatesWithDiscovery] B --> C{classify candidate} C -->|=storagePath.bak| D[accounts-backup prio 2] C -->|starts with storagePath.bak.| E[accounts-backup-history prio 3] C -->|other| F[accounts-discovered-backup prio 4] A --> G[describeAccountsWalSnapshot → accounts-wal prio 1] A --> H[describeAccountSnapshot primary → accounts-primary prio 0] D & E & F & G & H --> I[accountSnapshots array] I --> J[selectLatestValidAccountPath] J -->|ACCOUNT_BACKUP_PREFERRED_KINDS first| K{any valid backup/history/discovered?} K -->|yes| L[highest priority then newest mtime] K -->|no| M{any valid WAL?} M -->|yes| N[WAL path] M -->|no| O[any valid snapshot] L & N & O --> P[latestValidPath override] P --> Q[BackupMetadata.accounts.latestValidPath]Prompt To Fix All With AI
Reviews (2): Last reviewed commit: "Merge remote-tracking branch 'origin/pla..." | Re-trigger Greptile