[thomasluizon]

Skipping a habit now records the state needed to undo that single skip. Undo restores its due date, schedule anchor and completion state, removes only its flexible skip log, and refuses newer habit or log changes. Repeating a completed undo is a no-op.

Closes thomasluizon/orbit-tickets#1052

Approach

• HabitSkipUndo and its EF migration persist the pre-skip schedule, created log ID and post-skip state fingerprint. SkipHabitCommand and UndoSkipHabitCommand share the user transaction lock, goal synchronization and cache invalidation. HabitSkipUndoWriter conditionally updates the habit against its recorded timestamp, protecting edits that arrive after validation without changing ordinary habit writers.
• HabitsController keeps the existing skip response at 204, adds optional SkipHabitRequest.skipId, and exposes POST /api/Habits/{id}/skip/{skipId}/undo. A client generates one UUID per skip, sends it as skipId, and keeps it for undo and retries. Missing receipts return 404; newer state returns 409.
• Astra's existing skip tool now dispatches the same application command and returns the persisted skip_id. Both Astra and MCP expose undo_skip_habit; MCP uses the existing executor policy and audit bridge. The catalog and pending approval action key are registered.
• src/Orbit.Api/openapi.json is regenerated. Handler, domain, validator, conditional-write, controller, MCP and OpenAPI contract tests cover the behavior. Architecture artifacts were regenerated and remain uncommitted as instructed.

Test evidence

• Before changing the agent skip path, dotnet test tests/Orbit.Application.Tests --filter FullyQualifiedName~SkipHabit --verbosity quiet passed all 60 unchanged tests, including SkipHabitToolTests.SuccessfulSkip_RecurringHabit_AdvancesDueDate, while the tool still bypassed the application command.
• Strengthening that test to require the caller's SkipHabitCommand, then running dotnet test tests/Orbit.Application.Tests --filter FullyQualifiedName~SuccessfulSkip_RecurringHabit_AdvancesDueDate --logger 'console;verbosity=normal' before the dispatch fix failed with ReceivedCallsException: no matching command was sent. After the fix it passes, including in the full suite.
• Focused application checks passed 250 tests; focused infrastructure checks passed 134 tests. These include restoration across fresh contexts, existing logs and tombstones, monthly and legacy anchors, end-date completion, duplicate undo, ownership, later edits and a write-time race.
• The conditional update's affected-row result is exercised through actual EF Core and SQLite execution in HabitSkipUndoWriterTests, for both a matching row and a concurrent edit. No external service response fields or fixtures are introduced.
• dotnet build Orbit.slnx --verbosity quiet: succeeded with 0 errors.
• dotnet test --verbosity quiet: all 8,570 tests passed (32 analyzer, 656 domain, 4,908 application, 2,974 infrastructure), with no skips. git diff --exit-code -- src/Orbit.Api/openapi.json is clean after the build.
• Before the fix: env -u LANG dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~HabitSkipUndoWriterTests --logger 'console;verbosity=normal' passed both unchanged tests.
• Before the fix: env -u LANG dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~ReviewSkipBaselineTests --logger 'console;verbosity=normal' failed at the intended assertion: expected April 2, found April 3.
• After the fix: the reproduction and 104 focused tests passed.
• env -u LANG dotnet build Orbit.slnx: zero errors.
• env -u LANG dotnet test: all 8,579 tests passed, none skipped.
• Architecture generation and all commit hooks passed. EF database-value behavior was verified through actual SQLite executions.

Assumptions

• The UI supplies an optional skip UUID while existing callers keep 204 responses; rejected replacing the existing response body or status.
• Flexible undo soft-deletes the skip-created log so sync receives its removal; rejected hard deletion, which would leave offline copies. Existing logs and tombstones are untouched.
• The UI contract snapshot belongs to the paired Keep Astra request prefixes stable so the provider cache serves them #1053 work; rejected editing a second repository in this API-only order.
• Used the existing SQLite persistence test folder rather than creating a new integration harness.

Manual steps

• After this reaches API redesign/main, Keep Astra request prefixes stable so the provider cache serves them #1053 must rebaseline UI packages/shared/src/types/__generated__/api.generated.ts and packages/shared/src/types/__generated__/api.spec.commit against the carrying API commit. In the UI repository's GitHub Actions screen, run Redesign drift control (redesign-drift.yml) on redesign/main, or regenerate from that pinned spec with its existing orval configuration. The generated snapshot must contain the undo route and optional skip ID, and contract drift checks must pass.
• Keep Astra request prefixes stable so the provider cache serves them #1053 must send and retain skipId for the Undo toast, add the new endpoint constant and callers, and handle the pending operation action key undoSkipHabit in the UI approval copy. Deploy the API carrying the migration before those UI callers; a successful skip followed by undo verifies activation.
  None specific to this fix. No new configuration, secrets, migration, or backfill is required.

[pullfrog]

Caution

Undo can overwrite a committed schedule edit when the skip receipt is captured from a stale tracked habit. The two-context SQLite reproduction confirms this data-integrity issue.

Reviewed changes Reviewed the complete PR diff, its persistence and agent execution paths, and the existing consumer contract.

• Persisted undo receipts: Records pre-skip schedule state and post-skip guards, restores a single skip, and creates a sync tombstone for its flexible log.
• Conditional persistence: Adds the receipt table, transactional locking, write-time timestamp checks, habit-ceiling checks, goal synchronization, and cache invalidation.
• REST and agent surfaces: Preserves the skip endpoint's 204 response, appends optional skipId and an undo route, and registers Astra/MCP undo through the existing execution policy.
• Coverage and rollout: Adds handler, domain, validator, controller, MCP, OpenAPI, and conditional-write tests; documents API-first deployment and the follow-up UI ticket.

The existing focused suites passed 207 tests. An additional temporary SQLite regression test failed because undo replaced the newer due date with the stale baseline; its source is linked in the inline finding. No repository changes remain.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using openai/gpt-6.1-sol ｜ 𝕏}

[pullfrog]

No new blocking issues found.

Reviewed changes Reviewed 0b2ea91e, the changes since the prior Pullfrog review at 21398269, with the complete PR diff as context.

• Refreshed skip baselines: Reloaded unchanged tracked habit values under the mutation lock before validation and receipt capture, preventing undo from restoring a stale schedule.
• Preserved staged mutations: Retained pending edits when their original timestamp matches persisted state and rejected conflicting edits without saving or discarding them.
• Moved bulk reads under the lock: Loaded and refreshed selected habits before processing skips and computing due-date resolution.
• Added regression coverage: Verified preservation of committed schedule edits, valid staged changes, conflict rejection, current-schedule validation, and unrelated staged habit changes through actual SQLite execution.

All 94 focused tests passed: 83 application skip/undo/bulk tests and 11 persistence regression tests, with none skipped. The prior finding is fixed and settled. No full-suite rerun was performed in this review.

  ^{｜ View workflow run ｜ Using openai/gpt-6.1-sol ｜ 𝕏}