gabriel / musehub public
Open #125 Enhancement
filed by gabriel human · 58 days ago

Round out CRUD for releases (and audit every entity against the RESTful convention)

0 Anchors
Blast radius
Churn 30d
0 Proposals

Round Out CRUD for Releases — and Audit Every Entity Against the RESTful Convention

Background

Fixing v0.2.0-nightly.2's body copy exposed a real gap: releases have no Update operation anywhere. musehub/api/routes/musehub/releases.py has create/list/get/asset-* routes — no PATCH/PUT, and (a second, more surprising finding while verifying the fix) no DELETE route for the release entity itself either. The only @router.delete in that file is /repos/{repo_id}/releases/{tag}/assets/{asset_id} — removing an asset from a release, not the release. Release deletion exists only through the MuseWire binary protocol (delete_release_by_tag is called from musehub/api/routes/wire.py, used by muse release delete <tag> --remote <name>) — there is no REST path at all. muse hub release delete --hub <url> (the plain REST-API CLI command, distinct from the wire-protocol muse release delete --remote) genuinely 405s against any server, not because of stale code — confirmed by rebuilding and verifying both the musehub API and musehub_worker containers were running fresh images before re-testing and getting the identical 405. muse hub release --help undersells the gap further: list | create | read | delete | asset-list | asset-attach | asset-delete implies a working REST delete that doesn't exist — the CLI command is real, it just can't succeed against a plain REST deployment, only against the wire-protocol path. The only way to change a release's title or body today is delete-then-recreate through the wire protocol specifically, which is destructive (needs explicit sign-off every time), loses the original release_id, and is the wrong shape for what should be a routine edit.

Checking whether this was a one-off oversight or a real pattern gap, a direct grep across every router in musehub/api/routes/ for @router.patch/ @router.put found working update routes for identities, collaborators, issues, labels, users, domains, repos, mists, and proposals — releases are the outlier. So is one other entity: webhooks have create and delete but no update route either (musehub/api/routes/musehub/webhooks.py) — you can't change a webhook's URL or subscribed events without deleting and recreating it, the identical gap.

Delete semantics turned out to be more interesting than expected. Reading the actual delete implementations (not assuming) surfaces a real, three-tier pattern that already exists across the codebase, just never written down:

  1. No delete route at allissues and proposals cannot be deleted, period. They can be closed/reopened (a status field), but the row is permanent. This is deliberate: an issue or proposal is part of the project's historical record.
  2. Soft delete — issue comments ("Soft-delete a comment... marks it as deleted; it is excluded from list results") and attestations (revoked_at set, "never deletes the row (immutable audit trail)") are never physically removed. The row stays; a flag marks it retired.
  3. Hard delete — mists and releases both call await session.delete(row) directly. The row is gone the moment the request succeeds. No recovery path exists in either case. For mists, this is exposed as a normal REST route. For releases, this same service function is only reachable through the MuseWire protocol — the REST API has no delete route for releases at all, confirmed above, not merely undocumented.

Releases sit in tier 3 today (in spirit — hard, unrecoverable delete), alongside mists, but reached through a completely different protocol path than every other entity's delete operation. That's worth questioning directly rather than leaving as an accident: a mist is closer to arbitrary user content (you might legitimately want to fully scrub one), while a release is a permanent marker of what actually shipped — structurally closer in spirit to an attestation (an audit-trail record you retire, not erase) than to a mist. This plan does not assume the answer; Phase 1 makes it an explicit, documented decision instead of an implicit one nobody chose.

The broader principle behind this ticket — "every entity should have real CRUD, following the RESTful convention (index/show/create/update/destroy) consistently" — is the actual reason this plan exists. Releases and webhooks are the two confirmed gaps; this plan fixes both and produces a definitive, checked audit of everywhere else, so any further gap becomes its own tracked ticket instead of a surprise discovered mid-incident again.

Goal

  • muse hub release update <tag> (and the matching PATCH /api/repos/{repo_id}/releases/{tag} route) exists, editing a release's descriptive metadata without ever requiring delete-and-recreate again.
  • Releases' delete semantics are an explicit, documented decision — soft- delete-by-default with a --hard escape hatch, or confirmed-intentional hard-delete — never left as an implicit accident the way it is today.
  • Webhooks get the identical Update fix, since it's the identical gap found by the identical audit.
  • A real, checked audit of every musehub entity's CRUD completeness against the RESTful convention exists as a concrete artifact (a table, verified against actual route registration, not grep-and-assume).
  • Every additional gap the audit surfaces is filed as its own follow-up ticket — this plan does not silently absorb unrelated scope.

"Done" means: muse hub release update works end-to-end against real staging data (closing the loop on the incident that triggered this ticket), release delete behaves according to a decision this plan actually made and tested, webhooks have the same update capability, and the audit table is a real artifact anyone can check future entities against.

Non-Goals / Out of Scope

  • Fixing every gap the audit finds in this ticket. Phase 1 produces the list. Only the two gaps already confirmed here (releases, webhooks) are fixed in this plan; anything else the audit surfaces gets filed as a separate ticket, not folded in.
  • Changing delete semantics for any entity other than releases. Issues' and proposals' no-delete stance, and attestations'/mists'/comments' existing soft/hard-delete behavior, are working as designed. Not touched.
  • A generic, reusable "soft-delete framework." If releases need a deleted_at field, it's added directly, mirroring attestations' revoked_at pattern exactly — not architected as abstract middleware for a need that doesn't exist yet.
  • Retroactively soft-deleting anything already hard-deleted. Rows already gone via the existing hard-delete path stay gone; this plan only changes behavior going forward.

Design

Update — what's mutable, what isn't

A release's tag, commit_id, snapshot_id, channel, and semver components (major/minor/patch/pre/build) are facts about what was actually released — derived from the commit graph at creation time, per muse release suggest's own stated philosophy ("a machine-verifiable claim... not a number someone typed"). Letting an update endpoint silently rewrite any of these would let someone lie about history after the fact. Only title and body — genuinely descriptive metadata, exactly the two fields this ticket's triggering incident needed to fix — are mutable via update. changelog is derived and also immutable through this path.

PATCH /api/repos/{repo_id}/releases/{tag}
Body: { "title"?: string, "body"?: string }
Rejects (422) any attempt to include tag/commit_id/snapshot_id/channel/
semver_*/changelog in the body — these fields are not part of the update
schema at all, not merely ignored, so a client can't be confused into
thinking it changed something it didn't.

muse hub release update <tag> --title <str> --body <str> mirrors the CLI shape of every other update command. Given release bodies can carry substantial copy (the exact scenario that triggered this plan), it should also accept --body-file <path>, following the workspace's own local-file convention already used for issues and proposals with real body content — read the local file first, edit it, push via --body-file, never inline --body "..." for anything longer than a one-liner.

Delete — the tiering decision for releases

This plan's recommendation, to be confirmed (not assumed) in Phase 1: move releases from tier 3 (hard) to tier 2 (soft), matching attestations' precedent exactly — add a deleted_at timestamp, excluded from list_releases's default results (mirroring the existing is_draft exclusion pattern already in that function), with a --hard flag on muse hub release delete for the rare case a release genuinely needs to be purged (e.g., an accidental credential leak in a release body). Default behavior becomes recoverable; --hard remains available and is exactly today's existing behavior, just opt-in instead of the only option.

Why webhooks ride along

Webhooks were found by the exact same audit query (grep for update routes across every router) that found releases' gap — not a separate investigation with separate justification. Fixing both in one plan, rather than splitting into two nearly-identical tickets, is the efficient choice; WebhookUpdate mirrors WebhookCreate's shape (url, events, active) with every field optional.


Phases

Each phase is fully green before the next begins. Test IDs use the RCRUD_NN prefix.

Phase 1 — Full CRUD audit (no new code, a report/table + confirming tests)

  • RCRUD_01 — Enumerate every entity/router under musehub/api/routes/ and produce a definitive table: entity | list | read | create | update | delete (none/soft/hard) | source file references. This plan's own Background section is a hypothesis based on spot-checks (issues, proposals, mists, releases, webhooks, attestations, comments, labels, collaborators, domains, repos, identities, users, orgs) — Phase 1 must verify every one of those against actual route registration, and check every entity this plan's research did not already spot-check.
  • RCRUD_02 — For each "no delete route" and "soft delete" entity found, confirm by reading the actual service-layer implementation (not just the route decorator) that the behavior matches its documented summary — e.g. confirm delete_comment really does exclude soft-deleted comments from list results, don't just trust the docstring.
  • RCRUD_03 — Document the finalized decision on releases' delete tier (soft-with---hard-flag per this plan's recommendation, or a considered alternative) as a written decision in this ticket before Phase 3 starts — not decided implicitly by whatever gets implemented first.

Phase 2 — Releases Update

  • RCRUD_10PATCH /api/repos/{repo_id}/releases/{tag} — updates title/body only; the request schema itself excludes every immutable field (not just silently ignoring them) so a 422 on an out-of-schema field is the actual enforcement mechanism.
  • RCRUD_11muse hub release update <tag> --title --body --body-file --json, following the existing local-file convention for substantial body content.
  • RCRUD_12 — Server-side test: attempting to include tag, commit_id, snapshot_id, channel, any semver_* field, or changelog in the PATCH body is rejected.
  • RCRUD_13 — CLI test: muse hub release update correctly builds the PATCH request and handles both --body and --body-file (mutually exclusive — reject if both given, consistent with how other dual-input flags in this codebase behave).
  • RCRUD_14 — End-to-end regression against a real (or staging-fixture) release: update title and body, confirm commit_id/snapshot_id/ changelog are byte-identical before and after.

Phase 3 — Releases delete: add the missing REST route, then apply the delete-tier decision

  • RCRUD_20 — Add DELETE /api/repos/{repo_id}/releases/{tag} to musehub/api/routes/musehub/releases.py — this route does not exist today; muse hub release delete --hub <url> currently 405s against every deployment because there is nothing for it to call. This is the load- bearing fix Phase 3 exists for; the soft/hard tiering below is designed into it from the start, not retrofitted onto pre-existing behavior.
  • RCRUD_21 — Schema migration adding deleted_at: datetime | null to the releases table — additive, backward-compatible, existing rows get NULL.
  • RCRUD_22 — Default behavior of the new REST route is soft: sets deleted_at, excluded from list_releases's default results (same exclusion mechanism as the existing is_draft filter), row and all its data preserved.
  • RCRUD_23muse hub release delete <tag> --hard calls delete_release_by_tag (the existing, unchanged session.delete(row) service function already used by the wire-protocol path) — genuinely permanent, opt-in only, never the default. This reuses the same service function the wire protocol already calls; the new REST route and the existing wire-protocol path converge on identical delete semantics rather than diverging into two different implementations of "permanently delete a release."
  • RCRUD_24 — Existing hard-delete tests (from before this plan, against the wire-protocol path) become the --hard path's regression suite — confirm they still pass unmodified against the flag-gated behavior.
  • RCRUD_25 — New tests for the soft-delete default: deleted release excluded from list, still readable by direct tag lookup with an explicit --include-deleted flag (mirroring --include-drafts's existing shape), never returned by default read.
  • RCRUD_26muse release delete <tag> (the local/remote-retract command used in the incident that triggered this plan) updated to match — soft by default, --hard for the old behavior — with its own regression test since it's a distinct code path from the hub release delete CLI command.
  • RCRUD_27 — Regression test confirming muse hub release delete --hub <url> (REST) and muse release delete --remote <name> (wire protocol) now produce identical outcomes for the same tag — the gap that caused this ticket (one worked, the other 405'd) must not resurface as "both work, but differently."

Phase 4 — Webhooks Update

  • RCRUD_30PATCH /api/repos/{repo_id}/webhooks/{webhook_id}url/events/active all optional, mirroring WebhookCreate's shape; url re-validated against the existing SSRF rules on update, not just at creation time (an update is a new opportunity to point a webhook somewhere it shouldn't go).
  • RCRUD_31muse hub webhook update <webhook-id> --url --event --active/--inactive --json.
  • RCRUD_32 — Tests mirroring Phase 2's structure: server-side field validation, CLI request construction, end-to-end update-and-verify.

Phase 5 — Documentation and follow-up ticket filing

  • RCRUD_40 — Document the three-tier delete convention explicitly in MuseHub's developer docs (a natural addition alongside docs_muse_identity.html's existing pattern of documenting real, checked architecture) — this stops being tribal knowledge once it's written down with the actual entities in each tier named.
  • RCRUD_41 — File one staging ticket per additional confirmed gap from Phase 1's audit (e.g., if orgs turn out to have no update route — a real possibility given org identity is governed through the identity repo rather than a simple mutable DB row, worth checking rather than assuming either way) — filed as new tickets, not implemented here.

Acceptance Criteria

  • muse hub release update v0.2.0-nightly.2 --title ... --body-file ... works end-to-end against real staging data — the actual incident that triggered this plan is genuinely closed, not worked around.
  • A PATCH attempting to change tag/commit_id/snapshot_id/channel/ any semver component/changelog is rejected with a 422, verified by a real test, not just documented as an intention.
  • muse hub release delete --hub <url> (REST) works at all against a plain MuseHub deployment — verified against a fresh, freshly-restarted local instance, not just staging (staging's success masked this gap; a local MuseWire-less REST-only check must pass too).
  • muse hub release delete soft-deletes by default (release excluded from list, read without --include-deleted, but recoverable/inspectable); --hard performs full, permanent removal via the same delete_release_by_tag function the wire protocol already uses.
  • muse release delete --remote <name> (wire protocol) and muse hub release delete --hub <url> (REST) produce identical outcomes for the same tag and the same flag.
  • Existing pre-plan hard-delete test coverage (against the wire-protocol path) passes unmodified under the --hard flag.
  • muse hub webhook update exists and is tested with the same rigor as releases' update path.
  • Phase 1's audit table is a real, included artifact in this ticket/PR — not asserted, checked against actual route registration for every entity, including ones this plan's own research didn't already spot-check.
  • No regression in any existing release, webhook, mist, issue, proposal, attestation, or comment test.

Risks

  • Migration risk: adding deleted_at to a production-adjacent table needs the standard additive-migration discipline already documented in this workspace's musehub deploy runbook (generate, rename to the numbered convention, verify alembic current/heads before and after) — treat this as a real migration with the same care as any other, not a trivial column add.
  • Immutability-enforcement risk: the PATCH schema excluding immutable fields (rather than accepting-and-ignoring them) is the right design, but needs a test that actually sends those fields and checks for rejection — a schema that merely happens to lack a field is not the same as a schema that has been verified to reject it, especially if a future refactor accidentally widens the request model.
  • Scope-creep risk from the audit itself: Phase 1 will likely surface more than two gaps once every entity is actually checked. The explicit non-goal above (file, don't fix) exists specifically to prevent this ticket from silently absorbing an open-ended amount of unrelated work — hold that line when Phase 1's results come in, even if a newly-found gap looks trivial to fix while you're already in the file.
  • Soft-delete-default behavior-change risk: changing releases' delete default from hard to soft is a real behavior change for anyone already scripting against the old delete semantics (e.g., relying on a deleted tag becoming immediately re-creatable). Mitigation: RCRUD_23's regression suite plus clear documentation (RCRUD_40) of the new default, and the fact that --hard reproduces the exact old behavior for anyone who needs it.

Open Questions

  • Does the Phase 1 audit actually confirm releases should move to tier 2 (soft), or does it surface a reason releases are more like mists (tier 3, hard) than attestations (tier 2, soft) that this plan's Background section didn't consider? This plan states a recommendation, not a foregone conclusion — RCRUD_03 is where that gets decided for real.
  • Should muse release delete (local/remote-retract) and muse hub release delete (direct hub API) converge on identical soft/hard flag semantics, or is there a reason for them to differ given they operate through different paths (local release record vs. direct API call)? Needs a decision during Phase 3, not assumed to be identical by default.
  • Do orgs (from the companion org/quorum wiring ticket) need an update route, or is org identity intentionally immutable-except-through-the- identity-repo's own commit history? This plan doesn't assume org-update is a gap — Phase 1 should check it explicitly rather than lump it in with releases/webhooks by assumption.
  • Should webhook secret be updatable through this same PATCH endpoint, or does rotating a webhook secret deserve its own dedicated endpoint (mirror of how key rotation is its own distinct operation from a general profile update in the identity system)? Leaning toward a dedicated rotate endpoint, consistent with that precedent, but flagged for a decision during Phase 4, not assumed here.

Implementation Order

Phase 1 → Phase 2 → Phase 3 → Phase 4 → Phase 5. Phase 1 is strictly load- bearing: Phase 3's exact implementation depends on Phase 1's tiering decision (RCRUD_03), and Phase 5's follow-up tickets can't be filed accurately until Phase 1's audit is actually complete. Phase 2 (Update) has no dependency on Phase 3's decision and could technically run in parallel, but is kept sequential here since it's the smaller, higher-urgency fix (directly closes the triggering incident) and shouldn't wait on the audit to complete.


Plan-only issue. Each phase's deliverables should be checked off in place — this is a wiring/completeness project against existing architecture, not a new domain requiring further decomposition into separate staging issues.

Activity
gabriel opened this issue 58 days ago
No activity yet. Use the CLI to comment.