steid

@jamesgill /

docs: hand over what the archive and search work decided

Not `plans/` itself: five agents appending to `current.md` at once conflicts
every time, so this is folded in by the orchestrator at merge.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EXVUnompCMVxQ28a9T3HAx
JamesPatrickGill authored 15 hours agoparent1b7587dBrowse files6f86e0f02d910e357036114d6aec2b083de79cb7

1 file changed+111 −0

plans/handover-archive.md+111 −0View file
@@ -0,0 +1,111 @@
1+# Handover — `archive` (archive download + code search)
2+
3+Branch `feat/archive-search`. Two commits, both compiling, tests green at each.
4+Fold this into `progress.md` (decisions), `current.md` (holes) and `ui.md` (UX) at
5+merge time.
6+
7+## What shipped
8+
9+- `GET /{handle}/repos/{name}/archive/{rev}.tar.gz` and `.zip` — streamed `git archive`.
10+- `GET /{handle}/repos/{name}/search?q=&rev=` — fixed-string `git grep` over one revision.
11+- Entry points: two links under the clone URL in the About sidebar; a search box on the
12+ right of the landing page's toolbar row. Both are the slots `ui.md`'s table reserved.
13+- The blob's line-number cells now carry `id="L{n}"`.
14+
15+## progress.md material — decisions worth not rediscovering
16+
17+- **`GitArchive` is a fourth narrow port, not a `GitQuery` method.** `GitQuery`'s whole
18+ contract is *capped bytes, collected*; an archive is as large as the repository. It
19+ therefore lives in `infrastructure/git.rs` beside the protocol server, whose shape it
20+ shares, and reuses `web::git::GitBody` verbatim — one subprocess's stdout becoming a
21+ response body is now written once and used twice.
22+- **The archive use case is two steps and that is what makes the `ETag` cheap.**
23+ `archive_repo` authorizes and resolves, handing back an `ArchiveTarget` that nothing
24+ else can construct; `open_archive` consumes it and runs git. A conditional request is
25+ answered from step one, so a repeat download of a tag costs one `cat-file`, not a
26+ pack. The `ETag` is the resolved commit, so `main` correctly re-downloads after a push
27+ and a tag never does.
28+- **`git grep` is the one command in `git_query.rs` whose exit status is an answer.** It
29+ exits 1 for "matched nothing". `run_within_allowing` takes an allowed-exit list and
30+ `NO_MATCHES` names the exception at the call site, so the module's rule — a non-zero
31+ exit is always a fault — is broken deliberately in one visible place rather than
32+ softened everywhere.
33+- **`git grep -z` is not optional.** Without it the field separator is a colon, and a
34+ path may contain colons, so `src/a:b.rs:12:3:text` has no unambiguous reading. With
35+ `-z` the record is `<commit>:<path> NUL <line> NUL <column> NUL <text> LF`, and the
36+ parser walks NULs rather than splitting lines — which is also what survives a newline
37+ inside a *path*.
38+- **`RepoPath` refuses a colon, so a file named `a:b.rs` is unsearchable as well as
39+ unbrowsable.** The parser drops those hits rather than linking to a page that must
40+ 404. Pre-existing constraint, newly visible; there is a test pinning it.
41+- **A timeout is a page, not a 500.** `search_repo` catches `GitQueryError::is_timeout`
42+ and returns `Searched::TimedOut`, which is the first real consumer of the timeout kind
43+ the previous commit added.
44+- **Archive and search both 404 a private repository for a stranger**, matching every
45+ other read page. See the open question below — the brief asked for 401.
46+- **A subtlety in the dev loop, not in the code:** `topcoat dev` in this worktree exited
47+ rather than restarting after two of the three source edits made while the server was
48+ running, and had to be relaunched by hand. Nothing in the log explained it. Worth
49+ watching; may be two `topcoat dev` instances sharing a `target/`.
50+
51+## current.md material — holes opened, shortcuts taken
52+
53+- **`git grep` output is collected whole before it is capped.** `run` reads the
54+ subprocess's stdout into memory, so a one-letter query against a large repository
55+ allocates all of git's output even though only 200 hits are kept. `git grep` has no
56+ portable global maximum; `-m` is per file. Bounded in practice by the 20-second
57+ timeout, not by anything deliberate.
58+- **The search box searches the revision being browsed but the results page does not
59+ offer a revision switcher.** Changing revision from the results means editing the URL.
60+ The switcher lives in `browse.rs` and is private; exposing it is a one-word change
61+ when someone wants it.
62+- **A query is `trim()`ed**, so a search for a trailing space is a search without it.
63+ Conventional, and wrong for anyone hunting trailing whitespace.
64+- **Matched lines are cut at 500 characters**, and the cut can remove the match itself
65+ on a minified line. The alternative was letting one bundle make the page megabytes.
66+- **Highlighting re-scans the line for every occurrence** and ignores the column git
67+ reported, so `GrepHit::column` is carried but unused by the page.
68+- **No paging on search**, the same v1 decision as the log's 50.
69+- **The archive endpoint has no rate limiting.** It is the most expensive anonymous
70+ request in Steid — a full pack of the repository per hit, with a 20-second-plus
71+ ceiling nothing enforces because `git archive` does not go through `GitQuery::run`.
72+ Cheap to abuse, and the first thing to look at if an instance is ever put under load.
73+- **`git archive` is not covered by the read timeout.** Deliberately: a legitimately
74+ large repository takes longer than twenty seconds to pack, and killing that is worse
75+ than the request being slow. It is bounded only by `kill_on_drop` when the client
76+ goes away.
77+- **A late `git archive` failure is a truncated download**, not an error — the status is
78+ known only after headers have been sent. Logged, same as the protocol server's.
79+
80+## ui.md material
81+
82+- **Archive download** and **Code search** in "Where the next features go" are both
83+ filled in; the table entries can be marked done.
84+- **The download links are for the revision being viewed**, not always the default
85+ branch — someone reading a tag wants that tag's tarball. An empty repository shows no
86+ links, because there is nothing to pack.
87+- **The search box is the width of the About sidebar** on `sm` and up and full width
88+ below it, so the two columns of the landing page line up. On the search page itself it
89+ is full width.
90+- **`<mark>` is `bg-primary/25`**, which is the only place other than the active tab and
91+ the latest-commit dot where the primary colour appears on a repository page. It marks
92+ the thing being looked for, which is the same rule.
93+- **Every search state is a bordered panel with one sentence**: no commits, no query, no
94+ matches, query too long, timed out. They read as the same object because they are.
95+- **A long matched line scrolls inside its own `<code>`**, verified at 800px — the page
96+ itself does not scroll horizontally.
97+
98+## Verified
99+
100+- `cargo test` — 503 pass. `cargo clippy --all-targets` — zero warnings. `cargo fmt`.
101+- Against a real instance on :3103, this repository pushed into it with 7 branches and
102+ two annotated tags: both archives downloaded and **extracted** (`tar tzf`, `unzip`)
103+ with the `steid-main/` and `steid-v0.2.0/` prefix directories; `If-None-Match` on the
104+ returned `ETag` answered 304; unknown revision, unknown extension, and a private
105+ repository as an anonymous visitor all answered 404.
106+- Search verified live for results, no matches, no query, truncation at 200, an
107+ over-long query, a private repository (404) and an unknown revision (404).
108+- Both colour modes looked at by flipping the layout's `class="dark"`, restored after.
109+- **Not verified:** the timeout state in a browser — it is unit-tested through the
110+ in-memory port, but no repository here is slow enough to trigger a real 20-second
111+ grep. The `Searched::TimedOut` page has never been rendered.