Browser foundations: shared types + pure logic - #467
Conversation
Wire types for the embedded browser plus the pure decision layer they feed: URL/omnibox normalization, the tab model (add/remove/activate), top-sites merging, visit history, persisted-state sanitizers, and the browser:* IPC channel constants (snapshot-locked). No Electron imports — every function is pure and unit-tested directly.
The decision layer the manager orchestrates: omnibox→URL normalization (host:port handling, search fallback), scheme allowlist, tab model transitions, top-sites merge (Chrome import + cowork history), redacted visit history, persisted tabs/history sanitizers, favicon validation, download-name dedupe. Pure functions only — the full unit matrix runs without Electron.
![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: b97dce253a
ℹ️ 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".
| tabs.push({ | ||
| id: tab.id, | ||
| url, | ||
| title: typeof tab.title === 'string' ? tab.title : '', | ||
| favicon: sanitizeFaviconUrl(typeof tab.favicon === 'string' ? tab.favicon : null), | ||
| }); |
There was a problem hiding this comment.
Because sanitizePersistedTabs is the launch-restore validator, a corrupted or hand-edited tabs.json containing more than 50 otherwise valid http tabs is returned in full here. That bypasses the documented hard cap and can make restore allocate an unbounded number of tab views; stop adding once tabs.length reaches MAX_TABS or reject the file.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — restore is capped at MAX_TABS (78ff69c).
| out.push({ | ||
| url: entry.url, | ||
| title: typeof entry.title === 'string' ? entry.title : '', | ||
| ts: typeof entry.ts === 'number' ? entry.ts : 0, | ||
| }); |
There was a problem hiding this comment.
When history.json comes from an older build or a corrupt/manual edit, this copies entry.url back into the history unchanged. The rest of the history pipeline treats entries as already redacted, so query strings or hashes containing tokens/searches can be surfaced through top sites and persisted again; run the URL through redactUrlForLog here and skip empty results.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — restored history entries go through redactUrlForLog and empty results are skipped (78ff69c).
Review follow-up: sanitizePersistedTabs now stops at MAX_TABS so a corrupt tabs.json can't bypass the cap, and sanitizeHistory runs every restored URL through redactUrlForLog so stale/hand-edited files can't reintroduce query strings into top sites.
1 participant
What
The shared vocabulary for the embedded browser (part 1 of 5, replacing #462).
src/shared/browser-types.ts: the tab/state wire types used by renderer, main, and the agent bridge.browser:*IPC channel constants, snapshot-locked as usual (renames are breaking changes).src/main/browser/browser-logic.ts: the pure decision layer — omnibox→URL normalization (bare domains,localhost:3000, search fallback), the scheme allowlist, tab model transitions, top-sites merging across Chrome import + in-app history, redacted visit history, persisted-state sanitizers, favicon validation, and download-name deduping.No Electron imports anywhere — every function maps inputs to outputs and is tested directly, in the
update-logic.tstradition.Why this split
The previous core PR (#462) bundled ~5,000 lines. These are the foundations everything else reads from; landing them first means later PRs are pure behavior.
Verification
file:///javascript:rejection and host:port handling), tab ordering, history merge/redaction, and persisted-state repair.For QA
Nothing user-visible in this one — it's types and pure functions. If the suite is green, this ships. The interesting checks come in the follow-ups.