● Increment 6 complete. All six increments now landed. #42

Merged

navicore merged 2 commits from increment-6 into main

2026-05-30 02:16:48 +00:00

navicore commented

2026-05-30 02:06:07 +00:00

Owner

What changed:

File: docs/ARCHITECTURE.md
Change: Updated Actors (Operator now spans local + remote); added
admin_api/, cli/backend/, server/admin/ to the Modules table; added
clap_complete / reqwest (rustls) / webbrowser to Solution Strategy;
expanded Error handling to mention 403 + 409 and the UNIQUE-constraint →
Conflict routing; added a new Admin API crosscutting section covering
off-by-default listener, reserved admin realm + anz-admin group,
live-state authz, denial audit, short admin TTLs, secret-once contract
────────────────────────────────────────
File: docs/ROADMAP.md
Change: Added remote admin to Current State with link to the retired design

doc; bumped test count to "150+"

────────────────────────────────────────
File: docs/design/remote-admin-api.md →
docs/design/done/remote-admin-api.md
Change: Retired via plain mv. Matches the convention with the other six
done/ docs.

The doc tree now matches reality: a fresh contributor can read
docs/ARCHITECTURE.md and understand the remote-admin posture without
consulting the design doc, and the link from ROADMAP to
done/remote-admin-api.md preserves the design rationale for future
reference until you choose to delete it.

The whole feature is shipped — bootstrap on the box, log in via browser
from a desktop, administer over HTTPS with tab completion + profile-driven
routing, audit log attributes every change to the operator. Same merge
recommendation: own PR. Nothing left on the increment list.

What changed: File: docs/ARCHITECTURE.md Change: Updated Actors (Operator now spans local + remote); added admin_api/, cli/backend/, server/admin/ to the Modules table; added clap_complete / reqwest (rustls) / webbrowser to Solution Strategy; expanded Error handling to mention 403 + 409 and the UNIQUE-constraint → Conflict routing; added a new Admin API crosscutting section covering off-by-default listener, reserved admin realm + anz-admin group, live-state authz, denial audit, short admin TTLs, secret-once contract ──────────────────────────────────────── File: docs/ROADMAP.md Change: Added remote admin to Current State with link to the retired design doc; bumped test count to "150+" ──────────────────────────────────────── File: docs/design/remote-admin-api.md → docs/design/done/remote-admin-api.md Change: Retired via plain mv. Matches the convention with the other six done/ docs. The doc tree now matches reality: a fresh contributor can read docs/ARCHITECTURE.md and understand the remote-admin posture without consulting the design doc, and the link from ROADMAP to done/remote-admin-api.md preserves the design rationale for future reference until you choose to delete it. The whole feature is shipped — bootstrap on the box, log in via browser from a desktop, administer over HTTPS with tab completion + profile-driven routing, audit log attributes every change to the operator. Same merge recommendation: own PR. Nothing left on the increment list.

navicore added 1 commit

2026-05-30 02:06:07 +00:00

● Increment 6 complete. All six increments now landed.

CI / ci (pull_request) Successful in 4m6s

Details

9904b55bb6

What changed:

  File: docs/ARCHITECTURE.md
  Change: Updated Actors (Operator now spans local + remote); added
    admin_api/, cli/backend/, server/admin/ to the Modules table; added
    clap_complete / reqwest (rustls) / webbrowser to Solution Strategy;
    expanded Error handling to mention 403 + 409 and the UNIQUE-constraint →
    Conflict routing; added a new Admin API crosscutting section covering
    off-by-default listener, reserved admin realm + anz-admin group,
    live-state authz, denial audit, short admin TTLs, secret-once contract
  ────────────────────────────────────────
  File: docs/ROADMAP.md
  Change: Added remote admin to Current State with link to the retired design

    doc; bumped test count to "150+"
  ────────────────────────────────────────
  File: docs/design/remote-admin-api.md →
  docs/design/done/remote-admin-api.md
  Change: Retired via plain mv. Matches the convention with the other six
    done/ docs.

  The doc tree now matches reality: a fresh contributor can read
  docs/ARCHITECTURE.md and understand the remote-admin posture without
  consulting the design doc, and the link from ROADMAP to
  done/remote-admin-api.md preserves the design rationale for future
  reference until you choose to delete it.

  The whole feature is shipped — bootstrap on the box, log in via browser
  from a desktop, administer over HTTPS with tab completion + profile-driven
  routing, audit log attributes every change to the operator. Same merge
  recommendation: own PR. Nothing left on the increment list.

navicore commented

2026-05-30 02:09:19 +00:00

Author

Owner

Review: Increment 6 — documentation (feature complete)

Verdict: accurate, ready to merge. Docs-only (+17 -6): ARCHITECTURE.md + ROADMAP.md updates and the mv of remote-admin-api.md into design/done/, matching the convention already used for the other retired designs. The value of reviewing a docs PR is checking the prose against the code — so I verified every falsifiable claim, with attention to the two that describe behavior that didn't exist when I reviewed Increment 3.

Claims verified against the code

"UNIQUE violations → 409 Conflict (not 500)" — real. error.rs has a Conflict variant, and both From<rusqlite::Error> and From<anyhow::Error> route SQLITE_CONSTRAINT_UNIQUE to 409. The anyhow impl correctly downcast_refs through the wrapper (the db layer returns anyhow::Result, so the raw rusqlite error is buried) — that subtlety is exactly right, and create_realm_duplicate_returns_409 covers it. This was my #39 finding; it's genuinely fixed, not just documented.
"The admin realm cannot be deleted over the API" — real. delete_realm guards realm == config.admin_realm and returns 403 with a "use the local CLI on the box" message, tested by delete_admin_realm_is_refused. This was my #39 self-lockout note; also genuinely implemented.
AppError status set "400, 401, 403, 404, 409, 429, 500" — all seven variants present, list matches exactly.
"no openssl in the dependency graph" — confirmed: no openssl* crates in Cargo.lock, rustls present.
"150+ tests" — actual count is 165; appropriately conservative.
Spot-checked the rest against prior increments — denial-reason list (no_bearer/invalid_token/foreign_realm/disabled/no_admin_group/unknown_subject), loopback redirect 127.0.0.1:8765/callback, live-state authz (issuer+kid checked twice), refresh-once-on-401, secret-once contract, local MFA-QR rendering, 10 db tables — all match.

Minor (optional, non-blocking)

The self-lockout guard the docs describe is scoped to realm deletion (the catastrophic cascade). The two lesser vectors I'd noted alongside it in #39 — an admin deleting/disabling their own admin user, or stripping anz-admin from themselves — remain possible (both operator-recoverable via another admin or break-glass). The docs don't claim those are guarded, so this isn't an inaccuracy; just flagging that "self-lockout" protection is partial-by-design if you want the doc to say so explicitly.

Nicely closes the loop on the series — the two behavioral gaps from the Increment 3 review were implemented and are now described faithfully. No fiction in these docs.

## Review: Increment 6 — documentation (feature complete) **Verdict: accurate, ready to merge.** Docs-only (+17 -6): `ARCHITECTURE.md` + `ROADMAP.md` updates and the `mv` of `remote-admin-api.md` into `design/done/`, matching the convention already used for the other retired designs. The value of reviewing a docs PR is checking the prose against the code — so I verified every falsifiable claim, with attention to the two that describe behavior that *didn't exist* when I reviewed Increment 3. ### Claims verified against the code - **"UNIQUE violations → 409 Conflict (not 500)" — real.** `error.rs` has a `Conflict` variant, and both `From<rusqlite::Error>` *and* `From<anyhow::Error>` route `SQLITE_CONSTRAINT_UNIQUE` to 409. The `anyhow` impl correctly `downcast_ref`s through the wrapper (the db layer returns `anyhow::Result`, so the raw rusqlite error is buried) — that subtlety is exactly right, and `create_realm_duplicate_returns_409` covers it. This was my #39 finding; it's genuinely fixed, not just documented. - **"The admin realm cannot be deleted over the API" — real.** `delete_realm` guards `realm == config.admin_realm` and returns 403 with a "use the local CLI on the box" message, tested by `delete_admin_realm_is_refused`. This was my #39 self-lockout note; also genuinely implemented. - **AppError status set "400, 401, 403, 404, 409, 429, 500"** — all seven variants present, list matches exactly. - **"no openssl in the dependency graph"** — confirmed: no `openssl*` crates in `Cargo.lock`, `rustls` present. - **"150+ tests"** — actual count is 165; appropriately conservative. - Spot-checked the rest against prior increments — denial-reason list (`no_bearer`/`invalid_token`/`foreign_realm`/`disabled`/`no_admin_group`/`unknown_subject`), loopback redirect `127.0.0.1:8765/callback`, live-state authz (issuer+kid checked twice), refresh-once-on-401, secret-once contract, local MFA-QR rendering, 10 db tables — all match. ### Minor (optional, non-blocking) - The self-lockout guard the docs describe is scoped to **realm deletion** (the catastrophic cascade). The two lesser vectors I'd noted alongside it in #39 — an admin deleting/disabling their *own* admin user, or stripping `anz-admin` from themselves — remain possible (both operator-recoverable via another admin or break-glass). The docs don't claim those are guarded, so this isn't an inaccuracy; just flagging that "self-lockout" protection is partial-by-design if you want the doc to say so explicitly. Nicely closes the loop on the series — the two behavioral gaps from the Increment 3 review were implemented and are now described faithfully. No fiction in these docs.

navicore added 1 commit

2026-05-30 02:11:44 +00:00

ready to merge

CI / ci (pull_request) Successful in 4m5s

Details

0df3637c17

navicore merged commit 1927223ec3 into main

2026-05-30 02:16:48 +00:00

navicore deleted branch increment-6

2026-05-30 02:16:48 +00:00

navicore referenced this pull request from a commit

2026-05-30 02:16:49 +00:00