fix(review-queue): atomic stale-safe review transition #131
Loading…
Reference in a new issue
No description provided.
Delete branch "kreativmonkey/fix-issue-95"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Queue completions now use an explicit
review_completesignal with a status-guardeddraft→pendingUPDATE in both stores; a stale action on a post that already left the draft state conflicts with 409 (review_conflict) instead of overwriting newer composer-scheduled state.review_completemust map to exactly one explicit action —publish_now: true, ordraft: falsewith an explicitly suppliedscheduled_at— and always requireswrite:schedule. Missing/incomplete/contradictory patches are rejected with 400 before Prepare, so no implicit past timestamp publishes without the publish-now marker.Past-time rejection is now a Prepare option (
RejectPastScheduledAt) so drafts andpublish_nowstay exempt while schedule paths fail on the past; agent-tools schedule/modify adopt it. OpenAPI, auth/scope DOX, and frontend queue payloads updated accordingly.Total Go coverage unchanged by this branch (make cover not run locally; store suite covered via sqlite + postgres tests).
CI runs the full suite (incl.
make test-postgres) on this branch.Invitations existed in the backend (create + accept) but had no UI and no way to review or withdraw them. Team owners can now invite people by email from team settings: the one-time invite link (?invite=<token>) is shown for sharing, pending invitations are listed with their expiry, and each one can be revoked again. - store: ListTeamInvitations / DeleteTeamInvitation (postgres + sqlite) - api: GET + DELETE /v1/teams/{teamID}/invitations[/{invitationID}] (owner-only) and audit events invitation.create / invitation.revoke - frontend: invitation card in team settings, audit filter entries - docs: invitation endpoints added to the OpenAPI spec Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>The auto-created personal workspace was a second kind of team with its own rules: a generated name ("Personal · <uid>") that could not be changed, no members, no invitations, and special-cased account migration. Onboarding (follow-up commit) replaces it: new users create their first team themselves or join one via invitation. - stop auto-creating a personal team on OIDC sign-in and admin bootstrap - startup migration converts existing personal workspaces into regular teams: markers cleared, generated names replaced with the owner's display name (email local part as fallback, unique-name collisions get a numeric suffix); custom names are kept - drop the personal special cases from team update, member management, invitations and account migration; moving an account between teams now requires owning the source team (or admin) and editor/owner access in the target - remove is_personal / personal_for_user_id from the domain model, API responses, agent tools and frontend (DB columns remain for the migration and can be dropped in a later release) - e2e: global setup seeds a team on fresh databases BREAKING CHANGE: teams no longer carry is_personal / personal_for_user_id fields, and new users start without a team. Existing personal workspaces are migrated to regular teams (renamed after their owner) on first startup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>handleGetProactiveSettings/handleUpsertProactiveSettings existed but were never wired into the router, so the frontend calls to /v1/teams/{teamID}/proactive-settings returned 404. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>Surface the running goloom version in the sidebar footer, above the user menu. When a newer release exists it turns into an accent-tinted link to the release notes. The update check runs server-side and cached: a ReleaseChecker polls the GitHub releases API (default every 24h) so a single server request covers all clients instead of every browser reaching out to GitHub. Exposed via the public GET /v1/version endpoint ({current, latest, update_available}). Dev builds and unparseable tags never trigger a hint. Opt out with UPDATE_CHECK_ENABLED=false for air-gapped/self-hosted setups (no outbound phone-home). Version compare is a small dependency-free vMAJOR.MINOR.PATCH parser. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>## What "Skip next" did not skip the next round. This fixes it, with the characterization test from the first commit reframed to match. ## The bug Two defects stacked on each other: **1. Wrong target.** The UI sent `item.next_materialize_at` as the occurrence to skip. That field is the *materialization cursor* — the first round **not yet** materialized — not the next round coming up. With a horizon active those are different dates, so the skip landed on a slot that had no posts yet. The imminent announcement and main post stayed in the calendar, nothing was renumbered, and weeks later a round nobody meant to drop disappeared instead. "Shift next" had the same bug. **2. Skip was materialize-time only.** `IsPostTemplateOccurrenceSkipped` is consulted in exactly one place — `materializeTemplateOccurrence`, when the cursor first walks over an occurrence. A round already materialized was never revisited: no deletion, no counter reset, no re-materialization. So even with the right target, the skip row alone changed nothing. ## The fix Skip and shift now go through the scheduler: - **Frontend** resolves the occurrence from the linked posts (earliest `pending`/`draft`/`failed` round) and falls back to the cursor only when nothing is materialized yet. Both skip and shift use it. - **`SkipPostTemplateOccurrence`** records the skip, then tears down every materialized round **from that occurrence onwards** and re-walks the horizon. Everything after the skipped slot has to be rewritten, because dropping a slot shifts each later edition number down by one. - **The cursor is rewound** to the skipped occurrence carrying the counters that round owned. The re-walk then steps over the slot without burning the edition number and hands it to the next round — which is the semantics the skip branch in `template_materialize.go` already implemented but never got to apply. Guard rails: published/publishing rounds block the tear-down with `409 skip_blocked_posted`, and in-flight AI enhancement for the deleted rounds is cancelled (`cancelPendingRecurringAIJobsFrom`). Without a scheduler wired in, the endpoint still records the skip and answers `204` exactly as before. ## Effect Skipping edition 385 on a 1st+3rd-Wednesday template, verified against a real sqlite store (not mocks): ``` before: 08-19=#385 09-02=#386 09-16=#387 10-07=#388 after: 09-02=#385 09-16=#386 10-07=#387 ``` The slot is dropped, its announcement goes with it, and 385 moves to the next date instead of being burned. ## Tests - `template_skip_test.go` — drop-and-renumber, unmaterialized round records only, blocked by posted round, shift keeps edition at the new time. Driven through a stateful mock store so the re-walk is exercised, not stubbed. - `template_skip_integration_test.go` — the same flow against a real sqlite store, which is what catches SQL-level mismatches (skip-row timestamp round-trip, linked-post delete, materialization rewind) that mocks cannot. Also asserts the skip survives a second materializer tick. - `post_templates_skip_test.go` — handler routes through the scheduler, passes the shift target, maps blocked to 409, and still records the skip when no scheduler is present. - `TestSkipNext_skipsBeyondHorizonNotImminentOccurrence` → renamed to `TestMaterialize_cursorRunsAheadOfMaterializedRounds`. The cursor running ahead is still true and worth pinning; it is just no longer what skip reads. ## API change `POST /post-templates/{id}/skip` now answers `200` with `{deleted_posts, regenerated_occurrences}` when a scheduler is present (still `204` without one). The endpoint is not in `docs/api/openapi.yaml` — post-templates are not documented there at all — so there is no spec to update. New i18n keys: `recurring.skipNextConfirm` (skip is destructive now, so it asks first) and error `skip_blocked_posted`, both de + en. ## Note Verified: `go build ./...`, `go test ./...`, `go vet`, `gofmt` on all touched files, plus `tsc --noEmit` and `eslint` clean on the frontend. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Sebastian Preisner <preisner@puzzle-itc.de> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.Merge
Merge the changes and update on Forgejo.Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.