feat: add Safari bookmarks and frequently visited commands#37
Conversation
athena-omt
added
area:infra
jmcte
force-pushed
the
pheidon/hermes-issue-group-20260514012033
branch
from
c6c0593 to
ca16b19
Compare
![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: c6c0593592
ℹ️ 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".
| switch (lhs.element.score, rhs.element.score) { | ||
| case let (left?, right?) where left != right: return left > right | ||
| default: return lhs.offset < rhs.offset | ||
| } |
There was a problem hiding this comment.
Ensure the sort comparator treats entries with a numeric score as higher priority than entries with nil score. As written, any mixed pair (one scored, one unscored) falls through to original traversal order, so an unscored URL that appears earlier can outrank a scored URL and get a lower rank, which makes frequently-visited output inaccurate on plists where some items omit score fields.
Useful? React with 👍 / 👎.
| guard FileManager.default.fileExists(atPath: fileURL.path) else { throw SafariFrequentlyVisitedError.missingTopSites(fileURL.path) } | ||
| let data: Data | ||
| do { data = try Data(contentsOf: fileURL) } catch { throw SafariFrequentlyVisitedError.unreadableTopSites(fileURL.path) } | ||
| let plist = try PropertyListSerialization.propertyList(from: data, options: [], format: nil) |
There was a problem hiding this comment.
Handle PropertyListSerialization.propertyList failures the same way file-read failures are handled. Right now a malformed or partially-written TopSites.plist throws a raw Cocoa error without the file path, so users lose the command-specific unreadableTopSites(path) context and cannot tell which source file caused the failure.
Useful? React with 👍 / 👎.
3 participants
Summary
icloud-cli safari bookmarksicloud-cli safari reading-listicloud-cli safari frequently-visitedCloses #20
Closes #36
Validation
bash scripts/ci/run-fast-checks.shswift testswift buildbash scripts/check-privacy-fixtures.sh