An honest, deployable landing page - #34
Merged
Merged
Conversation
`repos/register.ts` runs the parser with execFile from the queue worker. It is a plain child process carrying the worker's network, filesystem and user. `--network none`, the read-only rootfs and the dropped capabilities apply under `make parser-isolated` and nowhere else. Five documents said otherwise. docs/SECURITY.md ticked "clone/parse runs in an isolated container" and "parser has no outbound network access" from Phase 1 onward. CLAUDE.md said isolation "was built in Phase 1, not deferred". ARCHITECTURE.md's diagram labelled the parser box "network none". PLAN.md said Phase 1 built it to avoid a later retrofit. The landing page told readers their repository is cloned into a sandbox with no network and no capabilities -- and that page is about to be deployed. The claim is corrected rather than the code. Both fixes cost more than the gap: shelling out to `docker run` needs the Docker socket mounted into the worker, which grants root-equivalent host control and is a worse property than the one it buys; namespaces, seccomp or bubblewrap avoids that but is Linux-only and a project of its own. R38 records the choice and what would change it. What survives is enforced on every path, because it lives in the parser rather than around it: symlinks hard-fail, files over 1 MB are skipped, file count and depth are capped, the clone is depth-one with credential prompts disabled, no repo script is ever invoked, and the checkout is removed on the failure path. The landing page now leads with the strongest claim that is actually true -- nothing in the repository is executed -- instead of the one that was not. Two things worth keeping from how this hid for four phases. A checkbox ticked against a harness says nothing about the product, and the harness runs with `--no-deps`, which is also why nobody hit the `parser` service declaring both `network_mode: none` and a `depends_on` on Postgres. And the TOCTOU deferral in the same checklist had leaned on this claim inside its own justification, so one false statement had already propagated into a second decision.
There is no hosted instance, so the public face is the web app on its own. "Open the atlas" would send every visitor of that build to an error screen. VITE_SHOWCASE flips the one component that defines the call to action: `OpenAtlas` renders a link to the repository labelled "Run it yourself" instead. The label changes with the destination, because a button still reading "Open the atlas" that opened GitHub would be the same lie in a friendlier voice. Build-time rather than a runtime probe, deliberately: the landing page makes no request to our API, which is what lets it render when the backend is absent, and asking whether the API is up would give that away for a fact the build already knows. The limits copy now says the thing that is true in both builds -- this is a tool you run yourself, there is no instance to sign in to but your own. The canvas route is lazy. Statically imported, React Flow and the panels sat in the main bundle and every landing-page visitor downloaded a canvas they never opened; on a showcase build it can never be reached at all. Splitting `AppRoute` into its own module takes the entry chunk from 323 kB to 170 kB gzipped and drops react-flow out of it entirely. `ResolvingSession` moves to its own file because both sides need it and importing it from `AppRoute` would pull that chunk straight back into the entry. vercel.json carries the monorepo build and, more importantly, a rewrite of everything to index.html -- without it a deep link or a refresh on /app is a 404, since there is no server-side route for it. Verified against the built bundle under `vite preview`, which is the only local thing with the same fallback behaviour: all three calls to action read "Run it yourself" and point at the repository, and /app answers 200 and renders the unreachable-API screen rather than a 404. Three things left unverified in #33 are now measured rather than assumed. The draw animation runs -- the mask transform steps -374px, -62px, -5.7px, then is removed, with node opacities staggering 0.32, 0.81, 1 behind it. The breakpoints hold: 375px is single column with no horizontal overflow and the header's second call to action correctly hidden, 768px shows it. The earlier "requestAnimationFrame is dead" reading was a stale tab, not the page.
Two faults in one row. Selecting a function swapped the background with no transition, so it read as a repaint rather than as an answer to the click -- everything else on this canvas moves under one. The second was hiding inside the first. The line-number badge is `variant ="outline"`, bordered with `--border`, and `--accent` resolves to the same token; on the selected row the pill's edge was drawn in exactly the colour of the row beneath it and disappeared. Every other row had a pill and the selected one did not, which read as the badge being removed on selection. It keeps its own ground there now. Both transitions are under `motion-safe:`. No test: this is two class strings, and asserting the class names back would be a change detector rather than a check. Verified in a browser against pmndrs/zustand.
⌘K marks a result with `bg-accent` and `ring-1 ring-primary/40`. The card's rows had the accent ground and no ring, so the same object -- a function you can pick -- looked like two different things depending on which surface you found it on. They match now, with a lighter ring on hover so the row answers the pointer before it is clicked. box-shadow is named in the transition because that is what a ring is; `transition-colors` alone eased the fill and snapped the outline.
It was stale in ways that would waste a new contributor's afternoon. The phase table said Phase 5 "not started"; it merged in 4e240ed with eight languages. "How it works" said `.ts`/`.tsx` only. And the setup instructions told people to sign in with "Continue as a local dev user" and to POST to `/auth/dev-login` -- a route deleted in Phase 4 (R30). Following the README as written got you a 404 and a login screen with no such button. Rewritten around the reader who has just cloned it and wants it running. The confidence tiers are the first thing on the page, with a table of what each one means, because that is the product rather than a detail of it. The GitHub OAuth app now has its three exact field values written out, since that is the one step nothing can do for you, along with why the scope is `read:user` and why that limits it to public repositories. A "what it does not do" section states the limits in the README instead of leaving them to be discovered: public repositories only, no hosted instance, the incremental re-parse that is only incremental in its write (R35), and the parser sandbox being a harness rather than the running path (R38). The `docker compose up` gap is called out as not working rather than quietly omitted, since it is the next piece of work. Every link in the document was checked to resolve.
Splitting `/app` behind React.lazy made its tests wait on a dynamic import that really does pull React Flow and the rest of the canvas. Alone that lands well inside Testing Library's 1s default; in a full run, with eighteen other files competing for the same threads, it does not, and the route test failed intermittently. A longer ceiling for this file rather than a mock of the lazy boundary: the wait is a real property of the route, and every assertion still fails if it never renders. Confirmed over three consecutive full runs. `configure` is imported from @testing-library/react, not @testing-library/dom -- the latter is only a transitive dependency and does not resolve from this package.
Three complaints, one cause: the page was flat. Two hues across three thousand pixels, 30px headings sitting too close to 16px body for the eye to rank them, and eight languages listed as text. **A third ink.** `spot` -- #2ad4c4 dark, #0b7268 light. Named for what it is rather than as a tier, because it is the only colour in tokens.ts that carries no canvas meaning. A map is drawn in three inks and two of ours were already spoken for. It sits at 174°, 53° off `exact` and 153° off `name`, so it cannot be misread as either, and it never appears on the canvas -- there a colour meaning nothing would compete with three that mean something. Eyebrows, section detail, hover. Light is #0b7268 rather than the #0d7d72 that reads better, because that one measured 4.48:1 on paper and text wants 4.5. Worth recording: the palette database consulted for this returns slate plus #22C55E for every developer tool in it, which is UI_GUIDE §7's second cluster verbatim. It confirmed the default rather than escaping it, so its colour answer was not taken. **A type scale.** Section headings 40-48px against 18px ledes, and Bricolage's weight axis driven to 560 on headings against 400 in prose -- the axis was the reason the face was chosen and it had been sitting unused. **A language strip.** Eight marks drifting behind a mask that fades both ends, a CSS keyframe over `transform` with the set held twice and translated by exactly half, so the loop closes on itself. Reduced motion gets the same strip standing still, because the content is the point and the drift is decoration. The cards below keep the honest distinction and lose their logos: repeating all eight on one screen said it twice, and the cards carry the caveat rather than the names. Their language lists take the tier colour each group actually produces across files. Marks are vendored from simple-icons (CC0) rather than imported -- the package has no subpath exports, so using it means pulling three thousand icons to keep seven. Java is drawn here instead: Oracle's cup is a trademark simple-icons does not ship, OpenJDK's duke is legally clean but reads as nothing, so this is a plain coffee cup and a copy of no one's logo. A favicon, finally, which the app never had: the product's own notation at 16px, one caller reaching three callees along a solid, a dashed and a dotted edge, the last of them hollow.
The strip visibly restarted once a cycle. `gap-14` puts space *between* items, so a 16-item track carries 15 gaps: half its width is one set plus seven and a half gaps, while a seamless loop needs one set plus a whole one. Translating -50% therefore landed 28px short -- half a gap -- and the jump back was the reset. Each item carries its own trailing space now, so the two halves measure identically and -50% is exactly one set. Verified rather than eyeballed: both halves 1072.8px of a 2145.6px track, drift 0.
It points at two screenshots that do not exist and nothing renders it. Dead code should not merge, and rebuilding it is ten lines once there is an image to put in it -- `git show c9ad0a2` has the version with the reasoning in it.
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Free Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (29)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe changes correct parser isolation documentation and update the web application with lazy route loading, session states, landing-page content, showcase behavior, theme tokens, tests, and Vercel deployment configuration. ChangesProduct documentation and parser isolation
Web application routing and presentation
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to This PR updates the web landing page and related documentation without introducing an actionable merge-blocking risk; it is merge-ready after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Browser
participant App
participant AppRoute
participant Explorer
Browser->>App: Request /app
App->>AppRoute: Lazy-load application route
App->>AppRoute: Render ResolvingSession during session resolution
AppRoute->>Explorer: Render authenticated explorer layout
Warning Some tools did not complete. Review the errors below. 🔧 ESLint
apps/web/src/App.test.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. apps/web/src/App.tsxESLint skipped: the matched ESLint configuration already failed (missing-dependency). apps/web/src/AppRoute.tsxESLint skipped: the matched ESLint configuration already failed (missing-dependency).
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/settings/billing. Comment |
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 by CodeRabbit