Repository navigation
Add multi-page application crawling - #6
Conversation
![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: 72b8e5b403
ℹ️ 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".
| seen.add(result.canonical.toString()); | ||
| result.depth = item.depth; | ||
| results.push(result); |
There was a problem hiding this comment.
When a queued URL redirects to a canonical URL that was already captured (for example /index.html or /login redirecting to /), this code still records the result. The later screen loop derives the id from result.artifactName, so the second result gets the same screen-* id and may have overwritten the first page's evidence files, causing serializeAppSpec or verification to fail instead of producing a crawl. Check result.canonical against the captured set before adding the result.
Useful? React with 👍 / 👎.
| const target = spec.screens.find((screen) => screen.id === edge.targetScreenId); | ||
| return { | ||
| id: flowId(edge.sourceScreenId, edge.targetScreenId, edge.trigger, index), | ||
| name: `${source?.title ?? source?.route} to ${target?.title ?? target?.route}`, |
There was a problem hiding this comment.
For crawls where linked pages have long titles, concatenating two screen titles can exceed the AppSpec 300-character flow-name limit enforced by validateFlow, even though each individual screen title is valid. A page with long SEO titles and a link between them will make the final serializeAppSpec call reject the generated crawl; truncate or otherwise bound the generated flow.name before serialization.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 926234fa97
ℹ️ 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".
| const title = cleanText(await page.title(), 300) || cleanText(finalUrl.hostname, 300); | ||
| while (queue.length && results.length < maxPages) { | ||
| const item = queue.shift(); | ||
| const requested = canonicalizeCrawlUrl(item.url, { origin: crawlOrigin ?? undefined, includeQuery }) ?? new URL(item.url); |
There was a problem hiding this comment.
When capturePublicPage delegates to this crawl path, includeQuery is false and the initial URL is canonicalized before page.goto, so reconstruct capture https://site/search?q=foo navigates to /search instead of the requested URL. This regresses single-page capture for query-dependent pages, and the capture command has no --include-query escape hatch.
Useful? React with 👍 / 👎.
| includeQuery | ||
| }); | ||
| crawlOrigin ??= result.finalUrl.origin; | ||
| if (result.finalUrl.origin !== crawlOrigin) throw new Error(`Redirected outside crawl origin: ${result.finalUrl.origin}`); |
There was a problem hiding this comment.
If a queued same-origin route redirects to another public origin after the first page, captureOnePage has already written that redirected page's DOM/HTML/screenshot into staging before this post-check throws. Because later page failures are tolerated, the transaction still commits and leaves unmanifested external-origin evidence files in the output despite the same-origin crawl boundary.
Useful? React with 👍 / 👎.
| return String(value ?? "").replace(/[\u0000-\u001f\u007f]/g, " ").replace(/\s+/g, " ").trim().slice(0, max); | ||
| function flowId(sourceScreenId, targetScreenId, trigger, index) { | ||
| const triggerSlug = cleanText(trigger, 60).toLowerCase().replace(/[^a-z0-9]+/g, "-").replace(/^-|-$/g, "") || "link"; | ||
| return `flow-${sourceScreenId.replace(/^screen-/, "")}-${targetScreenId.replace(/^screen-/, "")}-${triggerSlug}-${index + 1}`.slice(0, 128); |
There was a problem hiding this comment.
For multiple links from the same source to long routes with the same leading slug, this final .slice(0, 128) can cut off the target hash, trigger, and index, producing identical flow IDs. validateUniqueIds then causes serializeAppSpec to reject the whole crawl; keep a hash or index at the end when clamping the ID.
Useful? React with 👍 / 👎.
Reconstruct v0.3 adds multi-page application mapping, route graph output, shared component detection, navigation flow records, updated exports, documentation, and test coverage.