Resuming the repo-wide simplification
Paused 2026-11-01 because every cargo command needed a manual approval.
The ask: /simplify over the entire repo, then “do all suggested simplifications”. Ten read-only reviewers reported about 147 findings, deduplicated to 139 items in docs/plans/repo-simplification.md. Quality only: no behaviour change, no new precondition. Anything that would change behaviour is held back for the user, not applied.
State of the tree
- Branch
devata23f54e, level withorigin/dev. Nothing is committed; all of it is in the working tree (88 tracked files, +2409/-3163, plus the untracked plan and this file). Do not commit or push without being asked. docs/plans/repo-simplification.mdis the checklist. A[x]means applied and building.- Clippy passed on the main crate at the last check. The last full test run predates the web changes listed under “First steps”.
local/openapi-new.jsonis a scratch copy, gitignored; delete it.
Next
Only the plan’s unticked items are left: test helpers, two preview fixtures, and CI and docs tidying. Run cargo clippy --workspace --all-targets -- -D warnings and cargo test --workspace -q --no-fail-fast after each group.
For anything touching templates or CSS, build the commit before these edits in a local/ worktree and compare with scratch/preview_html_diff.sh and scratch/preview_visual_diff.sh. The invite blob on /peers differs on every run.
The cargo setup
.claude/settings.json carries excludedCommands: ["cargo *"], so a plain cargo command runs without a prompt. A redirected one (cargo test > file) falls back into the sandbox: use -q instead. That settings change is in the tree and should not be swept into a simplification commit unnoticed: it may belong in the untracked .claude/settings.local.json.
Known sandbox artefacts:
- Inside the sandbox a wiremock test can fail with “did not receive any request” in a full run and pass alone. A
<sandbox_violations>line naming127.0.0.1:<port>confirms it. Rerun that test alone before believing it. scripts/check.sh terraform zizmorcannot run inside the sandbox; hand that command to the user.
Traps found while applying
- Read a failing
the_committed_document_is_currentbefore regenerating. It caught a real bug: a constant inserted between a route’s doc comment and#[utoipa::path]removed the route’s OpenAPI summary. The document should stay byte-identical throughout this work. - Never insert an item between a doc comment and what it documents; multi-line perl edits did this twice.
Store::fail_after(n)counts queries, so a change to the query count shifts these tests’ budgets. Check that the assertion still means the same thing, then adjust.- A handler returning
Result<_, Response>fails clippy’sresult_large_err; use a small error type (Unavailableinapi.rs) or a boxed response. - Use
git grep -nE, or quote globs: the shell is zsh, and--include=*.rsunquoted fails.\|in plaingit grepmatched nothing. git grepprints a harmless.gitmodules: Permission deniedwarning.
Held back for the user’s decision
Each changes behaviour, so none is applied. None has been answered yet.
- Cache the notification webhook and heartbeat URLs (
notify.rs). Saves about 1,440 Argon2 runs a day, but a URL set through thevaultCLI would need a restart. light-dark()palette (style.css:104-176). Removes about 60 lines, drops browsers older than May 2024.- Delete the inert
.wide .wideCSS rule (style.css:895), which its comment defends as documentation. - Dispatch
previewandopenapibefore config load (main.rs:50), which drops the error log on a bad config.
Possible bug, outside this work and unconfirmed: at 32rem or below, style.css:1624 hides .bar nav button svg, including the theme toggle’s system glyph, so the toggle may look empty on a phone.
Decisions already made
- Early returns use the
ok_or_return!macro rather than anEarlynewtype, since an extractor would change which response wins. refuse_publicis a plain function rather than aPrivateCallerextractor, because about six tests call those handlers directly.sharerr-delugekeeps its own connection; only Transmission and rTorrent useRpcConnection.peer_gossip_recordis not converted toquery_scalar: it swallows decode errors and converting it would change behaviour.persist’screate_dir_allin the vault stays; a direct-call test pins it.- Log-only change accepted: rTorrent
addnow also emitsapply_limits’ info line. metrics::renderskips the whole last-run block whenfinished_atisNone, whichgatheralready filters out.
Notes for the open items
Line numbers are from before these edits; grep for the names.
Doctor async: make doctor::checks natively async using secrets::open_vault_async, checks::scan_libraries and spawn_blocking around check_paths_of, then call json_report directly from api.rs. Drop the item and report it if the future is not Send. A panic now returns 500 through the JoinError arm; keep that handling.
Doctor tests: tests at :1243 and :1491 are the same test; :1277 and :1308 are subsumed by :1524. Move helpers to the top, add qbit_check(&config, &vault, fix) as a twin of arr_check, and make qbit_config take a &Url.
with_open_vault: one state::fixtures::with_open_vault(config, seed, body) plus with_no_master_key for the clear_env cases. peers.rs’s export_degrades_gracefully_without_an_openable_vault changes the environment partway, so it stays hand-written.
Identity::from_seed: sharerr-testkit is a dependency of sharerr but not of sharerr-lighthouse; leave lighthouse’s own copy.
test_support::shared_item and peer: sharerr-testkit has no sharerr-core dependency, so these go in the sharerr crate’s test_support.
Testkit mounts: only stubs with no .expect or body matcher fit mount_json, mount_text and mount_ok; check each.
internal_docs!: keep full URL literals. .github/link-check-paths.txt has lychee sweep web/docs.rs, so building URLs with concat! would break that sweep. Also fix the false REPO comment.
import_config: build a PendingConfig and call commit_config(&state, "config", None, pending), then run import_peers if response.status().is_redirection(). Keep the import’s own log line.
Edit helpers: leave the differing i64::try_from(..).unwrap_or(60/900/604_800/MAX) fallbacks in settings alone; unifying them changes behaviour.
ItemFacts: the templates and preview.rs’s five ItemRow literals change to .facts.x.
preview mock route: keep page’s body inside mock; render would turn a template failure into a 500. title_case and arr_section need wider visibility.
Templates: the page-width rules in style.css stay untouched. Widen the lock-key scanner (templates/mod.rs:1398) when adding ui::text_field, or it stops seeing those keys. Keep .topo-icon out of the stroke selector, because CSS stroke would override its attribute. Keep class="invite": peers.rs uses it as a test marker. Check each page with the /ui-check skill.
CI, scripts, docker:
- Only the repo’s own CI files are in scope:
ci,coverage,docker,docker-image,integrationandprepareworkflows,check_tool_versions.local.sh,validate_compose.sh,actions/rust-toolchain. Everything else under.githubis synced from another repo; never edit those copies. - No git hooks, no new step or gate on a release, no bot commits, no cross-repo
uses:. Actions stay SHA-pinned. A tag gate checks only the tag. - Before deleting
.tool-versions, check whether anything local reads it; it pins yq and terraform, and terraform has drifted fromtools.txt. - Hadolint:
foundmeans non-empty output there but non-zero exit for semver; confirm they match before sharingadvisory-check. - YAML anchors are new to these workflows. Run
scripts/check.sh zizmor actionlint workflows;lint_workflows.sh:92reads the first step through yq and must follow the alias. .dockerignore: confirm no crate embedsdocs/; only a test-only read atopenapi.rs:435was found.- Docker runners cannot run here. Say so: only shellcheck and
validate_compose.shverify them.
Finishing
- Run
scripts/check.shand read itsskipped:line. Hand the user anything the sandbox blocks. - Grep the diff outside
docs/plans/for§|\bS[0-9]\b|\bC[0-9]\b|the plan. - Delete
docs/plans/repo-simplification.mdonce every item is applied or recorded, and delete this file. - End with the end-of-task summary: changes, what was verified and not, suggested commit boundaries by group, pending GitHub actions, scripts added to
scratch/(none so far), agents and their tokens, and assumptions.
Agents used: 10 read-only reviewers, all finished. Tokens: core and store 269,820; torrent and client 232,807; arr, probe, lighthouse, testkit 248,036; sync, tracker, torznab 197,410; gossip, notify, state 278,686; commands 228,590; web auth 223,131; web settings 199,400; templates 221,227; CI, scripts, docker 244,361. The ask capped agents at 10, so the rest is done in the main session unless the user says otherwise.
Assumptions so far: the log-only rTorrent change and the metrics::render block above; the Unavailable error type in api.rs.