Skip to content

Code-quality cleanup grab-bag (orphaned fetch.rs, no-op flags, temp-dir leak, docs, ...) #58

Description

@lmorchard

Found during a code-review pass (2026-07-23). Grab-bag of smaller maintainability items surfaced by the review; each is small and independent.

  • Orphaned src/cli/fetch.rs — 122-line ActivityPub-outbox fetcher not declared as a module anywhere (src/cli.rs has no mod fetch;), so it never compiles. Contains its own dead logic (count/commented-out break). It appears to be WIP for Use ActivityPub outbox resource for incremental backup fetches #12 — either wire it in and finish it, or remove it.
  • Dead import_many in src/db/activities.rs — no callers; carries the transaction-leak bug from the DB-transactions issue.
  • No-op global flags--verbose / --quiet / --debug are declared on the top-level Cli (src/cli.rs) and shown in --help, but nothing reads them; log level comes only from config/env in app.rs. Wire them into init_logging() or remove them.
  • Temp media dir leaksrc/mastodon/importer.rs extracts media under tmp-<random> before actor.json is seen; the dir is only relocated/removed inside handle_actor. If there's no actor.json or any earlier error, the extracted media is left behind under media_path/tmp-<random>/…. Use RAII cleanup (e.g. tempfile::TempDir) and handle the "no actor.json" case.
  • Inconsistent error typesBox<dyn Error> vs anyhow::Result mixed across db/mastodon/cli modules; standardize on anyhow::Result.
  • Lossy attachment mime mappingsrc/activitystreams.rs coerces media types to guessed strings (audio/mp3 instead of audio/mpeg, Unknown → "unknown"); the template only special-cases video/, so audio/unknown render as broken <img>. Preserve the real mime type and add an audio/ template branch.
  • Disabled template tests — the only templates.rs test module is fully commented out (TODO: get this test passing on windows). Restore it, gating path-sensitive asserts behind cfg(not(windows)) rather than disabling everything.
  • Missing CLI docsdocs/src/cli/ lacks pages for fetch, mastodon (link/verify/code), and upgrade; add them and link from SUMMARY.md.
  • cargo-audit — no dependency-vuln check in CI; add a non-blocking cargo audit job (pairs with the dependency-consolidation issue).

Severity: low individually; collectively worth chipping away.


Appended from the code-review wrap-up (2026-07-23) — minor items:

  • CI uses deprecated Node 20 actionsactions/checkout@v4 is being force-run on Node 24 (deprecation warning in CI logs). Bump to an action version targeting Node 24, or pin as GitHub guidance evolves.
  • INSERT OR REPLACE resets created_atdb/activities.rs (import, import_activity) and db/actors.rs (import_actor) delete-and-reinsert on conflict, so the created_at DEFAULT CURRENT_TIMESTAMP column is rewritten on every re-import. No impact today (created_at is never queried), but if first-seen semantics are ever wanted, use ON CONFLICT(id) DO UPDATE SET json=excluded.json.
  • Migrations are up-only (no down) — a deliberate decision, not necessarily a defect. Decide whether reversibility matters for this app; if not, document that migrations are forward-only (relates to the validation-test work in Add migration validation test; adopt single-quote SQL string literals #56).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    code-qualityMaintainability, refactoring, tech debt

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions