diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 0000000..454ffe8 --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,19 @@ +# Cargo settings for this checkout. Committed on purpose: the documentation +# build has to be one command that behaves the same for everyone, and both +# settings below are part of that command rather than local preference. + +[alias] +# The documented doc build. --document-private-items is not optional: atgc is +# a binary crate, so almost nothing in it is meaningfully `pub` and a plain +# `cargo doc` produces a nearly empty page. --no-deps keeps the output to +# this crate; the dependency docs are on docs.rs. +docs = "doc --no-deps --document-private-items" + +[build] +# Deny rustdoc warnings, of which the one that matters is +# broken_intra_doc_links. The narrative pages under docs/ link into the code +# with intra-doc paths precisely so that renaming an item breaks the docs +# build instead of quietly leaving a dead link, and that only works if the +# warning is fatal. Set here rather than as a RUSTDOCFLAGS prefix so the +# documented command stays a single word with no environment to remember. +rustdocflags = ["-D", "warnings"] diff --git a/.gitignore b/.gitignore index 95ab61c..7599afc 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,12 @@ **/*.rs.bk *.pdb +# Generated documentation lands in target/doc, so /target above already +# covers it. Named here because it is the one build output someone might +# reasonably think belongs in the tree: it is the whole rendered docs site, +# and it is generated from docs/ plus src/ by `cargo docs`. Never commit it. +/target/doc + # Cargo.lock is tracked on purpose: atgc is a binary crate, so the lockfile # pins the exact dependency versions a build is reproduced from. Do not # ignore it. diff --git a/.tangled/workflows/ci.yml b/.tangled/workflows/ci.yml index 936e72f..a4d15f9 100644 --- a/.tangled/workflows/ci.yml +++ b/.tangled/workflows/ci.yml @@ -48,3 +48,24 @@ steps: # No tests exist yet. Kept so the step is wired up when they land. - name: "test" command: "cargo test --locked" + + # The documentation build doubles as a link check: .cargo/config.toml sets + # rustdocflags = -D warnings, so a narrative page under docs/ that links to + # an item that has been renamed fails here rather than shipping a dead + # link. `cargo docs` is the alias documented in docs/writing-documentation.md; + # spelled out here so a reader of this file does not have to go and look it + # up, and --locked to match the steps above. + # + # UNVERIFIED, like everything else in this file, and in one extra way: no + # runner has executed any of it, so this step's dependency on the nixpkgs + # rustdoc coming with cargo (rather than as its own attr, the way clippy and + # rustfmt do above) is a guess. If it fails to find rustdoc, that is the + # first thing to check. + - name: "docs" + command: "cargo doc --no-deps --document-private-items --locked" + + # The two checks the doc build cannot make: untested rust code fences, and + # relative links between docs/ pages whose target file is gone. Needs only a + # POSIX shell, awk and grep. + - name: "doc lint" + command: "sh scripts/doc-lint.sh" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6a9f3c2..dd5c8e5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -23,6 +23,21 @@ left to manual checks and how to run those, and the two constraints that trip people up: `HOME` cannot be redirected in a test, and the working directory is process-wide. +## Docs + +``` +cargo docs +``` + +An alias for `cargo doc --no-deps --document-private-items`, defined in +`.cargo/config.toml`. It renders the narrative pages under `docs/` and the +documentation generated from `src/` into one tree at +`target/doc/atgc/index.html`. `.cargo/config.toml` also denies rustdoc +warnings, so this is where a prose page linking to an item that no longer +exists is caught. [docs/writing-documentation.md](docs/writing-documentation.md) +covers the rules — chiefly that no `docs/` page may contain an untested Rust +example, because rustdoc does not run doctests for binary crates. + ## Commits ### prek diff --git a/README.md b/README.md index 5e2703b..3342839 100644 --- a/README.md +++ b/README.md @@ -36,53 +36,30 @@ atgc repo clone # clone a Tangled repo and set its git identity atgc repo list # list an account's repos ``` -## Accounts - -Several accounts can be logged in at once; `auth login` is additive. Tokens -live in `~/.config/atgc/sessions.json`, the accounts themselves in -`~/.config/atgc/accounts.json` (both 0600). They are separate files because -identities outlive credentials: a session that runs out leaves the account -listed and selectable, it does not log you out. - -Accounts are keyed by DID, which never changes hands. Handles are for typing -and display, and are re-resolved whenever you type one. If a handle now -resolves to a DID other than the one it's cached under, atgc refuses and -asks you to name a DID rather than guess. If resolution is unavailable it -falls back to the cached mapping with a warning. - -Every OAuth operation — token requests, refreshes, session reads and -writes, prunes, logouts — is appended to `~/.config/atgc/oauth.jsonl` (0600), -one JSON object per line. It exists because a session was twice destroyed -without warning and nothing recorded what had touched it. It carries the -`client_id` actually sent to the token endpoint, the endpoint's error code -and body when it refuses, and a per-invocation id with the pid and -sub-millisecond timestamps, so two concurrent `atgc` processes interfering -with each other is visible after the fact. - -No secret ever goes in it: tokens, authorization codes and PKCE verifiers -appear only as the first 8 hex characters of a SHA-256, which is enough to -match two sightings of one token and useless for anything else. Read it with -`jq`, for example the token endpoint's own words on the last refusal: +Every flag and default is in `atgc --help`. -``` -jq -c 'select(.event == "token_refused")' ~/.config/atgc/oauth.jsonl | tail -1 -``` +## Documentation -Set `ATGC_OAUTH_LOG` to log somewhere else, or to `0` to switch it off. It -rotates to `oauth.jsonl.1` at 8 MiB, so it is bounded at about 16 MiB. +`docs/` holds the conceptual material — the things worth reading before any +individual command makes sense. Start with [`docs/index.md`](docs/index.md), +which has a suggested order; the short version is: -Which account is active, highest precedence first: +- [Architecture](docs/architecture.md) — PDS, knot, appview and spindle, and + which commands talk to which. Read this one first. +- [Pull requests](docs/pull-requests.md) — how a Tangled PR is stored, and + why `atgc pr create` needs no access to the repo it targets. +- [Accounts](docs/accounts.md) — several accounts at once, why DIDs are the + key and handles are not, and how atgc picks which one to act as. +- [Sessions](docs/sessions.md) — OAuth, what "expired" means, why an account + outlives its token, and the OAuth event log in + `~/.config/atgc/oauth.jsonl`. -1. `--account ` -2. `ATGC_ACCOUNT=` -3. the checkout's own `user.email` in `.git/config`, when it holds a DID — - so working in a Tangled repo acts as the account that repo belongs to -4. the account last given to `atgc auth switch` -5. the only logged-in account, if there is exactly one +Those pages are also compiled into the API documentation, so prose and +generated reference are one build: -`atgc auth status` lists every account, marks the active one and says what -selected it. `pr create`, `pr resubmit`, `repo create` and `repo clone` -print an `acting as …` line before doing anything, `--dry-run` included. +``` +cargo docs +``` ## Cloning @@ -96,27 +73,13 @@ it falls back to HTTPS, which needs neither a key nor a login. `--ssh` and ## Checkout identity -`repo create` and `repo clone` write a Tangled `[user]` section into the -checkout's **local** config — never your global or system one: - -``` -[user] - name = @your.handle - email = did:plc:... -``` - -`user.name` is the active account's `@`-prefixed handle and `user.email` is -its DID, which is how Tangled attributes a commit to an account rather than -to an email address. It is also what makes precedence rule 3 above work: a -checkout created or cloned as an account goes on acting as that account. - -Nothing is ever overwritten. If the checkout already has a local `user.name` -or `user.email` that disagrees, atgc prints what is there alongside what it -would have set, and writes neither — half an identity is worse than none. -An identity inherited from `~/.gitconfig` counts as absent, since that is -the case this exists to fix. `--no-git-config` skips the step. Cloning with -no account selected still works: you get a note saying why instead of an -identity. +`repo create` and `repo clone` write a Tangled `[user]` section — handle as +`name`, DID as `email` — into the checkout's **local** git config, never +your global or system one. That is how Tangled attributes a commit to an +account rather than to an email address, and it is what makes a checkout go +on acting as the account it belongs to. Nothing is ever overwritten; +`--no-git-config` skips it. [docs/accounts.md](docs/accounts.md) has the +rest, including what happens when a checkout already disagrees. ## Development diff --git a/TODO.md b/TODO.md index fd25933..5de2c54 100644 --- a/TODO.md +++ b/TODO.md @@ -165,6 +165,18 @@ - [x] --debug / ATGC_DEBUG=1 — HTTP traffic, token claims, full error bodies - [x] prek hooks over file hygiene, `cargo fmt --check` and clippy `-D warnings`, on the pinned toolchain (prek.toml, rust-toolchain.toml) +- [x] Documentation: narrative pages in docs/ (architecture, pull requests, + accounts, sessions), compiled into rustdoc through src/docs.rs so prose + and generated reference are one build — `cargo docs`, aliased in + .cargo/config.toml, with rustdocflags = -D warnings making a + prose-to-code link that no longer resolves a build failure. mdBook was + rejected: a second tool outside the rust-toolchain.toml pin, and it + cannot check the links that rot +- [ ] Publish the built docs somewhere. target/doc is a self-contained static + tree with no external requests, and Tangled hosts static sites, but + building it on a push needs a spindle this repo does not have +- [ ] Narrow the `blob:*/*` scope. Patch blobs are application/gzip; the + scope grants every MIME type, which is wider than anything atgc uploads - [ ] Add held-back scopes when their commands ship: publicKey (ssh-key), secrets, repo.delete/deleteBranch, collaborator/membership changes - [ ] Drop the vendored jacquard-oauth once the rpc `?aud=*` serialization diff --git a/docs/accounts.md b/docs/accounts.md new file mode 100644 index 0000000..b5d099f --- /dev/null +++ b/docs/accounts.md @@ -0,0 +1,219 @@ +# Accounts + +atgc can hold sessions for several ATProto accounts at once, and every +command decides which one it is acting as before it does anything. This page +is about how that decision is made and why it is made that way. The code is +[`crate::account`], which carries the same argument in more detail at the +point where it is implemented. + +## Two files, because credentials expire and identities do not + +State lives in two files under `~/.config/atgc/`, both mode 0600 (a third, +`oauth.jsonl`, sits beside them, but it is a log written and never read back +— see [Sessions](sessions.md)): + +- **`sessions.json`** — jacquard's own session store, holding OAuth tokens + keyed by `oauth:/`. atgc does not own its format and + writes nothing of its own into it. +- **`accounts.json`** — atgc's registry: which accounts it knows about, a + cached handle for each, when that handle was last confirmed, and the + "active account" pointer that `atgc auth switch` moves. + +They are separate on purpose. A public OAuth client's sessions are capped at +two weeks, so tokens run out routinely, and an account whose token has +lapsed must not disappear from `atgc auth status`. Keeping the registry +apart from the store means an expired account still lists, still resolves, +and can be re-authorized by name. "Logged out" and "the token ran out" are +different states and only the first should require typing a handle again. +See [Sessions](sessions.md). + +Both files are caches of things that can be rebuilt, with one exception: the +registry is the only record that an account *exists* for atgc's purposes if +its session has been deleted. So a damaged `accounts.json` is not fatal — an +unparseable one is noted under `--debug` and an unreadable one is passed over +in silence, and either way atgc carries on with an empty registry and +recovers the account list from the session store. + +The two are unioned when listing. Anyone who logged in before the registry +existed has a session and no registry entry, and still sees their account. + +## DIDs are the key. Handles are not. + +The DID is what everything is addressed by: registry keys, the active +pointer, the session store, and the `[user] email` line that Tangled +checkouts carry. A DID never changes hands, so selecting by DID can never +select the wrong account. + +A handle is the opposite trade. It is the thing a human can type and the +thing worth displaying, and it is mutable: an account can rename itself, and +a handle that has been released can later be registered by a *different* +account. So a cached handle is treated as a hint and never as proof: + +- Selecting by DID — repo config, the active pointer, `--account did:…` — + touches no network at all. +- Selecting by handle re-resolves it through + `com.atproto.identity.resolveHandle` and believes the answer over the + cache. A plain rename is invisible and harmless, because the DID did not + move; atgc repairs the cached handle on the way past. +- If the handle resolves to a DID that is *not* the one atgc has it cached + under, and the cached DID is an account you hold, that is a hard error. +- If resolution fails outright — offline, or the resolver is down — the + cached mapping is used with a warning on stderr, so the tool still works + on a plane. + +The hard error is the interesting case, and it is worth being precise about +when it fires. It requires both halves: the handle must now resolve +somewhere else, *and* atgc must be holding an account cached under that +handle. Both "act as the DID it used to mean" and "act as the DID it means +now" are plausible readings, and one of them publishes work under the wrong +identity, permanently and in public. There is no safe guess. There is also +no prompt, because atgc is meant to run unattended in scripts and under +agents, where a prompt either hangs forever or gets answered by whatever +happens to be on stdin. So it refuses, names both DIDs, and asks for an +unambiguous `--account `. + +If the handle resolves to a DID you simply have no session for, that is a +plain "no session for @handle" error, not the ambiguity refusal. + +## Which account a command acts as + +Highest precedence first: + +1. `--account ` +2. `ATGC_ACCOUNT=` +3. the checkout's own `user.email` in repo-local `.git/config`, when it + holds a DID +4. the account last named to `atgc auth switch` +5. the only logged-in account, if there is exactly one + +If none of those settles it — more than one account, no active pointer — +atgc refuses and lists what it knows, rather than picking. + +Each rank exists for a reason the one below it cannot serve. + +**`--account`** is the per-invocation override, and the only thing that can +resolve the ambiguity refusal above. + +**`ATGC_ACCOUNT`** exists because `--account` cannot be threaded through a +wrapper script, a CI step, or an agent that shells out to `atgc`. Exporting +one variable for a subshell is the natural way to say "this whole session is +that account". It sits below the flag so a single command can still override +it, and `atgc auth status` names it as the source, so an exported value can +never quietly persist unnoticed. + +**The checkout's `user.email`** is the one that surprises people, and it is +the most useful. Tangled checkouts carry `[user] name = @handle` and +`[user] email = did:plc:…`, written at clone or create time. That is a +per-repo, already-present, stable statement of which account the checkout +belongs to — exactly the selector this wants, costing no resolution because +it is already a DID. Working inside a Tangled repo therefore acts as the +account that repo belongs to, without being told. + +It is read with `git config --local` deliberately. A DID in someone's global +git config would follow them into every checkout, which is the opposite of +what a per-repo identity is for. An ordinary email address in that field is +ignored, which is the state every checkout not created through atgc is in. + +There is a deliberate hard failure here: if the checkout names a DID that +atgc holds no session for, the command stops and says so. Falling through to +some other account would be the worst behaviour available, since the +checkout has stated whose it is and publishing under somebody else's +identity is public and permanent. + +### Where that line comes from + +`atgc repo create` and `atgc repo clone` write it. Both put a Tangled +`[user]` section into the new checkout's **repo-local** config, and never +into a global or system one: + +```text +[user] + name = @your.handle + email = did:plc:... +``` + +`user.name` is the selected account's `@`-prefixed handle and `user.email` +is its DID. That is how Tangled attributes a commit to an account rather +than to an email address, and it is what closes the loop with rule 3 above: +a checkout created or cloned as an account goes on acting as that account +without ever being told again. + +The handle is re-read from the DID document at write time rather than taken +from the registry cache, falling back to the cache only if the lookup fails. +A cached handle is a hint everywhere else in this document; here it is about +to be baked into commits, so it is worth one request to be sure. The DID +needs no lookup — the selection already holds it, and it is the half that +cannot be wrong. + +Nothing is ever overwritten. If the checkout already has a local `user.name` +or `user.email` that disagrees, atgc prints what is there beside what it +would have set and writes neither: a value that is already there was set +deliberately, and half an identity is worse than none. An identity inherited +from `~/.gitconfig` reads as absent, on purpose — that is the case this +exists to fix. `--no-git-config` skips the step entirely. + +Failing to write an identity is never fatal. Cloning a public repo needs no +account at all, so a clone with no account selected still succeeds and +prints the reason the identity was skipped, using the selection error's own +wording, which already says what to do about it. + +**`auth switch`** sets a persisted default in the registry. It is only a +default, and `atgc auth switch` says so out loud when the current checkout's +`.git/config` names a different account and will therefore keep winning +here — a pointer that moved but changes nothing where you are standing is a +trap worth naming. + +**A sole account** is the last rank so that the single-account case needs no +configuration at all. + +`atgc auth login` also moves the active pointer, because logging in is an +explicit statement about who you are right now. It moves only the pointer: +a repo that names a different DID still wins inside that checkout. + +## Saying so before writing + +The commands that publish something, or that write an identity into a +checkout — `pr create`, `pr resubmit`, `repo create`, `repo clone` — print a +line before doing anything: + +```text +acting as @permadeath.com (did:plc:nlzmjyfv6loqtxyzvdcznwgf) (via this repo's .git/config) +``` + +The "via" clause is the point. A surprising choice of account is traceable +to the thing that caused it, from the output, without running anything else. +The line is printed under `--dry-run` too, so a dry run answers "who would +this be from?" as well as "what would it contain?". + +Read-only commands stay quiet. They resolve an account too, but nothing they +do is public or permanent, so the line would be noise. + +`atgc auth status` is the full version: every account, the active one +marked, what selected it, and the session state of each. It re-reads every +handle from its DID document rather than trusting the cache, and repairs the +cache as it goes — it is the command you run when you want the truth rather +than the fast answer. It also prints even when account selection *fails*, +which matters because a failed selection is exactly the thing you run it to +diagnose. + +## Logging out + +`atgc auth logout` removes one account — the active one, or a handle or DID +you name. `--all` removes every account. A bare `logout` used to wipe every +session; with more than one account that is far too blunt, so it was +narrowed. The change can only ever destroy less than it used to, never more. + +Logout is local. The tokens are deleted from the session store, not revoked +at the PDS, and they expire on their own schedule. Half-finished login +states are cleared whenever anything is logged out, since they belong to +nobody in particular. + +## In the code + +- [`crate::account`] — the whole model, and the module documentation says + much of this from the other direction. +- [`crate::account::select`] — the precedence chain. +- [`crate::account::Selection`] and [`crate::account::Source`] — what was + chosen and what chose it. +- [`crate::account::repo_did`] — the `.git/config` selector. +- [`crate::account::resolve_handle`] — the one place a handle becomes a DID. diff --git a/docs/architecture.md b/docs/architecture.md new file mode 100644 index 0000000..83134a5 --- /dev/null +++ b/docs/architecture.md @@ -0,0 +1,189 @@ +# Architecture + +Tangled is not one service. It is four kinds of service that agree on a set +of record types, and `atgc` is a client of all of them at once. Which one a +command talks to decides what it needs — a token, a network, a repo, or +nothing — and most of the surprising behaviour in this tool falls out of +that split rather than out of anything atgc does. + +## The four services + +### PDS — the user's Personal Data Server + +Every ATProto account has one. It holds the account's records and blobs, and +it is the only party that can write them. On Tangled that means the PDS is +where the interesting things live: `sh.tangled.repo` records for repos you +own, `sh.tangled.repo.pull` records for pull requests you have opened +anywhere, `sh.tangled.publicKey` records for the SSH keys you push with, and +the gzipped patch blobs a pull request's rounds point at. + +atgc finds an account's PDS by reading the `AtprotoPersonalDataServer` +service entry out of its DID document — [`crate::auth::pds_from_did_doc`]. +It never has to be told where a PDS is. + +The PDS is also the OAuth authorization server: `atgc auth login` sends you +to *your* PDS's consent page, not to anything Tangled runs. And it is what +mints the short-lived service-auth tokens that authorize calls to a knot, +via `com.atproto.server.getServiceAuth`. So the PDS is the root of trust for +every write atgc makes, direct or indirect. + +Reads from a PDS do not need a token. `com.atproto.repo.getRecord` and +`listRecords` are public, which is why [`crate::pr::resubmit`] can read your +own pull record back before it has authenticated anything, and why +[`crate::ssh::find_push_identity`] can list your registered keys. + +### Knot — the git host + +A knot is the thing that actually stores git objects. It serves the repo +over SSH (`git@tangled.org:owner/name`, which routes to a knot) and over +smart HTTP, and it exposes a small XRPC surface of its own — +`sh.tangled.repo.create` is the one atgc calls today; merge, fork, branch +deletion and collaborator management exist and do not have commands yet. + +Two things about knots matter for reading the code: + +**A knot has a DID of the form `did:web:`.** That is the audience +atgc asks the PDS to mint a service-auth token for, in +[`crate::repo::create`]. The knot then trusts the token because it can +verify it against the signing key in your DID document. The knot never sees +your OAuth token. + +**Repos have DIDs, minted by the knot.** As of knot v1.13 a repository is +itself an ATProto identity, distinct from the DID of the account that owns +it. This is what makes a rename safe and what a pull request's `target` +points at. It is also why `atgc` has to resolve a git remote to a repo DID +before it can do anything with a repo it does not own: clone URLs redirect +to `https:////`, so [`crate::resolve::repo_ref`] follows the +same redirects git itself would follow and reads the DID out of the +`Location` headers. If the remote URL already contains a DID, that costs no +network at all. + +The default knot is `knot1.tangled.sh`, hardcoded as `repo create --knot`'s +default. Discovering it from your membership records instead is on the +roadmap and has not been done. + +### Appview (Bobbin) — the read-only index + +Nothing above answers "what pull requests exist on this repo". Records are +scattered across the PDSes of everyone who has ever opened one, so answering +that question means having consumed the firehose and built an index. Bobbin +is Tangled's appview and does exactly that. + +atgc reads it at `https://api.tangled.org`, overridable with the +`ATGC_BOBBIN` environment variable ([`crate::pr::bobbin_base`]). Four +endpoints are used: `sh.tangled.repo.listPulls`, +`sh.tangled.repo.listPullsBy`, `sh.tangled.repo.listRepos`, and +`sh.tangled.repo.getRepoByRepoDid`. All are unauthenticated, which is why +every listing command in atgc works with no session and works for accounts +that are not yours. + +**Bobbin can lag, and the web view is a different index.** `tangled.org` +does not read from `api.tangled.org`; it maintains its own. So the web page +for a repo can show a pull request that Bobbin has not indexed yet — hours +later, in practice. This is not hypothetical and it is not an atgc bug, so +`atgc pr list` distinguishes "no pull requests in this state" from "Bobbin +has nothing at all for this repo", and says so in the second case. When a +listing looks wrong, check the web view before believing it. + +### Spindle — the CI runner + +Spindles execute the workflows in `.tangled/workflows/`. atgc does not talk +to one. The OAuth scopes it requests include +`rpc:sh.tangled.ci.triggerPipeline` and `cancelPipeline`, granted ahead of +the commands that will use them so that shipping those commands does not +force a re-login, but no code path in the tree calls either today. + +This repo has no spindle attached. `.tangled/workflows/ci.yml` exists, and +has never executed once — its schema has not been validated by a runner, so +nothing in it should be read as known-good. See the comments in that file +and the pipelines section of [TODO.md](../TODO.md). + +## Everything else atgc talks to + +Two more hosts appear in the code and belong to neither Tangled nor you: + +- **`plc.directory`** — resolves `did:plc:` DID documents, which is where + handles and PDS endpoints come from. `did:web:` accounts are resolved from + `https:///.well-known/did.json` instead. Both in + [`crate::auth::handle_from_did_doc`] and `pds_from_did_doc`. +- **`public.api.bsky.app`** — used for `com.atproto.identity.resolveHandle` + (handle to DID, in [`crate::account::resolve_handle`]) and for + `app.bsky.actor.getProfile`, which supplies the display name and avatar on + the login success page. The profile lookup is decorative and its failure + is ignored; the handle resolution is not, and its failure has a documented + fallback (see [Accounts](accounts.md)). + +Using a Bluesky-operated resolver for a tool that has nothing to do with +Bluesky is a real dependency worth being aware of, not a design statement. + +## Which commands talk to which + +Every row is what the command does today, read off the code rather than +intended. "DID docs" means `plc.directory` or a `did:web:` host. + +| command | PDS | knot | Bobbin | DID docs | bsky.app | +| --- | --- | --- | --- | --- | --- | +| `about` | | | | | | +| `auth login` | OAuth, token exchange | | | handle | profile | +| `auth status` | | | | handle, PDS | | +| `auth switch` | | | | | resolveHandle | +| `auth refresh` | token exchange | | | | | +| `auth token` | token exchange | | | | | +| `auth logout` | | | | | resolveHandle | +| `browse` | | redirect probe | | | | +| `pr create` | uploadBlob, createRecord | redirect probe | | | | +| `pr list` | | redirect probe | listPulls | author handles | | +| `pr status` | | | listPullsBy, getRepoByRepoDid | owner handles | resolveHandle | +| `pr view` | | redirect probe | listPulls | author handle | | +| `pr resubmit` | getRecord, uploadBlob, putRecord | redirect probe | | PDS lookup | | +| `repo clone` | listRecords | redirect probe, git clone | | handle, PDS | | +| `repo create` | getServiceAuth, createRecord, listRecords | repo.create, git push | | handle | profile | +| `repo list` | | | listRepos | handle | resolveHandle | + +Two things the table leaves out, because they are true of nearly every row. +Any command given `--account ` resolves that handle through +`bsky.app` before doing anything else, whatever the row says. And every +command that authenticates hands a DID to jacquard, which resolves the DID +document itself as part of restoring the session — so "DID docs" in the +table means atgc's own explicit lookups, not every request that reaches +`plc.directory`. + +Things worth reading off that table: + +- **`about` touches nothing.** It is the only command that works with no + network and no configuration. +- **`pr create` never touches Bobbin, and never touches the target repo's + knot except to ask where it is.** The redirect probe is a public GET + against the clone URL. Nothing is pushed, nothing is forked, and no + permission on the target repo is required or checked. See + [Pull requests](pull-requests.md). +- **Everything that reads is unauthenticated.** `pr list`, `pr status`, + `repo list`, `browse` and the read half of `pr view` need a DID and a + network, not a token. That is why they keep working for an account whose + session has run out — see [Sessions](sessions.md). +- **`auth logout` reaches the network only to resolve a handle you typed.** + It deletes tokens locally; it does not revoke them at the PDS. +- **`repo create` is the only command that talks to a knot as an + authenticated client**, and the only one that runs `git push`. +- **`repo clone` is best-effort about almost everything.** The redirect + probe confirms the repo exists and learns its DID, but git is about to + make the authoritative attempt, so a failed lookup does not pre-empt a + clone that would have worked. The PDS lookup is only there to list the + account's registered SSH keys, which decides whether an SSH clone can be + expected to work; over HTTPS the whole command needs no account at all. + +## Where this leaves atgc + +atgc holds no state of its own beyond two files in `~/.config/atgc/` +(accounts and OAuth sessions — see [Accounts](accounts.md)), plus an +append-only log of its own OAuth operations that nothing reads back (see +[Sessions](sessions.md)), and it has no server. Everything it does is either +a local `git` invocation, a public +read, or a write to your own PDS. There is no atgc account, no atgc +database, and nothing to migrate if you delete it. + +The one consequence worth stating: because atgc writes to your PDS and the +appview indexes asynchronously, a successful write is not the same as a +visible result. `pr create` printing `created at://…` means the record +exists. Whether Bobbin has noticed is a separate question with its own +answer time. diff --git a/docs/index.md b/docs/index.md new file mode 100644 index 0000000..a73a06a --- /dev/null +++ b/docs/index.md @@ -0,0 +1,88 @@ +# Documentation + +The conceptual half of atgc's documentation: the things a reader needs +before any individual command makes sense. Command syntax is in +`atgc --help`, which is generated from the same source as the +CLI itself and cannot go stale; this is the part that can, so it says what +it is sure of and what it is not. + +These pages are plain CommonMark under `docs/`. They are readable in a repo +browser or an editor exactly as they sit, and they are also compiled into +the rustdoc output, where they appear alongside the module, type and +function documentation generated from the code. One build, one tool, no +tooling outside the pinned toolchain. See [Writing documentation] for why +it was set up that way and what the arrangement costs. + +## Reading order + +Roughly the order in which the ideas depend on each other: + +1. [Architecture] — PDS, knot, appview and spindle, and which commands talk + to which. Nothing else here makes sense first. +2. [Pull requests] — how a Tangled PR is actually stored, and why + `atgc pr create` needs no access to the repo it targets. +3. [Accounts] — several accounts at once, why DIDs are the key and handles + are not, and how atgc decides which account a command acts as. +4. [Sessions] — OAuth, what "expired" means, why an account outlives its + token, and the append-only log of every OAuth operation. +5. [Vendored dependencies] — why there is a patched copy of a crate in the + tree, and what has to happen for it to leave. +6. [Writing documentation] — the docs build, and the rule about examples. + +## Building the reference documentation + +```text +cargo docs +``` + +That is an alias, defined in `.cargo/config.toml`, for: + +```text +cargo doc --no-deps --document-private-items +``` + +The output is a self-contained static tree at `target/doc/atgc/index.html`, +covered by `.gitignore`. `--document-private-items` is not optional: atgc is +a binary crate, so almost nothing in it is `pub` in a way that means +anything, and a default `cargo doc` on it produces a nearly empty page. + +`.cargo/config.toml` also sets `rustdocflags = ["-D", "warnings"]`, which is +what turns a broken cross-reference from prose into code — the main way a +docs folder rots — into a failed build rather than a dead link. Adding +`--open` opens the result in a browser. + +The build needs no network beyond whatever the crate's dependencies already +needed, and no tool that is not already in `rust-toolchain.toml`. + +## Where the reference documentation lives + +Every module in `src/` carries its own documentation, and several of them +carry a lot of it: `crate::account` on identifier handling, +`crate::auth` on the OAuth client, `crate::oauthlog` on the event log and the +reasoning behind its format, `crate::models` on the record shapes, +`crate::resolve` on turning a git remote into a repo DID. The pages here do +not repeat those; they link into them, and the links are checked at build +time. + +## Documentation that is not here + +- **`atgc --help`** — every flag and default. Generated from the + clap definitions in `main.rs`. +- **[CONTRIBUTING.md](../CONTRIBUTING.md)** — building, the commit hooks, + and why pull requests go through `atgc pr create`. +- **[TESTING.md](../TESTING.md)** — what earns a test here and what does + not, the two constraints that make parts of this crate untestable, and + the list of things that stay manual. It lives at the repo root rather + than in this folder, which is arguable; see the pull request that added + this folder. +- **[TODO.md](../TODO.md)** — what has shipped and what has not, in more + detail than a roadmap usually gets. It is the honest answer to "does atgc + do X yet". +- **[README.md](../README.md)** — what atgc is and the command list. + +[Architecture]: architecture.md +[Pull requests]: pull-requests.md +[Accounts]: accounts.md +[Sessions]: sessions.md +[Vendored dependencies]: vendored-dependencies.md +[Writing documentation]: writing-documentation.md diff --git a/docs/pull-requests.md b/docs/pull-requests.md new file mode 100644 index 0000000..676e51d --- /dev/null +++ b/docs/pull-requests.md @@ -0,0 +1,142 @@ +# How a Tangled pull request works + +The shape of a Tangled pull request is the single most useful thing to +understand about this tool, because it explains why `atgc pr create` can do +what it does from a laptop with no push access, no fork, and no branch +anywhere but locally. + +## The record lives in the author's PDS + +A pull request is a `sh.tangled.repo.pull` record. It sits in the *author's* +own repository of records, on the author's own PDS, even when it targets +somebody else's repo on somebody else's knot. The appview picks it up off +the ATProto firehose and indexes it against the target. + +Everything else follows from that. Opening a pull request is a write to your +own account and nothing more. It does not need permission on the target +repo, because it does not touch the target repo. It does not need the target +knot to be reachable for anything except answering "what is your repo DID", +and that is a public GET. There is no fork step, because a fork exists to +give you somewhere you are allowed to write, and you already have somewhere. + +The record needs exactly one piece of information about the target: the +repo's DID. atgc gets it from the git remote, by following the same +redirects a `git clone` would follow — see [Architecture](architecture.md) +and [`crate::resolve::repo_ref`]. + +## Patch-based, in rounds + +atgc opens patch-based pull requests. `pr create` runs + +```text +git format-patch --stdout /..HEAD +``` + +gzips the result, uploads it to your PDS as an `application/gzip` blob, and +references the blob from the record. The branch is never pushed anywhere. +For a branch of any normal size the compressed patch is a few kilobytes, and +the whole exchange is two writes to your own PDS. + +A record's `rounds` array is the revision history. Each round is a blob +reference plus a timestamp: + +```json +{ + "$type": "sh.tangled.repo.pull", + "title": "Implement pr resubmit", + "target": { + "repo": "did:plc:…", + "repoDid": "did:plc:…", + "branch": "main" + }, + "rounds": [ + { "patchBlob": { "$type": "blob", "…": "…" }, "createdAt": "…" } + ], + "createdAt": "…" +} +``` + +`atgc pr resubmit --pr ` appends a round. Because the record is in +your own PDS, that is a read-modify-write against yourself: read the record +back (with `com.atproto.repo.getRecord`, which is public and needs no +token), upload a fresh gzipped patch of the current branch against the +target branch the record already names, push it onto `rounds`, and +`putRecord` the whole thing. + +Earlier rounds are left exactly as they were. Tangled keeps every round +addressable at `/pulls//round/`, so a resubmit revises a pull request +rather than erasing what was already published. This is also the one place +in atgc where a bug could damage a real pull request, since the whole record +is rewritten each time — the target branch is taken from the stored record +rather than recomputed for exactly that reason. + +`--dry-run` on either command builds the patch and sends nothing. `pr create` +prints the target repo and branch, the commit count, the raw and gzipped +sizes, and the title; `pr resubmit` prints the same sizes plus the pull +request it would append to and the round number it would become. + +## What the record does and does not say + +**`target.repo` and `target.repoDid` both hold the target repository's own +DID** — not the owner's account DID. Repos have had their own DIDs since +knot v1.13. Live records carry the value under both names, so atgc writes +both. Records from before that change instead carry a `targetRepo` at-URI +whose authority is the repo *owner's* DID, and atgc's listing commands still +read that shape, treating it as close enough for display. + +**`source` is not set.** It is where a branch-based pull request would name +the branch it comes from, and a patch-based one has no such branch. This has +a visible consequence: `atgc pr view` matches pull requests by +`source.branch` first, and since atgc's own pull requests carry no source, +it falls back to "your newest pull request on this repo". That fallback is +not a nicety, it is the path atgc's own PRs actually take. + +**Nothing in the record is a pull request *number*.** The `/pulls/12` +numbering is the appview's, not the record's, which is why `atgc pr view` +links to the repo's pulls page rather than to the pull request itself. Fixing +that means asking the appview, and has not been done. + +**Old records had no `rounds` at all**, just an inline patch string. The +listing code treats a missing `rounds` array as a single round so those +still display. + +## Reading pull requests + +Reads go to Bobbin, the appview, and are unauthenticated: + +- `atgc pr list` — `sh.tangled.repo.listPulls` for the repo the current + checkout's remote points at. +- `atgc pr status` — `sh.tangled.repo.listPullsBy` for an account, across + every repo. `--author` points it at somebody else. +- `atgc pr view` — the same listing, filtered to the current branch's pull + request. + +Because these need only a DID, they work for an account whose OAuth session +has run out, and for accounts that are not yours at all. + +They also inherit the appview's indexing lag. An empty `pr list` on a repo +that visibly has pull requests on the web usually means Bobbin has not +caught up, not that anything is wrong — `tangled.org` runs a separate index +and is often ahead. atgc distinguishes the two cases: if a state filter +returns nothing but the repo has *some* pulls indexed, it says "no open pull +requests"; if the repo has none at all, it says so and names the lag as the +likely reason. + +## What atgc does not do yet + +Merging, commenting, closing, reopening, editing, checking out a pull +request's patch and diffing rounds are all absent. Merging in particular is +not a PDS write at all — it is a knot XRPC (`sh.tangled.repo.merge`, +preceded by `mergeCheck`) authorized by a service-auth token, which is a +different shape from everything `pr` does today. The OAuth scopes for all of +these are already granted, so adding them will not force a re-login. See +[TODO.md](../TODO.md) for the current state of each. + +## In the code + +- [`crate::models::Pull`], [`crate::models::Round`] and + [`crate::models::Target`] — the record shapes, with notes on what is + omitted and why deserialization stays permissive. +- [`crate::pr::create`] and [`crate::pr::resubmit`] — the two writes. +- [`crate::git::format_patch`] — the patch, before gzip. +- [`crate::resolve::repo_ref`] — git remote to repo DID. diff --git a/docs/sessions.md b/docs/sessions.md new file mode 100644 index 0000000..328f317 --- /dev/null +++ b/docs/sessions.md @@ -0,0 +1,203 @@ +# Sessions and OAuth + +atgc authenticates with ATProto OAuth against your own PDS. There are no app +passwords, and there is no atgc account — the only credential involved is a +grant your PDS issued to a client running on your machine. + +## Logging in + +`atgc auth login ` binds an ephemeral loopback port, resolves the +handle to a PDS, and opens that PDS's authorization page in a browser. The +redirect comes back to `http://127.0.0.1:/oauth/callback`, which atgc +is listening on itself rather than delegating to jacquard's own listener — +so the browser lands on a real page showing your handle, display name and +avatar, instead of a connection error after the listener has gone away. A +failed login gets a page too. The whole thing times out after five minutes. + +The port is chosen by the operating system, not fixed. Any documentation +that names a specific port is out of date. + +Login is additive: it adds an account and leaves the others alone. It also +prunes *that DID's* superseded sessions, so repeatedly logging in as the same +account collapses to a single grant rather than leaving a dead one behind +every time. Another account's live session is never touched. + +## Scopes + +atgc asks for granular scopes, not account-wide access. The list is +deliberately wider than the commands that exist: it covers repo, pull, issue, +comment, label, artifact and string record writes, blob uploads, and the knot +and CI RPC methods on the roadmap. Requesting them once means shipping +`pr merge` or `issue create` later will not force everyone through a second +consent page. + +Two of those are worth naming precisely, because both have been described +too narrowly in this project's own documentation before. The record scopes +are per-collection and there are eleven of them, not one. And the blob scope +is `blob:*/*` — **any** MIME type, not just the `application/gzip` that +patch blobs use. Narrowing it would be a real improvement and has not been +attempted. + +Scopes that would let atgc destroy things are deliberately absent until the +commands that need them exist: `repo.delete`, `deleteBranch`, secrets, +collaborator and membership changes, `publicKey` writes, and social records. + +The exact list is the `SCOPES` constant in [`crate::auth`], which is the +only place worth reading it from. `atgc auth status` prints the scopes the +active session was actually granted, which is the more useful question. + +## Expired is not logged out + +This is the distinction the whole session model is built around. + +An access token is short-lived. The refresh token behind it usually outlives +it by a lot, and jacquard exchanges one for the other automatically when a +request needs it. So a token past its expiry is in the state "needs a +refresh", not the state "you are logged out". atgc models three states, in +[`crate::auth::SessionState`]: + +- **Missing** — atgc knows the account but holds no session for it at all. +- **Live** — a token, with the time left on it. +- **Expired** — a token past its expiry, which refreshes on next use. + +None of these removes the account. The account registry is a separate file +from the session store precisely so that a lapsed token cannot make an +account vanish — see [Accounts](accounts.md). + +Identity survives a lapsed token in a stronger sense too: almost nothing +atgc reads needs a token at all. DID documents are public, the appview is +public, and `com.atproto.repo.getRecord` on your own PDS is public. So with +every token in the store expired, `atgc auth status`, `pr list`, +`pr status`, `repo list` and `browse` all still work, and still work +correctly per-account. What stops working is exactly the set of things that +write. + +`atgc auth refresh` exchanges the active account's token, and does nothing +when the token is still healthy — that is jacquard's meaning of refresh, not +a bug. A run that reports the same expiry as before did its job. It never +opens a browser and never adds an account; if the grant itself is gone, that +is a login, not a refresh. + +*Unverified:* the refresh path has been exercised against a live session, +but only down its no-op branch. The actual token exchange, and the error +message that fires when a grant has genuinely lapsed, have not been observed. + +## The two-week cap, and what would lift it + +atgc is currently a **public** OAuth client, of the localhost variety: its +client metadata is generated on the fly and there is no key it can prove +possession of beyond DPoP. The ATProto OAuth profile caps a public client's +sessions at two weeks, after which logging in re-opens the browser. Two +weeks is not something atgc chooses or can configure. + +The upgrade is a **confidential** client: client metadata and a JWKS hosted +at a stable URL, with the private key held locally in jacquard's +`ClientData.keyset`. That is a different kind of client with a different +session lifetime, and it is the only way to fix a second, smaller annoyance +— a localhost client cannot set a `client_name`, so your PDS's consent page +describes atgc as "an application on your device" rather than naming it. + +*Unverified:* the project's TODO targets 180-day sessions from that change. +That figure comes from the ATProto OAuth profile's allowance for +confidential clients and has not been measured here, because the change has +not been made. + +The vendored `jacquard-oauth` patch is entangled with all of this: without +it, the `rpc:?aud=*` scopes serialize in a form Bluesky PDSes treat as +matching no audience, and service auth to a knot fails. See +[Vendored dependencies](vendored-dependencies.md). + +## The session store + +Tokens live in `~/.config/atgc/sessions.json`, mode 0600, in jacquard's +format. atgc reads it and prunes it but does not own its schema. + +Keys look like `oauth:/`. A DID can in principle hold +several entries, so when atgc needs "the session for this DID" it takes the +one with the furthest-out expiry. Every resume names its DID explicitly; +there is deliberately no "just give me an agent" entry point, because the +one that existed resumed an arbitrary stored session, which is harmless with +one account and a coin flip with two. + +`atgc auth token` prints the active account's access token, refreshing first +so a stale one is never handed out. It exists for hand-driving XRPC calls +atgc has no command for. One caveat before piping it into `curl`: these +tokens are **DPoP-bound**, so a bare `Authorization: Bearer` header will be +rejected by the PDS. It is still the fastest way to inspect a session or +feed a tool that can do DPoP itself. + +`atgc auth logout` deletes tokens locally. It does not revoke them — they +expire on their own. + +## The OAuth event log + +Nothing above explains how you find out what actually happened to a session, +and that turned out to matter: twice in one day a live session was destroyed +without warning, `sessions.json` found emptied to `{}`, with no record of +what had touched it. So every OAuth operation atgc performs is now appended +to `~/.config/atgc/oauth.jsonl`, mode 0600, one JSON object per line. It sits +beside the two files it is about rather than in `~`. [`crate::oauthlog`] is +the module, and its own documentation carries the full argument. + +It records the store being read and rewritten, sessions fetched, upserted and +deleted, auth state saved and dropped, prunes, the authorize/callback +handshake, every token request with the `client_id` **actually sent** to the +token endpoint, every grant, every refusal with the OAuth error code and a +scrubbed body, transport failures, logouts and which account was selected. +The events are a typed enum, [`crate::oauthlog::Event`], not free-form +strings, so a reader can match on `event` and know what fields exist. + +Each line is `{"ts", "inv", "seq", "event", …payload}`. `inv` is a +per-invocation id — millis, pid and a random field — and `seq` counts events +within one invocation, which is what makes two atgc processes interleaving +visible after the fact rather than merely suspected. + +**No secret is ever written to it.** Not access or refresh tokens, DPoP keys, +PKCE verifiers, authorization codes or client secrets. Where identifying a +particular token matters — proving a rotation happened, or that two processes +spent the same refresh token — the value goes through [`crate::oauthlog::Fp`], +which keeps only the first 8 hex characters of its SHA-256: enough to match +two sightings of one token against each other, useless for recovering it. +Every field that could carry a secret is *typed* `Fp`, so writing a raw one +does not compile, and response bodies are scrubbed by key name before they go +in. + +Lines are capped at 4 KiB and written with a single `O_APPEND` `write(2)`, +which on a local filesystem lands atomically — several atgc processes really +do run at once here, and a torn line would fail at exactly the moment it was +needed. A record too long to fit is replaced by an `oversize` line naming +what was dropped, so a gap in the log is always explained by the log. + +It rotates once at process start, when the file passes 8 MiB: +`oauth.jsonl` becomes `oauth.jsonl.1`, replacing the previous one. So the log +is bounded at roughly 16 MiB and no invocation's events are ever split across +two files. `ATGC_OAUTH_LOG` points it somewhere else, or `0` switches it off. + +There is no command to read it yet — `jq` is the intended tool in the +meantime, which is why the shape is flat: + +```text +jq -c 'select(.event == "token_refused")' ~/.config/atgc/oauth.jsonl | tail -1 +``` + +*Unverified:* the log was built to tell two hypotheses about the destroyed +sessions apart — a `client_id` that differs between login and refresh, and a +race over single-use refresh tokens between overlapping processes — not from +having told them apart. Neither is confirmed, and the module documentation is +careful to say so. + +## In the code + +- [`crate::auth`] — the OAuth client, the session store, and the module + documentation for the same material from the implementation side. +- [`crate::auth::agent_for_did`] — resume a named account's session, + refreshing if needed, never opening a browser. +- [`crate::auth::SessionState`] — the three states and how they are + described to the user. +- [`crate::auth::sessions_by_did`] — reading the store. +- [`crate::oauthlog`] — the event log: why it exists, the secret rule, the + single-`write(2)` argument and the rotation. +- [`crate::oauthlog::LoggedAuthStore`] and + [`crate::oauthlog::LoggedHttpClient`] — the two wrappers that see every + session write and every token-endpoint request without the rest of the + code knowing they are there. diff --git a/docs/vendored-dependencies.md b/docs/vendored-dependencies.md new file mode 100644 index 0000000..95f82da --- /dev/null +++ b/docs/vendored-dependencies.md @@ -0,0 +1,53 @@ +# Vendored dependencies + +There is one, and it should not be permanent. + +## `vendor/jacquard-oauth` + +A copy of `jacquard-oauth` 0.12.1 with a one-line change, wired in through +`[patch.crates-io]` in `Cargo.toml`. + +Upstream serializes a scope of the form `rpc:?aud=*` as a bare +`rpc:`, dropping the audience parameter. Bluesky PDSes treat the bare +form as matching *no* audience rather than any, so requesting service auth +for a knot fails with `ScopeMissingError`. Since every knot call atgc makes +— `sh.tangled.repo.create` today, merge and fork later — is authorized by a +service-auth token minted by the PDS, that failure takes out +`atgc repo create` entirely. + +The patch keeps the `aud` parameter. It is marked with an "atgc patch" +comment in `vendor/jacquard-oauth/src/scopes.rs`, so the diff against +upstream is findable without a checkout of upstream. + +## What has to happen for it to leave + +Report it upstream, and drop the `[patch.crates-io]` stanza when a release +carries the fix. That has not been done. The honest reason recorded at the +time was laziness rather than any difficulty: the fix is a one-liner and +whoever gets to it first should just send it. + +## Rules while it is here + +**It keeps its own license.** The vendored code stays under upstream's +MPL-2.0, not atgc's `MIT OR Apache-2.0`. Nothing about copying it into this +tree changes that. + +**The formatters leave it alone.** `prek.toml` excludes `vendor/` from every +hook that edits files, so the copy stays close to verbatim and the diff +against upstream stays readable. The read-only checks are deliberately *not* +excluded — a merge marker or a broken `Cargo.toml` in there is still worth +hearing about. + +The lint configuration in `Cargo.toml`'s `[lints]` tables does not reach it +either. It is a `[patch.crates-io]` dependency, not a workspace member, so +`cargo fmt --all` never sees it, and while clippy does compile it, +`-D warnings` applies only to the primary package. Both of those were +checked by injecting violations into the vendored crate and confirming +nothing failed. + +## In the code + +The authoritative version of this is the comment above `[patch.crates-io]` +in `Cargo.toml`, which sits where someone changing the dependency will read +it. The scope list it exists to serve is `SCOPES` in [`crate::auth`], and +the knot call it unblocks is in [`crate::repo::create`]. diff --git a/docs/writing-documentation.md b/docs/writing-documentation.md new file mode 100644 index 0000000..a0231d0 --- /dev/null +++ b/docs/writing-documentation.md @@ -0,0 +1,156 @@ +# Writing documentation + +How this folder is built, why it is built that way, and the one rule about +examples that exists because of a property of the crate rather than a matter +of taste. + +## One build + +```text +cargo docs +``` + +An alias in `.cargo/config.toml` for +`cargo doc --no-deps --document-private-items`. It renders both halves of +the documentation into one static tree at `target/doc/atgc/index.html`: + +- **Reference**, generated from the code — every module, type and function + in `src/`, including the private ones, which in a binary crate is nearly + all of them. +- **Narrative**, the CommonMark files in `docs/`, pulled in through + `#![doc = include_str!(…)]` by `src/docs.rs` and rendered as module pages + under `atgc::docs`. + +`.cargo/config.toml` also sets `rustdocflags = ["-D", "warnings"]`, so the +build fails on a broken intra-doc link rather than emitting a dead one. +Nothing is required beyond `rust-toolchain.toml`'s pinned toolchain, and the +build needs no network the crate's dependencies did not already need. + +## Why rustdoc rather than mdBook + +mdBook is the idiomatic Rust answer for long-form prose and is better at it: +real chapter ordering, full-text search over the prose, a navigation model +built for a book rather than for an API. It was not chosen, for three +reasons in descending order of weight. + +**Cross-references from prose into code are checked.** When a page here +writes [`crate::account::select`], that is an intra-doc link, resolved by +rustdoc at build time against the actual item. Rename the function and the +docs build goes red. Under mdBook the same reference is a hand-written URL +into separately generated rustdoc output, checked by nothing, and stale +cross-references are the main way a docs folder decays. Several statements +in this project's README had already gone stale by the time these pages were +written; a mechanism that makes a class of that impossible is worth more +here than better navigation. + +**No tool outside the pin.** `rust-toolchain.toml` pins 1.97.1 and ships +rustdoc with it. mdBook is a separate binary that every contributor, and any +CI runner, would have to install and keep in step — on a repo whose CI +workflow has never executed once. For a single-user codebase that says +outright it makes no stability guarantees, a docs pipeline with its own +install step is a cost with no matching benefit. + +**One output.** One command, one directory, one thing to publish, no +question about which of two sites a given fact belongs on. + +What that costs, stated plainly: rustdoc's sidebar is an API sidebar, so the +six pages here appear as an alphabetical list of modules with no sense of +order. The reading order in [index.md](index.md) is the substitute, and it +is worse than a book's table of contents. rustdoc's search indexes items, +not prose, so searching for a phrase from these pages will not find it. + +Neither is expensive to reverse. The files under `docs/` are plain +CommonMark with relative links between them — which is exactly what mdBook +expects — so if the prose ever outgrows this, the content moves across +unchanged and only the cross-references into code have to be rewritten. + +## Two audiences, one source + +The files in `docs/` are read in two places and they are not equally good in +both. + +Read as files — in a repo browser, in an editor, in `git show` — they are +complete and self-contained, and the relative links between pages work. + +Read as rustdoc, the prose is rendered and the links into code work, but the +relative `*.md` links between pages point at the source files and do not +resolve; navigate with the sidebar instead. This is a real wart. It is +accepted rather than fixed because the alternative — writing every +prose-to-prose link as an intra-doc path — breaks the files in the place +most readers actually land, and because the relative form is what keeps the +mdBook exit open. + +## Examples are not Rust + +**Do not write a ```` ```rust ```` block in `docs/`.** A `prek` hook rejects +it. + +The reason is a property of the crate. `atgc` is a binary-only target, and +**rustdoc does not run doctests for binary targets**. A Rust example in +these pages would compile in nobody's build, be checked by nothing, and rot +silently while looking exactly like the tested examples readers are used to +seeing in Rust documentation. That is worse than no example: it is an +example that lies about its own status. + +There are two ways out and this project takes the second. + +The first is to restructure the crate — a `lib.rs` holding the real code +with `main.rs` reduced to a thin shim — after which doctests run. That is a +genuine improvement for a crate with an API worth exercising, and it is the +right move for one. It is not right here, and certainly not in a +documentation change: `atgc` has no library API and no intention of growing +one. Its interface is a command line. Making every module `pub` so that +examples of an API nobody can call could be compiled would be a structural +change to the crate in service of documentation nobody needs, and it would +land a public surface that then has to be maintained. + +The second is to notice that the examples this tool actually wants are shell +transcripts, JSON record shapes and git invocations — none of which rustdoc +would ever have run, and none of which pretend otherwise. That is what these +pages contain. + +If a Rust snippet is genuinely the clearest way to say something, tag it +`rust,ignore`. rustdoc renders `ignore` blocks with a marker saying the +example is not tested, which is the honest presentation, and the hook allows +it. + +The corollary is that **every** code fence in `docs/` needs a language tag, +`text` included. rustdoc reads an untagged fence as Rust, so an untagged +shell transcript is a doctest that happens not to compile — usually a build +failure, and on the day its contents accidentally parse, a silent one. The +hook checks this too. + +Nothing about this applies to `src/`. Doc comments there are reference +documentation for a binary and are subject to the same non-execution, which +is why they describe behaviour rather than demonstrate calls. + +## The checks + +Two `prek` hooks, both also runnable by hand: + +- **`cargo doc`** — the doc build, with `-D warnings`. Catches broken + intra-doc links, and catches a `docs/` page that no longer builds. +- **`docs`** — `scripts/doc-lint.sh`. Catches untagged Rust fences, and + relative links between `docs/` pages whose target file does not exist. + +`.tangled/workflows/ci.yml` has a `docs` step doing the same thing. +**Unverified**, like every other step in that file: no spindle is attached +to this repo and the workflow has never run. + +## House rules for the prose + +Match what is already here. This codebase explains *why* at length and is +unusually willing to say what it has not proven; both are features. + +- Say what is unverified, in the text, where the claim is. The word + *unverified* appears in these pages several times and should keep + appearing. +- Verify against the code, not against other prose. The material these pages + were built from had accumulated errors — a fixed callback port that had + been ephemeral for a long time, a command that no longer existed, a scope + list that had grown by a factor of ten — all of it faithfully copied + forward because nobody re-read the source. +- Prefer a link into the code over a paraphrase of it. The link is checked; + the paraphrase is not. +- `--help` owns flags and defaults. Repeating them here creates a second + place to be wrong. diff --git a/prek.toml b/prek.toml index e2a955d..bf8b1ec 100644 --- a/prek.toml +++ b/prek.toml @@ -33,4 +33,16 @@ hooks = [ { id = "cargo-fmt", name = "cargo fmt", entry = "cargo fmt --all -- --check", language = "system", types = ["file"], files = '\.rs$', pass_filenames = false }, # clippy type-checks as it lints, so there is no separate cargo check hook. { id = "cargo-clippy", name = "cargo clippy", entry = "cargo clippy --all-targets --all-features -- -D warnings", language = "system", types = ["file"], files = '(\.rs$|^Cargo\.(toml|lock)$)', pass_filenames = false }, + # The doc build is a check, not just a build: .cargo/config.toml sets + # rustdocflags = -D warnings, so a prose page that links to an item that no + # longer exists fails here. That is the whole reason the narrative docs are + # compiled into rustdoc rather than kept as a separate site — see + # docs/writing-documentation.md. Runs after clippy, and mostly off the same + # warm target dir. Triggered by docs/ as well as src/, since the pages are + # include_str!'d into the crate and a change to one is a change to the doc + # build's input. + { id = "cargo-doc", name = "cargo doc", entry = "cargo docs", language = "system", types = ["file"], files = '(\.rs$|^docs/.*\.md$|^Cargo\.(toml|lock)$|^\.cargo/config\.toml$)', pass_filenames = false }, + # The two things the doc build cannot see: an untested rust code fence, and + # a relative link between pages whose target no longer exists. + { id = "doc-lint", name = "doc lint", entry = "scripts/doc-lint.sh", language = "system", types = ["file"], files = '^(docs/.*\.md|scripts/doc-lint\.sh)$', pass_filenames = false }, ] diff --git a/scripts/doc-lint.sh b/scripts/doc-lint.sh new file mode 100755 index 0000000..e8c4409 --- /dev/null +++ b/scripts/doc-lint.sh @@ -0,0 +1,65 @@ +#!/bin/sh +# Two checks over docs/ that the doc build itself cannot make. +# +# `cargo docs` (with rustdocflags = -D warnings, set in .cargo/config.toml) +# already catches broken intra-doc links from prose into code, which is the +# main way these pages rot. It cannot catch either of the following, because +# to rustdoc both are just text. +# +# Run by prek; also runnable by hand from anywhere in the checkout. + +set -eu +cd "$(dirname "$0")/.." +status=0 + +# 1. Untested Rust code fences. +# +# atgc is a binary-only crate, and rustdoc does not run doctests for binary +# targets. A ```rust block in docs/ would therefore be checked by nothing +# while looking exactly like the tested examples readers expect in Rust +# documentation. A `rust,ignore` fence is allowed: rustdoc renders those with +# a marker saying the example is not tested, which is the honest +# presentation. See docs/writing-documentation.md. +fences=$(grep -rnE '^ *```(rust|rs)([,{ ].*)?$' docs | grep -v ignore || true) +if [ -n "$fences" ]; then + echo "doc-lint: untested rust fence in docs/ — tag it rust,ignore or use a shell transcript" + echo "$fences" + status=1 +fi + +# 1b. Fences with no language at all, which rustdoc treats as Rust and so +# would fail the doc build the moment their contents happened to parse. +# Every opening fence gets a tag: text, json, and so on. Counting fences per +# file makes the odd-numbered ones the opening ones. +bare=$(awk '/^ *```/ { n[FILENAME]++ } + /^ *```$/ && n[FILENAME] % 2 == 1 { print FILENAME ":" FNR ": untagged fence" }' \ + docs/*.md) +if [ -n "$bare" ]; then + echo "doc-lint: code fence with no language tag (rustdoc reads those as Rust)" + echo "$bare" + status=1 +fi + +# 2. Relative links between pages whose target file does not exist. +# +# These are ordinary Markdown links, so a rename leaves a dead one behind +# with nothing to notice. Covers inline [text](page.md) and reference-style +# [label]: page.md, in both cases only for relative paths ending in .md. +links=$(grep -rnoE '\]\([^):]+\.md(#[^)]*)?\)|^\[[^]]+\]: [^ :]+\.md' docs || true) +missing=$(printf '%s\n' "$links" | while IFS= read -r hit; do + [ -n "$hit" ] || continue + file=${hit%%:*} + target=${hit##*]} + target=${target#\(} + target=${target#: } + target=${target%)} + target=${target%%#*} + [ -f "$(dirname "$file")/$target" ] || + echo "doc-lint: $file links to a missing file: $target" +done) +if [ -n "$missing" ]; then + echo "$missing" + status=1 +fi + +exit $status diff --git a/src/auth.rs b/src/auth.rs index 075e1df..e1d1aa3 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -7,8 +7,13 @@ //! also replaces the PDS consent page's generic "an application on your //! device" label with a real client name; localhost clients can't set one. //! -//! Scopes are granular: only sh.tangled.repo.pull writes and gzip blob -//! uploads, not account-wide access. +//! Scopes are granular rather than account-wide, but "granular" has drifted +//! since this line first said `sh.tangled.repo.pull` and gzip blobs. `SCOPES` +//! below now covers eleven `sh.tangled.*` collections — the non-destructive +//! roadmap, requested up front so shipping a command is not a re-login — and +//! `blob:*/*`, which is every MIME type, not only the `application/gzip` a +//! patch blob uses. Read the constant, not this paragraph. See +//! [`crate::docs::sessions`]. //! //! Several accounts can be logged in at once. Sessions are keyed by DID in //! jacquard's store, and every resume names its DID explicitly rather than diff --git a/src/docs.rs b/src/docs.rs new file mode 100644 index 0000000..5cf27d8 --- /dev/null +++ b/src/docs.rs @@ -0,0 +1,39 @@ +#![doc = include_str!("../docs/index.md")] +//! +//! --- +//! +//! This module exists only to carry documentation. It declares no items and +//! compiles to nothing; each submodule below is an empty module whose entire +//! purpose is to give one of the Markdown files under `docs/` a page in the +//! rustdoc output, so that narrative prose and generated reference +//! documentation are one build rather than two. See +//! [Writing documentation](crate::docs::writing_documentation) for the +//! argument, including why `docs/` is CommonMark on disk rather than +//! rustdoc-only prose. +//! +//! The submodules are listed alphabetically by rustdoc. The intended reading +//! order is above. + +pub mod architecture { + #![doc = include_str!("../docs/architecture.md")] +} + +pub mod accounts { + #![doc = include_str!("../docs/accounts.md")] +} + +pub mod pull_requests { + #![doc = include_str!("../docs/pull-requests.md")] +} + +pub mod sessions { + #![doc = include_str!("../docs/sessions.md")] +} + +pub mod vendored_dependencies { + #![doc = include_str!("../docs/vendored-dependencies.md")] +} + +pub mod writing_documentation { + #![doc = include_str!("../docs/writing-documentation.md")] +} diff --git a/src/main.rs b/src/main.rs index ecc53e4..6caa6a8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -3,6 +3,10 @@ mod account; mod auth; mod browse; mod debug; +// Documentation only: empty modules carrying the narrative pages under +// docs/, so `cargo docs` renders prose and generated reference as one thing. +// See docs/writing-documentation.md. +mod docs; mod git; mod models; mod oauthlog; diff --git a/src/models.rs b/src/models.rs index 2c1510e..befb791 100644 --- a/src/models.rs +++ b/src/models.rs @@ -1,6 +1,6 @@ //! Records from the sh.tangled.* lexicons. //! -//! Source of truth: https://tangled.org/tangled.org/core/tree/master/lexicons +//! Source of truth: //! The lexicons are still evolving; keep deserialization permissive. #![allow(dead_code)]