Repository navigation
Conversation
Read library metadata and profiles through checked regular-file descriptors and preserve open error identity. Retain filtered OAuth child failure diagnostics used during validation. Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
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 |
|
✅ Deterministic PR hygiene checks passed. |
|
Maintainer integration (dev-only bypass per MAINTAINERS.md), not an independent approval. Union fix pushed by the maintainer: with #7007 on Exact head |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61adac5287
ℹ️ 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".
| Every read of Desktop library configuration — selected-profile inspection, `_meta.json` parsing, | ||
| foreign-key preservation during writes, refresh mode re-resolution, the refresh fingerprint hash, | ||
| the picker profile's inspection and apply snapshots, and the connection-owned store's metadata and | ||
| profile reads — goes through the shared verified regular-file reader in | ||
| `src/claude/desktop-3p-library.ts` (`readDesktopLibraryBytes`/`readDesktopLibraryText`). The path |
There was a problem hiding this comment.
During a normal (non-refresh) rewrite of an existing profile, this “every read” claim is false: after readDesktopProfileForeignKeys(configPath) returns, atomicReplaceDesktopConfig reopens the same path via copyFileSync(path, backupPath), and the rollback similarly path-copies the backup. If another process renames a FIFO onto the profile in that gap, the unchecked synchronous source open blocks indefinitely rather than producing desktop_config_not_regular; retain the bytes from the verified descriptor for backup/rollback (or use the same checked open for those reads) before documenting the invariant.
AGENTS.md reference: structure/AGENTS.md:L9-L10
Useful? React with 👍 / 👎.
| export function parseMetadata(path: string): Desktop3pMetadata { | ||
| if (!existsSync(path)) return { entries: [] }; | ||
| const parsed = JSON.parse(readFileSync(path, "utf8")) as Partial<Desktop3pMetadata>; | ||
| const parsed = JSON.parse(readDesktopLibraryText(path)) as Partial<Desktop3pMetadata>; |
There was a problem hiding this comment.
When _meta.json is a dangling symlink (for example, after a linked configuration target is moved), existsSync on the preceding line follows the missing target and returns false, so this checked read is skipped and parseMetadata returns an empty library. A normal apply then writes a newly allocated profile and fails only when atomicWriteFile reaches the dangling metadata link; because there was no backup, that new profile is left orphaned (and each retry can add another) instead of refusing before mutation. Use lstat/ENOENT handling for optional paths so an existing non-regular directory entry reaches desktop_config_not_regular.
Useful? React with 👍 / 👎.
2 participants
Summary
Verification
e8afb8bb41786ddd595fd8ab9f36cff46d403dabon current deve6d7e561f566ab343fbf545a6fa3e1b63012f817. All nine changed files match reviewed sourcea3137becda4102bfcc2c1cda6cdfe75aedb08a87byte for byte; the base movement only adds disjoint diagnostic restart isolation files.bun run typecheck,bun run structure:check,bun run privacy:scan, andgit diff --checkpassed on the integration candidate. Privacy scope is the materialized sparse checkout including all changed files.Checklist
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>