Repository navigation
fix: reflect habit operation reversibility in previews - #701
Merged
thomasluizon merged 4 commits intoOct 5, 2026
Merged
Conversation
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes Reviewed the habit reversibility metadata, preview contract, approval paths, and regression tests across all three commits, including compatibility with the consuming client.
- Risk classification: Bulk create, log, skip, update, and reschedule now use low risk; habit deletion retains destructive classification and fresh confirmation.
- Preview contract: Optional nullable
removesDatais appended to preview items and OpenAPI, with explicit flags for the scoped habit writes and deletion previews. - Approval preservation: Chat write holds still require fresh confirmation independently of catalog risk, including pending execution and clarification resolution.
- Regression coverage: Tests assert exact preview flags, legacy compatibility, catalog metadata, action keys, and actual executor approval responses. The focused suite passed 183 tests; the full suite passed all 9,375 tests, and the solution build completed with zero errors.
- Consumer compatibility: Checked
orbit-ui-mobileon bothmainandredesign/main. Existing schemas accept the changed risk value and approval controls remain available; consumption of the additive item flag can follow the API-first rollout described in the PR.
gpt-6.1-sol | 𝕏
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes Reviewed the calendar preview changes in b6d2f969 since the prior review at c265a122, with the full PR diff as context.
- Corrected dismissal metadata: Marked calendar suggestion dismissal as non-removing while retaining the shared loader's deletion defaults, ownership filtering, and state fingerprints; verified that execution retains the suggestion and sets its dismissal timestamp.
- Added calendar regression coverage: Tested owned versus foreign suggestions, fingerprint changes, auto-sync current and proposed values, run-sync action fields, and unknown-action handling.
- Verified regressions: Passed all 68 focused preview, contract, and chat approval tests and all 9,379 tests in the full suite. The solution build succeeded with zero errors after restoring missing project assets; existing dependency warnings remain.
gpt-6.1-sol | 𝕏
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.




Bulk habit creation, logging, skipping, updating and rescheduling inherited a destructive classification, so their approval previews could imply data deletion. These operations now use low risk in the catalog. Habit deletion keeps its destructive classification and fresh confirmation requirement. The existing chat write hold still requires approval for every applicable Astra write.
AgentCatalogService.Capabilities.cscorrects the shared bulk write capability, which supplies both chat and MCP operation metadata.AgentContracts.csappends optional nullableremovesDatatoPendingOperationItem.PendingOperationChangePreviewer.csandAgentArgumentPreview.csreport false for the scoped habit writes and true for habit deletes.DestructiveOperationPreviewer.csalso marks its existing deletion rows, while calendar settings and sync actions report false. The generatedsrc/Orbit.Api/openapi.jsonadds only this field.The signal belongs to each preview item so a batch can distinguish deletions without deriving them from internal risk classes. Existing response fields retain their names and types. Unit tests cover catalog metadata, actual executor approval responses, mixed preview items, legacy deserialization and the generated optional contract. Policy and pending-store tests distinguish static catalog confirmation from the chat approval hold.
Closes thomasluizon/orbit-tickets#1216
Deploy the API before the consuming UI ticket updates its shared Zod schema and presentation.
Assumptions
removesDataon eachPendingOperationItem, rather than a batch-level flag; it is nullable to preserve an unspecified value for unrelated generic previews instead of guessing from their risk class.TrueandFalse) rather than changing existing fields to lowercase.Test evidence
Before implementation, unchanged catalog and preview tests passed with the defect present:
Observed: 14 infrastructure and 21 application tests passed. Unchanged approval, destructive-preview and argument-preview tests also passed with:
Observed: 54 infrastructure and 14 application tests passed.
Before fixing the catalog, these strengthened tests failed:
Observed: 10 failures.
HabitOperations_ClassifyReversibilityandExecuteAsync_EveryChatWrite_WaitsForApprovalfound destructive risk where low risk was expected for each of the five bulk writes. After the catalog fix, the same command with--no-buildpassed all 73 tests.Before adding the preview field:
Observed: seven failures, including strengthened
PreviewAsync_Create_UsesStableInputIndexesandPreviewAsync_BulkDelete_ReturnsTargetPreview, because serialized items lackedremovesData. After the fix, all 26 preview tests passed within the 139-test focused preview, contract, revision, catalog and executor run. The updated policy, pending-store and approval tests then passed all 116 focused cases.The first broader run exposed four stale tests that expected bulk writes to remain statically destructive or confirmation-gated. Their replacement assertions retain deletion gates and verify that chat approval preserves low risk and existing action keys.
Final full solution checks:
Both builds completed with zero errors. Both full test runs passed 9,375 tests: 32 analyzer, 661 domain, 5,426 application and 3,256 infrastructure tests. Existing dependency and obsolete API warnings remain. All three commits passed the repository pre-commit hooks; the working tree is clean.
env -u LANG dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~DestructiveOperationPreviewerTests: the 2 existing tests passed with the defect present; with the new tests and no fix, 5 passed and 1 failed (DismissCalendarSuggestion_PreviewsOwnedSuggestionWithoutRemovingDataAndDetectsStateChange,RemovesDatawastrue); after the fix all 6 passed.env -u LANG dotnet build Orbit.slnxandenv LC_ALL=en_US.UTF-8 dotnet build Orbit.slnx: exit 0, zero errors.env -u LANG dotnet testandenv LC_ALL=en_US.UTF-8 dotnet test: exit 0, 9,379 passed each, zero failures or skips.Manual steps
None.