git-ents.gitmain
⌘K
foforge
commit 4ed6a21
docs: remove completed plan and handoff documents

Assisted-by: Claude:claude-fable-5

Joseph D. Carpinelli · 1 month ago

Reviews

No reviews of this commit yet — record a verdict below.

Start a review

verdict

PROMPT.adoc @@ -1,73 +1,0 @@ -= Development Plan -:doctype: article -:toc: -:toclevels: 2 -:sectnums: - -NOTE: This plan was generated by an AI agent as a structured development prompt. - -== Overview - -The `git-ents` project is a single-node git hosting service reached over HTTPS. -A thin gateway delegates all git protocol work to git's own `git http-backend` CGI, which implements the entire smart-HTTP protocol. -Access is open during this experimental phase: any client may clone, fetch, and push, and creates a repository by pushing to an unused name. -TLS, the public IP, and the custom domain are provided by Fly.io's HTTP proxy, so the gateway holds no certificates and opens no privileged ports. - -== Design Principles - -* Access is open. Stock git clones and pushes with no token, no plugin, and no custom protocol; the gateway adds nothing to the hot path. Authentication will return as the project matures, but it is deliberately absent now. -* The gateway is a thin translator. It maps Axum requests to the `git http-backend` CGI and back, and otherwise stays out of git's way. -* Commit signing is provenance, not authentication. If wanted, it is an optional "verified" check, never a gate that admits a push. -* Git objects are immutable and content-addressed; git serializes ref updates itself on a single node. No discovery, no consensus, no CI. - -== Repository Layout - ----- -git-ents/ - crates/ - git-ents-server/ # the HTTPS gateway binary - src/ - main.rs # entrypoint, config, startup - http.rs # git-http-backend dispatch - Dockerfile - .config/ - fly.toml ----- - -== Phase 1: HTTPS Git Delegation - -Goal: `git clone` and `git push` work over HTTPS against a persistent volume on Fly.io, with the gateway delegating all protocol work to `git http-backend`. - -=== Tasks - -. Build the gateway on Axum over a Tokio runtime so request bodies and CGI output stream without blocking. -. Implement `main.rs` to read configuration from the environment (the repository root), disable the default request body limit so large packfiles are accepted, bind the HTTP listener, and start the server. -. Implement `http.rs` to, for every request: -.. derive the repository path from the URL and reject any path containing `..` or escaping `/data/repos`; -.. auto-initialize the bare repo (with `http.receivepack=true`) if absent on a push; -.. invoke `git http-backend` as a CGI child: set `GIT_PROJECT_ROOT=/data/repos`, `GIT_HTTP_EXPORT_ALL=1`, `PATH_INFO`, `REQUEST_METHOD`, `QUERY_STRING`, and `CONTENT_TYPE`; pipe the request body to its stdin; and return its CGI stdout (status + headers + body) to the client. -. Write a `Dockerfile` whose runtime image installs `git`, mounts the persistent volume at `/data`, and listens on the `internal_port` from `fly.toml`. -. Confirm `fly.toml` keeps the `[http_service]` with `force_https` and the volume mount; no dedicated IP and no TCP service are needed. -. Deploy to Fly.io and verify that `git clone`, `git push`, and a re-clone round-trip all succeed. - -=== Acceptance Criteria - -* `git push https://<domain>/repo.git main` reaches `git http-backend` and updates the repo. -* A push to a previously unused name creates the repository. -* Objects persist across process restart; the volume survives redeploy. -* Cloning a repo created by a previous push returns the same objects. - -== Phase 2: Custom Domain - -Goal: the service is reachable over HTTPS at a custom domain on Fly.io. - -=== Tasks - -. Add a Fly certificate for the hostname (`fly certs add <hostname>`). -. Create the DNS records Fly reports (`CNAME` to the app, or `A`/`AAAA` to the shared IPs) and the `_acme-challenge` record for validation. -. Verify `git clone https://<domain>/repo.git` resolves, validates TLS, and connects. - -=== Acceptance Criteria - -* Cloning and pushing over `https://<domain>/...` succeed end to end. -* Fly issues and renews the certificate automatically; the gateway manages no certificates.
docs/cli-ux-rough-edges-plan.adoc @@ -1,74 +1,0 @@ -= CLI UX rough edges: fix plan -:doctype: article -:toc: -:toclevels: 2 - -Found by exercising `git-ents` end to end (account, members, checks, comment, -login) against a local bare "remote" with no server running. Each item below -is independent and can be fixed and committed separately. - -== 1. `account create` always prints "created account X" - -Even when updating an existing account (same username, changed -`display_name`/`bio`, or even a username change), `account_create` -(`crates/git-ents/src/main.rs:1076`) unconditionally prints `created account -{username}`. It already loads `existing` to preserve `created_at` — reuse that -to print `updated account {username}` when `existing.is_some()`. - -== 2. `members revoke <fingerprint>` doesn't validate its argument - -`members_revoke` (`crates/git-ents/src/main.rs:887`) accepts any string and -pushes it straight into the deny list. A typo'd fingerprint (e.g. `members -revoke notafingerprint`) succeeds silently and prints `revoked -notafingerprint`, giving false confidence that a key was denied when the -deny-list entry will never match a real fingerprint. Validate the argument -looks like a key fingerprint (the `xx:xx:...` colon-hex form `members list` -prints) before storing it, or at least warn when it doesn't match any known -member's key. - -== 3. Inverted `--lines` ranges produce a misleading error - -`comment add file --lines 4:2` (end before start, well within the file) and -`comment add file --lines 0:2` (out of bounds) both produce the same -`Error::LinesOutOfRange` message: `lines {start}..={end} do not fit {path} -({len} lines)` (`crates/git-anchor/src/lib.rs:52`, raised via the -single-slice-lookup validation in `lines_of`, `crates/git-anchor/src/lib.rs:168`). -Users can't tell "your range is inverted" from "your file is shorter than you -think." Add an explicit `start > end` check in `parse_lines` -(`crates/git-ents/src/main.rs:513`) or in `lines_of`, with its own message -distinguishing an inverted range from an out-of-bounds one. - -== 4. Bad-remote errors mix raw git output with the app's own error line - -`members add x totallybogus` (or any command against a nonexistent remote) -prints git's own `fatal: ...` lines from `git ls-remote` followed by the -app's `error: git ls-remote failed` — two different error "voices" stacked in -one message, with no attempt to explain what went wrong in the CLI's own -words. Catch the `ls-remote` failure and surface a single message like -`remote '{remote}' not found` (falling back to the raw git output only when -it doesn't look like a plain "not found"). - -== 5. `checks add <name> ""` accepts a blank command silently - -`add_check` (`crates/git-ents/src/main.rs:682`) calls `interactive::text_or` -for both `name` and `command`. The storage layer rejects an empty *name* -(it's a git-tree map key, so `""` fails `facet-git-tree`'s key validation -with `is not a valid collection key`), but `command` is stored as a plain -blob value with no such check, so `checks add empty ""` silently creates a -check with a blank command — `checks list` shows it as `empty` with nothing -after it, and nothing fails until (if ever) the check tries to run. - -Root cause: `interactive::text_or` (`crates/git-ents/src/interactive.rs:18`) -only distinguishes `Some`/`None`, unlike `optional_text_or` -(`crates/git-ents/src/interactive.rs:35`) which treats an empty reply as -`None`. Fix in one place: make `text_or` reject (or trim-and-reject) an -empty string the same way it rejects `None` in non-interactive mode, and -re-prompt or error accordingly when interactive. This also covers -`account create`'s username and any other `text_or` call site that could be -passed an explicit empty string. - -== Not fixing - -* Positional-arg ordering (`account create [USERNAME] [REMOTE]`) is correct - and well-documented in `--help`; the only "issue" was tester error during - manual exploration, not a CLI defect.
docs/component-plan.adoc @@ -1,243 +1,0 @@ -= Component Abstraction Plan -:doctype: article -:toc: -:toclevels: 2 - -NOTE: Working plan for consolidating the per-type meta-ref handling into one -compile-time `Component` abstraction. The goal is minimal, pluggable server -code: each abstraction (issues, checks, members, config, …) fulfills a small -set of traits alongside `Facet` — how to store one in git, how to store a -collection, how to render — and the CLI and server are generic over them. -Pluggable at compile time only; no runtime registration. - -== Where we already are - -Three partial versions of this abstraction exist, grown independently: - -[cols="1,3",options="header"] -|=== -| Layer | What exists - -| Storage -| `git-store` is already generic over `Facet` with three layouts in live use: - single-document refs (`load`/`store`: config, account), map documents - (`load_map`/`store_map`: checks, revocations), and keyed namespaces - (`load_item`/`store_keyed`/`list_items` + `HasId`: members, issues). - -| Rendering -| `git-ents-server/src/web/render.rs` has a `Render` trait with impls for - `Check`, `Config`, `Issue`, `Member`, `Run`, plus a generic `render_peek` - fallback that walks the Facet shape. - -| CLI -| `git-ents/src/main.rs` has a `Set` trait (`REF`, `NOUN`, `load`/`store`, - `key`/`value`) used by checks; members are handled bespoke. -|=== - -The plan merges these into one definition per type. The measure of success is -that adding a new meta-ref type is: define the Facet struct, implement the -component traits, add one line to a route/dispatch table — and it appears in -the CLI and the web UI. - -== Design - -Two trait layers, split by crate so `maud` stays out of the core crate: - -=== Core: storage layout (`git-ents`, over `git-store`) - -Model the three layouts as traits rather than one trait with an enum, since -their method sets differ: - -[source,rust] ----- -/// A type stored whole on a single meta ref. -trait Document: for<'a> Facet<'a> { - const REF: &'static str; -} - -/// A type stored as one `<key> -> body` map document on a single ref. -trait MapDocument: Sized { - const REF: &'static str; - type Body: for<'a> Facet<'a>; - fn compose(key: String, body: Self::Body) -> Self; - fn decompose(&self) -> (&str, Self::Body); -} - -/// A type stored decomposed, one ref per item, under a namespace. -trait Collection: for<'a> Facet<'a> + git_store::HasId { - const NS: &'static str; -} ----- - -Blanket helpers (`fn load<T: Document>(repo) -> …`, `fn list<T: Collection>`, -…) delegate to the existing `git-store` methods; the per-module `load`/`store` -wrappers in `config.rs`, `checks.rs`, `members.rs`, `issues.rs`, -`account.rs`, `revocations.rs` become one-line impls or disappear. - -Every component also carries identity metadata for messages and UI chrome: - -[source,rust] ----- -trait Component { - const NOUN: &'static str; // "check", "member", "issue" - const PLURAL: &'static str; // "checks", … -} ----- - -=== Server: rendering and pages (`git-ents-server`) - -`Render` stays where it is and stays per-item. On top of it: - -[source,rust] ----- -trait WebComponent: Component + Render + Send + 'static { - const TAB: Tab; - const TITLE: &'static str; - fn note() -> Markup; // the shell-note line - fn load(repo: &Path) -> Result<Vec<Self>, String>; // sync; wrapped once -} ----- - -The generic unit is a *card*, not a page: `component_card::<T>` does the -`spawn_blocking` load, error card, empty card, count badge, and per-item -`render()`. A page is then either one card in `repo_shell` -(`component_page::<Issue>`) or a composition of several cards — the Settings -page composes the config, members, and checks components rather than living -outside the abstraction. The route table keeps its explicit match but each -list arm becomes `component_page::<Issue>(…)`. - -=== What stays outside the abstraction - -Be explicit about this up front — forcing these through the trait is the main -failure mode: - -* *Git-data pages* (`files`, `tree`, `blob`, `commit`, `releases`): not - meta-ref components; untouched. -* *Checks extras*: the HEAD join, the live registry, and the - recording/live/download routes stay bespoke routes beside the generic list. -* *Settings editing*: the page itself is *inside* the abstraction — a - composite that stacks the config, members, and checks component cards. What - stays bespoke is the auth/editing overlay on the config card (session - resolution, editable fields, the POST path); the composition is generic, - the overlay is not. -* *Runs*: namespace keyed by commit with nested run lists; does not fit - `Collection` — leave as is. -* *POST handling* (`web/write.rs`): editing is component-specific and gated; - out of scope for the first pass. - -== Phases - -Each phase compiles, passes `cargo nextest run`, and lands as its own commit. - -=== C1 — storage traits in `git-ents` - -Add `Document`/`MapDocument`/`Collection` + `Component` and generic -`load`/`store`/`list` helpers; implement for `Config`, `Account`, `Check`, -`Revocation`, `Member`, `Issue`. Delete the per-module wrapper bodies these -replace. *No on-disk format changes* — the fixed-format load tests -(`loads_the_on_disk_issue_format` and friends) must pass unmodified; per the -meta-ref fragility rule, any test change here means the phase is wrong. - -=== C2 — CLI `Set` on top of the storage traits - -Re-derive `Set`'s `load`/`store`/`REF` from C1's traits so a `Set` impl -shrinks to `NOUN`/`key`/`value`/`empty_listing`. Fold the bespoke member -list/remove plumbing into it if it fits without widening the trait; if -members need more than one extra method, leave them bespoke and note why. - -=== C3 — generic list page in the server - -Add `WebComponent`, `component_card::<T>`, and `component_page::<T>`; migrate -`issues_page` first (purest list page), then rebuild `settings_page` as a -composition of the config, members, and checks cards — the proof that cards -compose — and swap the configuration card of `checks_page` to the generic -card. The open/closed filter and label chips on issues stay as a -component-provided header fragment, not new trait methods. The settings -editing overlay wraps the config card from outside; if that requires the card -to take hooks, the card is cut at the wrong altitude — recut it (e.g. card -chrome vs. row rendering) rather than adding hook parameters. - -=== C4 — route dispatch - -Collapse the meta-ref list arms in `web/mod.rs::route` to the generic page -calls. Keep the match explicit (a static components table is only worth it if -it deletes code, which at three components it likely does not). - -=== C5 — re-measure - -Count deleted lines and re-read the trait. If any trait grew an optional -method used by one impl, inline it back. Update this doc with the outcome. - -==== Outcome - -C1–C4 landed as four commits (`1892fa82`, `d89c4caa`, `70e8f5db`, `e442c808`). -Measured `git diff --stat` across the whole change: *+424/-196* lines in -`crates/` — a net addition, not the hoped-for shrink. This confirms the -"Net-negative simplification" risk rather than dodging it: `git-ents`'s -`component.rs` (storage traits) and `git-ents-server`'s `component.rs` -(`Loadable`/`WebComponent`) are ~165 lines of new infrastructure that six -implementors (`Config`, `Account`, `Check`, `Revocation`, `Member`, `Issue`) -each pay a small, fixed impl-block tax into. At this component count the tax -outweighs the boilerplate it removes. The win is qualitative, per the -original framing: one extension point (define the Facet struct, implement a -handful of trait methods, done) instead of three independently-grown partial -ones — the measure that matters if a seventh component ever gets added, not -today's line count. - -Deviations from the plan as written, and why: - -* *`component_page::<T>` was never built.* No page turned out to be a clean - fit: the only single-component page (Issues) needs an open/closed filter - and dual subtab counts that replace the generic header entirely, which - would require a hook the plan's own trait-bloat rule rules out. Building - it anyway for zero real call sites would be exactly the "design for - hypothetical future requirements" this plan otherwise avoids. The card - renderer (`card`) is the piece that actually paid for itself, reused - across `checks_page`'s Configuration card and `settings_page`'s Members - and Checks cards. -* *`WebComponent` split into `Loadable` + `WebComponent`.* `Issue` only - benefits from the shared `spawn_blocking` loader (`issues_page` keeps its - own header/body); the original single trait would have forced it to - implement a `TITLE`/`empty` it never calls. Splitting the sync loader out - as its own supertrait is the trait-bloat rule applied in the other - direction — a method (well, a whole sub-contract) with fewer than two real - users does not stay bundled into the bigger trait. -* *C4 was a no-op.* With no `component_page`, the route table already was - the "one explicit call per meta-ref page" shape C4 asked for; there was - nothing left to collapse. -* *A few small copy changes were accepted* as the cost of genuine reuse: - `checks_page`'s "Configuration" card is now titled "Checks" (matching - `settings_page`'s card of the same data), and both share one empty-state - message ("No checks configured on `refs/meta/checks`.") where the two - pages previously had two different copies. The `p.shell-note` lines that - used to sit *inside* the Members/Checks cards on the Settings page now sit - just above them, since the generic card has no slot for a page-specific - note between its header and its rows. - -== Risks - -[cols="1,4",options="header"] -|=== -| Risk | Mitigation - -| Trait bloat -| The checks live view and settings editing will tempt hooks onto - `WebComponent`. Rule: a method used by fewer than two components does not - go on the trait. - -| Meta-ref format fragility -| Traits change only *where* load/store code lives, never the tree shape. - Fixed-format load tests are the gate; they may not be edited in C1. - -| Facet bounds noise -| `for<'a> Facet<'a> + Send + 'static` bounds spread through generic page - fns. Contain them in the two helper layers; page code should not repeat - them. - -| Net-negative simplification -| Materialized: +424/-196 lines, not a shrink (see the C5 outcome above). - Accepted rather than reversed, since the measure that matters is one - extension point instead of three partial abstractions, not today's line - count — but this is the risk to revisit first if a future component makes - the trait machinery earn its keep even less. -|===
docs/document-rendering-plan.adoc @@ -1,114 +1,0 @@ -= Document rendering plan - -Two independent threads, both scoped from the "documents are a database, give -me nicer views" conversation. - -== 1. MIME-keyed document rendering - -Today, AsciiDoc/Markdown rendering is HTML-only and dispatched by filename -extension (`asciidoc::is_asciidoc`, `markdown::is_markdown` in -`git-ents-server`), and only reachable from the web UI's blob viewer -(`web/pages.rs`). `Issue.body`/`Comment.body` are prose documents but render -as raw unstyled text (`web/pages.rs:715`), and the CLI has no rendering at -all (`comment show` prints raw lines). - -`git-ents` (the CLI binary) already depends on `git-ents-server` as a -library (see its `Cargo.toml` and `lib.rs`'s doc comment: "so `git ents` can -embed this server as its own `server` subcommand"). So the registry doesn't -need a new shared crate — it lives in `git-ents-server` and both the server -and the CLI consume it as a library dependency. - -=== New module: `git-ents-server/src/render.rs` (`pub mod render`) - -Two entry points, one per output target: - -[source,rust] ----- -pub fn mime_for_name(name: &str) -> &'static str; // extension -> MIME, replaces is_asciidoc/is_markdown -pub fn to_html(mime: &str, source: &str) -> String; // HTML output, for the web UI -pub fn to_text(mime: &str, source: &str) -> String; // plain-text output, for the CLI ----- - -Internally, each is a small const dispatch table (same style as -`registry::RECIPES`), not a trait hierarchy: - -[source,rust] ----- -const HTML: &[(&str, fn(&str) -> Option<String>)] = &[ - ("text/asciidoc", asciidoc::to_html), - ("text/markdown", markdown::to_html), -]; -const TEXT: &[(&str, fn(&str) -> String)] = &[ - ("text/asciidoc", asciidoc::to_text), -]; ----- - -Unrecognized MIME types fall through to a passthrough (HTML-escaped for -`to_html`, verbatim for `to_text`) rather than an error — MIME is an open -namespace, unlike an exhaustive enum match. - -=== `asciidoc.rs` changes - -Add `pub(crate) fn to_text(source: &str) -> String` using -`acdc-converters-terminal` — a plain-text AsciiDoc converter from the same -upstream `acdc` project (same git rev) already used for HTML. It's already -resolvable via `Cargo.lock` transitively, but needs to become a **direct -dependency** of `git-ents-server` (new `Cargo.toml` line + workspace -`[workspace.dependencies]` entry) — flagging per house rule since this is a -new dependency to sign off on. `to_html`/`is_asciidoc` are otherwise -unchanged. - -`markdown.rs` has no terminal converter available upstream; `to_text` for -`text/markdown` isn't added to the dispatch table, so it falls through to -the generic passthrough (acceptable — still better than nothing). - -=== Call sites - -* `web/pages.rs` (~168, ~591): replace - `is_asciidoc(&name) || is_markdown(&name)` + hand-rolled branch with - `render::mime_for_name(&name)` + `render::to_html(mime, &source)`. -* `web/pages.rs` (~715): render `Issue.body`/`Comment.body` through - `render::to_html("text/asciidoc", &body)` instead of raw text — closes the - "comments render as raw unstyled text" gap. -* `git-ents/src/main.rs`'s `comment_show` (~753): replace the raw - `for line in comment.body.lines()` loop with - `render::to_text("text/asciidoc", &comment.body)`. - -`lib.rs` gains `pub mod render;`; `asciidoc`/`markdown` stay private -(`mod`), since `render.rs` is a sibling module in the same crate — no -visibility escalation needed beyond `render`'s own public wrapper functions. - -== 2. `git ents toolchain view <name>` - -Separate from rendering — a gh-CLI-style single-entity view command, not a -`toolchain list` column. - -* `git_toolchain::disk_usage(repo, name) -> Result<Usage, Error>` - (`git-toolchain/src/lib.rs`): walks `Toolchain.bin` - (`Bin::Embedded(RawTree)` recursively via the tree's git backend, summing - blob sizes; `Bin::Downloaded(components)` reports per-component - size-unknown since there's no local tree to walk) and `Toolchain.src` if - present. No existing code does this tree-walk/size-sum today — new logic, - not a reuse. -* `Usage` is a `#[derive(Facet)]` struct (`total_bytes`, `bin_bytes`, - `src_bytes: Option<u64>`), rendered via `facet_pretty`, consistent with - `toolchain recipes`. -* New `ToolchainAction::View { name: String }` in `git-ents/src/main.rs` - alongside `Export`/`Log`/`Recipes`: prints recipe/version/provenance - (reusing `git_toolchain::history`'s latest entry) plus the `Usage` - breakdown. - -== Files touched - -* `crates/git-ents-server/src/render.rs` (new) -* `crates/git-ents-server/src/asciidoc.rs` (+`to_text`) -* `crates/git-ents-server/src/lib.rs` (`pub mod render;`) -* `crates/git-ents-server/src/web/pages.rs` (swap dispatch; render comment/issue bodies) -* `crates/git-ents-server/Cargo.toml` (+ `acdc-converters-terminal`) -* `Cargo.toml` workspace deps (+ `acdc-converters-terminal` entry) -* `crates/git-ents/src/main.rs` (`comment_show` uses `render::to_text`; new `ToolchainAction::View`) -* `crates/git-toolchain/src/lib.rs` (`disk_usage` + `Usage`) - -New dependency requiring sign-off: `acdc-converters-terminal` (git dep, -already resolvable in `Cargo.lock` via the existing `acdc` git rev, but not -yet a direct dependency of any crate).
docs/p5-p6-handoff.adoc @@ -1,261 +1,0 @@ -= P5 / P6 Handoff: meta-ref structure & routing -:doctype: article -:toc: -:toclevels: 2 - -NOTE: *Complete.* P5 (members rename, `refs/meta/config`, topics migration, -Settings projection, `refs/meta/issues/<id>`, rendering) and P6 (explicit -`get`/`post` wire routes) are all done and committed; every item in the -Definition of done below holds. This document is kept as the record of the -work and the decisions behind it. - -NOTE: This continued `docs/simplification-plan.adoc`. P8, P3, P7, P2, P1, P4 -(no-op), and correctness items C1/C2/C3 were *done and committed* before this -handoff. C4 was *skipped* (its premise no longer holds — see below). C5 folded -into P5. The remaining work was *P5* (make Settings & Issues functional via -typed meta refs) and *P6* (routing); both are now landed. Read the -simplification plan's "New direction" section first; the layout decisions there -are settled and this document assumes them. - -== Decisions already made (do not re-litigate) - -* **`refs/meta/auth` → `refs/meta/members`: hard rename, no fallback.** The - owner will kill and redeploy the server; the live trust list is intentionally - discarded and re-pushed. *Do not* write a read-fallback that reads `auth` when - `members` is absent. One ref name, `refs/meta/members`, period. -* Config altitude is **Path A** (compile-time `Facet` structs; the compiler - holds the schema). -* `config = { description: String, homepage: String, topics: Vec<String> }`. -* **No worktree metadata — there is never a `.gitents` directory.** -* Decomposed refs, aggregated views: no `refs/meta/settings` god-doc; the - Settings page is a *projection* over multiple refs. - -== Ground rules (carried from CLAUDE.md + work so far) - -* Tests run with `cargo nextest run`, never `cargo test`. -* Every meta-ref document is loaded/stored through `git_store` (`crates/git-store/src/lib.rs`). - Single-field `BTreeMap<String,String>` "set" docs use the `MapDoc` trait + - `Store::load_entries`/`store_entries`. Mixed-field docs (like `Config`) use - `Store::load`/`store` directly with a plain `Facet` struct. -* **Format stability is load-bearing.** A doc's `Facet` shape *is* its on-disk - format. Every new meta-ref type MUST get a fixed-format load test built with - raw git plumbing, exactly like the existing ones — see - `crates/git-ents/src/signers.rs::tests::loads_the_on_disk_signers_format` and - the shared builder `crate::testutil::write_meta_doc`. This is the C1 policy, - documented in `git-store`'s module docs. -* `git_ents::ZERO_OID` is the canonical all-zero push oid; don't re-introduce - string literals. -* Commit per logical change, Conventional Commit headers + footers, terse body, - `Assisted-by: Claude:<model-id>` last. Run the `prek` hooks (they auto-fmt; - re-`git add` and re-commit if `cargo fmt`/eof hooks modify files). Never push. -* Confirm `cargo clippy --workspace --all-targets` and - `cargo nextest run --workspace` are clean before each commit. - -== Current state the next agent inherits - -* `crates/git-ents/src/signers.rs` — `AUTH_REF = "refs/meta/auth"`, `Auth` - doc (`signers: BTreeMap`), `Signer { fingerprint, key }`, implements - `git_store::MapDoc`. `load`/`store`/`allowed_signers`. -* `crates/git-ents/src/checks.rs` — `CHECKS_REF`, `RUNS_NS`, `Checks`/`RunDoc` - (both `MapDoc`), `Check`, `Run`, `RunOutcome`, `CommitRuns`. -* `crates/git-ents/src/main.rs` — the porcelain. `Set` trait drives generic - `list::<S>` / `remove::<S>`; `sync(remote, refname)` and - `push_signed(remote, refname, expected)` are the shared git plumbing. `add` - is per-noun. Top-level subcommands: `Auth` and `Checks`. -* `crates/git-ents-server/src/verify.rs` — `pre_receive` calls `signers::load`; - it picks up the ref name purely through the `signers` module constant. -* `crates/git-ents-server/src/web/` — `mod.rs` (routing into the web UI + - `RepoMeta` + `gather_meta` + shell/header/tab bar), `pages.rs` (per-tab - renderers, incl. the **stub** `issues_page` and read-only `settings_page`), - `render.rs` (the `Render` trait: structural `Peek` walk + `Signer`/`Run` - overrides), `git.rs` (git data layer incl. the new `git_output_capped`). -* `crates/git-ents-server/src/http.rs` — the routing heuristics P6 targets: - `wants_web_ui`, `is_browse_route`, `is_service_request`, `is_git_path`, - `is_receive_pack`, `query_service`, all feeding the single `fallback(http::git)`. - -NOTE on **C4** (skipped): every page renders through `repo_shell`, whose header -band + tab bar use branch/description/topics/releases on *every* tab. So -`gather_meta`'s git calls already feed visible chrome; "gather lazily per tab" -saves nothing. After P5 moves topics/description into `refs/meta/config`, -re-check this — `gather_meta` should then read config once instead of shelling -out, which is the real win. - -== P5 — work breakdown - -Do these as separate commits in roughly this order; tests between. - -=== P5.1 — Rename the trust ref to `refs/meta/members` (hard break) - -Files: `crates/git-ents/src/signers.rs`, `crates/git-ents/src/main.rs`, -`crates/git-ents-server/tests/pre_receive.rs` (any fixtures), and the C1 test. - -* In `signers.rs`: rename `pub const AUTH_REF` → `pub const MEMBERS_REF` with - value `"refs/meta/members"`. Update its doc comment and the module docs (drop - "open bootstrap window" wording only if you keep the behavior — keep it; an - empty members ref still means open bootstrap). -* **Subtree name decision:** the `Auth` struct's field is `signers`, which fixes - the on-disk subtree `signers/`. Since the trust list is being discarded - anyway, rename the field to `members` for coherence (struct `Members { members: - BTreeMap }`), OR keep `signers`. Either is fine — but whatever you pick, the - C1 fixed-format test (`loads_the_on_disk_signers_format`) MUST be updated to - build the matching subtree, and it is your guardrail that load still works. - Recommended: rename struct `Auth`→`Members`, keep the public `Signer` type - name (a member *is* one or more keys; the doc stays keyed by fingerprint→key). -* In `main.rs`: every `AUTH_REF` → `MEMBERS_REF`. **Command-name decision:** - keep `git ents auth …` as the verb OR rename the subcommand group to - `git ents members …`. The plan names the *concept* "members"; renaming the - command is more honest but is user-facing porcelain churn. Recommended: rename - to `members` and update help text, since there is no compatibility to keep. - The `Set` impl `Signers` should point `REF` at `MEMBERS_REF`. -* `pre_receive` needs no logic change — it flows through `signers::load`. -* Grep the whole tree for `refs/meta/auth` and `AUTH_REF` to catch stragglers - (docs, comments, the README if it names the ref). - -=== P5.2 — Add `refs/meta/config` - -New file: `crates/git-ents/src/config.rs`; register `pub mod config;` in -`crates/git-ents/src/lib.rs`. - -[source,rust] ----- -pub const CONFIG_REF: &str = "refs/meta/config"; - -#[derive(Debug, Clone, Default, PartialEq, Eq, Facet)] -pub struct Config { - pub description: String, // was git's .git/description file - pub homepage: String, // "" when unset - pub topics: Vec<String>, // migrated off HEAD:.gitents/topics -} ----- - -* This is NOT a `MapDoc` (mixed fields) — use `git_store::Store::load`/`store` - directly. Provide `pub fn load(repo) -> Result<Config, Error>` (absent ref → - `Config::default()`) and `pub fn store(repo, &Config)`. -* Add the C1 fixed-format test. `Config` serializes as a tree with a - `description` blob, a `homepage` blob, and a `topics/` subtree (list) — verify - the actual on-disk shape `facet-git-tree` produces by writing one with - `Store::store` and inspecting `git ls-tree -r` in a scratch repo first, THEN - pin it with a hand-built fixture so the test is independent of the writer. - `write_meta_doc` only builds single-subtree map docs; you will need a small - extra builder (or extend `testutil`) for the `Config` shape. Match the - builder to whatever `facet-git-tree` actually emits. - -=== P5.3 — Migrate topics off the worktree, source chrome from config - -Files: `crates/git-ents-server/src/web/mod.rs`. - -* `gather_meta` currently reads `description` from the `description` file and - `topics` from `git cat-file -p HEAD:.gitents/topics` (≈ line 122). Replace - both with a single `git_ents::config::load(repo)` (off the async runtime via - `spawn_blocking`, like `load_checks` in `pages.rs`). `branch` and `releases` - stay as git calls. -* Delete the `.gitents/topics` read entirely. After this there must be **no** - reference to `.gitents` anywhere (grep to confirm). This is the "no worktree - metadata" principle landing. -* `RepoMeta.description`/`topics` now come from `Config`. Keep the - `homepage` available for the About card (P5.4 / overview aside). - -=== P5.4 — Settings as a projection - -Files: `crates/git-ents-server/src/web/pages.rs` (`settings_page`). - -* Make it a real projection over `members` (`signers::load`) + `config` - (`config::load`) + derived checks (`load_checks`) + releases. Sections: - General (description, homepage — now editable-shaped, but see below), Members - (the signer rows, reuse the `Render` impl for `Signer`), Features (derived, - read-only status — keep `feature_row`), Checks summary. -* **Editing:** the plan's model is per-section save (one ref per form) or one - `git push --atomic`. A server-side write path does not exist yet and is a - larger lift. Minimum bar for P5: render the *real* values from the typed refs - (not stubs) and drop the "Not available yet"/"reflect current configuration" - read-only hedging where the data is now real. If you add write endpoints, - that is new routing (coordinate with P6) and needs members-gated auth — flag - it and confirm scope before building. -* This removes the C5 dead wiring indirectly: see P5.5. - -=== P5.5 — Issues as typed meta-ref docs (fixes C5) - -New file: `crates/git-ents/src/issues.rs`; `pub mod issues;` in lib.rs. - -* Layout: `refs/meta/issues/<id>` per issue (self-contained; labels are plain - strings). Mirror the `runs` pattern in `checks.rs` (`RUNS_NS` + `Store::list` - + per-ref `Store::history`/`load`). -* Suggested doc: -+ -[source,rust] ----- -pub const ISSUES_NS: &str = "refs/meta/issues"; - -#[derive(Debug, Clone, PartialEq, Eq, Facet)] -pub struct Issue { - pub title: String, - pub body: String, - pub state: String, // "open" / "closed" - pub labels: Vec<String>, - pub author: String, -} ----- -* Provide `load(repo, id)`, `list(repo) -> Vec<(id, Issue)>` (via - `Store::list(ISSUES_NS)`), and a count helper. Add the C1 fixed-format test. -* `gather_meta`: set `RepoMeta.issues` to the open-issue count (kills the - hardcoded `0` — C5). The tab-count rendering in `tab_bar` already keys off - `meta.issues > 0`. -* `pages.rs::issues_page`: render the real list + an empty state, and derive the - label filter chips from the labels that actually exist. Drop the - "Not available yet" stubs for what is now backed by data. -* Issue *creation/editing* is a write path — same caveat as P5.4; confirm scope - before adding endpoints. - -=== P5.6 — Rendering - -Files: `crates/git-ents-server/src/web/render.rs`. - -* `Config` and `Issue` should render through the existing `Render` structural - walk (`render_peek`). Add `impl Render for Config {}` / `impl Render for - Issue {}` and only override `render` if a field needs domain formatting (e.g. - labels as chips). The whole point of keeping `Render` (P4) is that new typed - docs render for free; lean on that before writing bespoke markup. - -== P6 — routing (do last, High risk) - -Files: `crates/git-ents-server/src/http.rs`, `crates/git-ents-server/src/main.rs`. - -* Today everything hangs off `fallback(http::git)` plus the overlapping string - heuristics `wants_web_ui` / `is_git_path` / `is_browse_route` / - `is_service_request` / `is_receive_pack` / `query_service`. -* Target: explicit Axum routes for the git wire endpoints, and a separate web - router for the browser UI. The git wire surface is small and well-defined: - ** `GET /:repo*/info/refs?service=git-upload-pack|git-receive-pack` - ** `POST /:repo*/git-upload-pack` - ** `POST /:repo*/git-receive-pack` - ** dumb-HTTP: `GET /:repo*/HEAD`, `GET /:repo*/objects/*` - These go to `http-backend`; `is_receive_pack` (push detection for auto-init) - stays but becomes a property of the matched route, not a string sniff. - Everything else is the web router → `crate::web::render`. -* The nested-repo depth resolution (a repo is the shortest valid bare-repo - prefix, up to `MAX_REPO_DEPTH`) currently lives in `web::render` and - `repo_path`/`enclosing_repo`. Axum path patterns don't natively express - "shortest existing prefix", so you will likely keep a wildcard capture and - resolve the repo boundary inside the handler — but you can still split git vs. - web into two handlers/routers and delete the `wants_web_ui` disambiguation. -* **Safety net:** `tests/server.rs` (push/clone round-trips, nested repos, - colliding pushes) and `tests/pre_receive.rs` are the contract. They must stay - green. Add route-level tests (`http::tests::routes_browser_gets`, - `validates_segments`) for any new explicit routes. Because a repo can be named - `tree`/`blob`/`commit`/`HEAD`, keep those collision tests — push/clone to a - repo named `commit` must still work. -* `reconcile_head` (~45 lines fixing an unborn HEAD after first push) may shrink - once the matched route knows the pushed branch — opportunistic, not required. - -== Definition of done - -* `refs/meta/auth` no longer exists anywhere; `refs/meta/members` is the trust - root; `git ents` manages it. -* `refs/meta/config` and `refs/meta/issues/<id>` exist as typed docs, each with a - fixed-format load test. -* No `.gitents` reference remains in the tree. -* Settings and Issues render real data, not stubs; `RepoMeta.issues` is live. -* `http.rs` serves git via explicit routes; the heuristic predicates are gone or - reduced to route-local checks; all integration tests green. -* `cargo nextest run --workspace` and `cargo clippy --workspace --all-targets` - clean; `prek` hooks pass.
docs/simplification-plan.adoc @@ -1,251 +1,0 @@ -= Simplification & Adversarial Review Plan -:doctype: article -:toc: -:toclevels: 2 - -NOTE: Working plan from an adversarial review of the whole workspace (~5k LOC -across `git-ents`, `git-ents-server`, `git-store`). Goal: drastically simplify -while keeping all functionality. Not yet executed — pending decisions on P4/P5 -and the `tempfile` dependency promotion (P3). - -== Status - -* *Fixed and committed* (`c5984991`): the push bug. A recent refactor wrapped - each check command in a `CheckDef` struct, changing the `refs/meta/checks` - tree from `checks/<name>` blobs to `checks/<name>/command` subtrees, so - loading a check set written before the change failed with "object … is not a - tree" and broke every push. Reverted to a plain command map, which both - restores compatibility with the live data and removes the indirection. -* *Done and committed:* P8, P3, P7, P2, P1, P4 (no-op — `Render` kept), and - correctness items C1, C2, C3. C4 was *not* done — its premise no longer holds - (every page renders through `repo_shell`, so `gather_meta`'s git calls feed - visible chrome on every tab; re-evaluate after P5 moves topics/description - into `refs/meta/config`). C5 folds into P5 (Issues). -* *Done and committed:* P5 and P6 (handed off in `docs/p5-p6-handoff.adoc`). - The `refs/meta/auth` → `refs/meta/members` rename landed as a *hard break with - no fallback* — the live trust list is discarded and re-pushed after a - redeploy. `refs/meta/config` and `refs/meta/issues/<id>` are now typed - documents (each with a fixed-format load test); no `.gitents` worktree - metadata remains; Settings and Issues render real data through the `Render` - trait; and the git wire endpoints are served by explicit `get`/`post` routes - rather than the predicate-laden catch-all. C4 stays skipped — `gather_meta` - now reads `config` once instead of shelling out for description/topics, which - was its only real win. -* *Remaining:* nothing. The plan is fully executed. - -== Correctness / robustness findings (open) - -[cols="1,4",options="header"] -|=== -| # | Finding - -| C1 -| *Meta-ref format fragility, unguarded.* `refs/meta/auth` (`Auth`) and - `refs/meta/runs` (`RunDoc`) carry the same risk as the bug just fixed: any - incompatible change to the Facet struct silently breaks reading existing - on-disk data, surfacing only at push/render time. No format versioning, and no - test loads old-layout data. Add a round-trip-against-fixed-bytes test per - document type, and decide a policy (version field, or "never change a meta-ref - type incompatibly"). - -| C2 -| *Unbounded blob/diff rendering (web OOM).* `blob_page`, `blob_pane`, and - `commit_page` read arbitrarily large objects fully into memory via - `git cat-file -p` / `git show`; `is_binary` only samples 8 KB but the whole - object is still highlighted and emitted. Cap the rendered size. - -| C3 -| *Checks worker head-of-line blocks.* `worker` drains the queue through one - `spawn_blocking` call in sequence; a check may run up to `CHECK_TIMEOUT` - (30 min). One slow repo stalls every repo's checks. Acknowledged in comments - but a real multi-repo availability bug. - -| C4 -| *`gather_meta` shells out to git 4× per web request* (branch, description, - topics, releases) even on tabs that use none of it — e.g. opening a blob still - runs `tag --list` and `cat-file …/.gitents/topics`. Gather lazily per tab. - -| C5 (minor) -| `RepoMeta.issues` is hardcoded `0`, so the Issues tab count can never render — - dead wiring. -|=== - -== Simplifications (prioritized) - -[cols="1,3,1,1",options="header"] -|=== -| P | Change | Files | Risk - -| P1 -| *Collapse the auth/checks CLI duplication.* `sync_auth`/`sync_checks`, - `push_auth`/`push_checks`, `list`/`list_checks`, `add`/`add_check`, - `remove`/`remove_check`, `run_auth`/`run_checks` differ only by ref name and - noun. Unify into one generic meta-ref-set flow. (~200 lines.) -| `git-ents/src/main.rs` -| Low - -| P2 -| *Unify the meta-ref "named-string-pair set" document.* The "stored - `BTreeMap<String,String>` ↔ public `Vec` of two-field structs" pattern appears - three times: `Auth`→`Vec<Signer>`, `Checks`→`Vec<Check>`, and - `RunDoc`→`Vec<RunOutcome>`. Extract one generic helper (in `git-store`) and - delete the duplicated document structs and their `load`/`store`. -| `git-store`, `signers.rs`, `checks.rs` -| Low–Med - -| P3 -| *Delete two hand-rolled temp dirs.* `Scratch` (main.rs) and `TempDir` - (verify.rs) are identical Drop-cleanup temp dirs; `tempfile` is already a - workspace dependency (test-only today). Replace both with `tempfile::TempDir`. - *Requires promoting `tempfile` from dev- to normal dependency — needs OK.* -| `git-ents/src/main.rs`, `git-ents-server/src/verify.rs`, two `Cargo.toml` -| Low - -| P4 -| *Keep the Facet `Render` trait.* WITHDRAWN (was: cut it). Under the - schema-driven meta-ref direction (see "New direction" below) a generic - structural renderer is the mechanism that lets new meta-ref types render - without bespoke Rust, so it is load-bearing, not over-engineering. The two - overrides (`Signer`, `Run`) are the trait working as intended. -| `git-ents-server/src/web/render.rs` -| — (keep) - -| P5 -| *Make Settings & Issues functional via typed meta refs.* WITHDRAWN cut (was: - trim the stubs). Settings is a projection over `refs/meta/members` + - `refs/meta/config` + derived checks/releases; Issues become typed meta-ref - documents. `Render`'s structural walk presents them. See "New direction". -| `pages.rs`, new config module, `git-store` -| Med–High - -| P6 -| *Simplify request routing.* `wants_web_ui` / `is_git_path` / `is_browse_route` - / `is_service_request` / `is_receive_pack` / `query_service` are overlapping - string heuristics on a single catch-all fallback. Explicit Axum routes for the - git wire endpoints + a separate web router would remove most of them. - Integration tests (`server.rs`, `pre_receive.rs`) are the safety net. Do last. -| `git-ents-server/src/http.rs`, `main.rs` -| High - -| P7 -| *Micro.* Hoist the duplicated zero-oid const (3 copies); delete `tagline()` - (confirmed unused dead code). -| `main.rs`, `git-ents/src/lib.rs`, `server/checks.rs` -| Low - -| P8 -| *Delete `--max-requests` and the shutdown plumbing.* The flag is used only by - the `responds_and_shuts_down` test, which can `kill()` the child like the other - three tests do. Removes the flag, the counter middleware, the `Notify`, and - `with_graceful_shutdown`. (~25 lines.) -| `git-ents-server/src/main.rs`, `tests/server.rs` -| Low -|=== - -== Also noted (lower value) - -* Test helper duplication: a `unique_repo()` / `git init` temp-repo helper is - copied across `checks.rs`, `signers.rs`, `pre_receive.rs`. -* Server-side sync git invocations are spawned ad hoc in `verify.rs` and - `checks.rs`; the async `git_output`/`git_output_bytes` in `web/git.rs` is the - only shared wrapper and is web-private. -* `reconcile_head` is ~45 lines to fix an unborn `HEAD` after first push; could - shrink once the routing/handler knows the pushed branch. - -== New direction: meta-ref structure (decided) - -Rather than cutting the non-functional UI, make it real, backed by typed -meta-ref documents rendered through `Render`. *Path A (compile-time typed docs) -chosen* — each meta ref is a Rust `Facet` struct; the compiler holds the schema. -Path B (runtime schema / facet-styx) stays reachable on the same `Peek` + -facet-git-tree substrate, so nothing here is wasted, but is not built now. - -=== Ref layout - ----- -refs/meta/members write-access keys read ONLY by pre-receive -refs/meta/config repo metadata read by UI chrome / write path -refs/meta/checks check definitions self-contained -refs/meta/runs/<commit> run logs self-contained -refs/meta/issues/<id> issue docs (future) self-contained; labels are strings ----- - -* *`members`* (rename of today's `auth`) — the OpenSSH keys whose signed pushes - are accepted. Named for the project concept (*not* "contributors", which are - commit authors); the document stays keyed by fingerprint → key (a member is - one or more keys). Kept on its own ref because it is the *trust root*: - `pre-receive` parses exactly this one well-known document on every push, so - unrelated config can never widen its parse surface or share its history. - -* *`config`* — the repo's loose metadata promoted to first-class, members-gated, - versioned data. Today exactly: -+ -[source,rust] ----- -struct Config { - description: String, // was git's `.git/description` file - homepage: String, // "" when unset; the About card link - topics: Vec<String>, // migrated off the tracked HEAD:.gitents/topics file -} ----- -+ -`topics` is the important migration: today it lives in the worktree -(`HEAD:.gitents/topics`), versioned with code and editable by anyone who can -push content. Moving it into `config` makes it members-gated, out of the -worktree, and independently historied. - -*Principle: no project metadata in the worktree — there is never a `.gitents` -directory.* All project metadata lives on `refs/meta/*`. The lone offender is the -topics read at `web/mod.rs:122` (`HEAD:.gitents/topics`); the `topics` migration -removes it and the convention for good. - -=== Principles that fix the apparent tensions - -* *Decomposed refs, aggregated views.* Storage is normalized — one ref per - concern, each independently loadable. Presentation denormalizes — the Settings - page is a *projection* that reads members + config + (derived) checks/releases - and stacks them as sections. There is no `refs/meta/settings` god-doc; the - unification is in the renderer, not the data. (The repo header band already - aggregates this way.) -* *A content loader never reads another ref.* Config is read only by the write - path and by aggregating views (UI chrome), never by load-one-issue / - load-one-check. This is what keeps the components decoupled. -* *Editing a unified page over decomposed refs* is either per-section save (one - ref per form, as forges actually do) or one `git push --atomic …` across the - refs — the latter is exactly the "atomic updates across multiple refs" the - README claims, so the decomposition is what makes atomic settings meaningful. - -=== Deliberately excluded from config (the membership test) - -Belongs in config only if repo-global policy that *can't be derived from -content* AND is read by the write path or UI chrome (never a content loader): - -* *Default branch* — derivable; it is `HEAD`. -* *Primary language / license* — derivable from content (extensions; `LICENSE`). -* *Visibility (public/private)* — not derivable, but unenforced today (access is - open), so it would be a fake control. Add it when read-auth lands. -* *Feature toggles* — a toggle means only a deliberate opt-*out* of an otherwise - available feature; the UI already shows "enabled but empty" via blankslates, - and nothing needs opt-out yet. The Settings "Features" card shows *derived, - read-only* status, not fake switches. -* *Issue labels / templates* — component-owned; issues carry labels as strings, - and the index derives its filter set from the labels that exist. - -== Proposed execution order - -Each as its own commit, tests run between: - -. P8 → P3 → P7 (smallest, lowest risk) -. P2 → P1 (the structural dedup wins) -. P4 / P5 (after decisions) -. P6 (last, behind the integration tests) -. Correctness C1–C4 as separate hardening commits. - -== Open decisions - -None — all resolved. - -Resolved: P3 (`tempfile` → normal dependency, approved); P4 (keep `Render`); P5 -(make Settings/Issues functional); config altitude = Path A (compile-time); -write-access set named `members`; config = `{ description, homepage, topics }`; -decomposed refs with aggregated views; no worktree metadata (no `.gitents`).