Fix confirmed bulk habit creation and preserve approval on failure

[thomasluizon]

Confirmed daily bulk creates rejected an omitted quantity even though the tool schema promises a default of 1. The bulk tool now applies that default, and failed bulk execution rolls back confirmation consumption together with habit writes so the pending operation remains executable.

Closes thomasluizon/orbit-tickets#1215.

Implementation

• BulkCreateHabitsTool.cs normalizes omitted quantities to 1 when a recurrence unit exists, including sub-habits. Explicit quantities remain subject to the existing validator and entity guards; one-time tasks keep a null quantity.
• IAiTool.cs declares transaction participation for tools whose writes use the same unit of work and have no external side effects. Bulk create opts in.
• AgentOperationExecutor.cs evaluates policy and executes participating tools inside the existing unit-of-work transaction, through ExecutePolicyAndToolAsync. Exceptions leave the transaction delegate with their original type, so the Npgsql retrying execution strategy and the unit of work's timeout conversion behave as before. A failed tool result escapes as ToolOutcomeRollbackException before its failure response and audit entry are produced. This preserves approval without manually restoring a token or risking a partial write on retry.
• The completed policy decision reaches the failure audit through an explicit OperationPolicyState passed to the execution method and reset before each attempt, so the audit records the shadow decision when evaluation completed and none when evaluation threw.
• The executor logs caught exceptions at error level with operation and correlation IDs. The logged exception preserves its original type and stack, while discarding messages and inner exceptions that may contain habit text. ToolOutcomeRollbackException and RedactedOperationException are public types in their own files. The existing audit error remains available.
• BulkCreateHabitsToolTests.cs covers recurrence defaults, explicit quantities, one-time tasks, and sub-habits. BulkCreateAgentOperationTests.cs uses the existing SQLite fixture with the real catalog, confirmation store, policy evaluator, MediatR validation pipeline, handler, repositories, and unit of work.

Preview validation is excluded by the ticket's binding scope decision because its previewer exists only on redesign/main. The carry into that branch belongs to thomasluizon/orbit-tickets#746.

Test evidence

• Before implementation changes, env -u LANG dotnet test tests/Orbit.Application.Tests --filter FullyQualifiedName~BulkCreateHabitsToolTests passed all 7 unchanged tests, including ValidHabitsWithSubHabits_ReportsSuccessCount, while the defect was present.
• Also before implementation changes, env -u LANG dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~AgentExecutionAndSanitizerTests passed all 18 unchanged tests.
• The new production-path regression, ConfirmedDailyItemsWithoutQuantity_CreateEveryItemWithQuantityOne, failed before the parser fix under env -u LANG dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~BulkCreateAgentOperationTests. The audit contained the required-quantity validation error for every one of the 12 items. After the fix, that command passed and verified 12 persisted daily habits with quantity 1 and a consumed confirmation.
• Before the executor fix, the same command failed ValidationFailure_LeavesPendingOperationApprovable and PayGateFailure_LeavesPendingOperationApprovable because ConsumedAtUtc was populated. FailureBeforeWrite_LogsSafeExceptionAndAllowsSuccessfulRetry failed because no log was emitted. After the fix, all pass. Coverage also verifies rollback after habit writes, successful retry, replay rejection after success, and removal of private habit text from the logged exception.

Review batch 1 (SonarCloud reliability C on new code)

SonarCloud flagged csharpsquid:S2583 at AgentOperationExecutor.cs:258-259 (a null-initialised local read with ?. after a closure assigned it) and csharpsquid:S3871 at :401 and :408 (private nested exception types). The batch removed the captured local and moved both exception types into their own public files. AgentOperationExecutor_FailureAuditRecordsOnlyCompletedPolicyEvaluation pins the shadow decision in the failure audit when policy evaluation completed and none when it threw.

Review batch 2 (exception identity inside the transaction)

The orchestrator's review of batch 1 found that it wrapped every exception inside the transaction delegate, which hid transient database failures from the retrying execution strategy and timeout cancellations from the unit of work's TimeoutException conversion. The installed EF Core 10.0.12 unwraps only DbUpdateException before ShouldRetryOn, and Npgsql 10.0.3 treats TimeoutException as retryable.

• Red: env -u LANG dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~AgentOperationExecutor_TransactionDelegatePreservesOriginalException failed 2 cases on batch 1; both observed the wrapper instead of the original timeout or cancellation exception.
• Green: env -u LANG dotnet test tests/Orbit.Infrastructure.Tests --filter 'FullyQualifiedName~AgentExecutionAndSanitizerTests|FullyQualifiedName~UnitOfWorkTests|FullyQualifiedName~BulkCreate' passed 49 tests, including both regression cases and the retry audit tests.

Full runs on the final head

• env -u LANG dotnet build Orbit.slnx and env -u LANG dotnet test both passed: 0 build errors and 7,135 tests passed (32 analyzer, 645 domain, 3,816 application, 2,642 infrastructure).
• env -u LANG LC_ALL=en_US.UTF-8 dotnet build Orbit.slnx and env -u LANG LC_ALL=en_US.UTF-8 dotnet test both passed with the same 7,135 tests and 0 build errors.

Assumptions

• Atomic execution applies to bulk create through an explicit tool contract; wrapping every tool was rejected because other tools can produce external side effects that a database rollback cannot undo.
• The existing per-item partial-success behavior remains: successful batches consume approval. Rolling back successful sibling items because another item failed was rejected as a change to the established bulk contract.
• Exception-based rollback is retained because UnitOfWork.ExecuteInTransactionAsync commits returned results.
• Policy evaluation stays inside the transaction, so a rolled back failure also restores the approval it consumed.
• An explicit policy state object carries the decision rather than Exception.Data, because the unit of work replaces a timeout cancellation with a new TimeoutException, which would drop data attached to the original.

Manual steps

• After merge, use thomasluizon/orbit-api GitHub Actions, Release API, Run workflow from main, with environment=staging and branch=main. A successful deployment and the workflow's health and live-commit verification prove the API release took effect. In staging Astra, create three daily habits, approve and execute them, verify all three exist, then delete them.
• Ship the production fix through the same Release API workflow with environment=production and branch=main; verify the deployed commit and API health through that workflow.
• Carry the fix into redesign/main through thomasluizon/orbit-tickets#746.
• No new configuration keys, secrets, migrations or backfills are required.

[pullfrog]

✅ No new issues found.

Reviewed changes Reviewed the complete PR diff and traced bulk creation through policy evaluation, confirmation storage, transaction ownership, and the API, chat, and MCP callers.

• Quantity defaults: Recurring bulk items and sub-habits default omitted quantities to 1; explicit integer quantities remain available to existing validation, and one-time items without a quantity retain null.
• Approval and write atomicity: Bulk create opts into ITransactionalAiTool, placing confirmation consumption and habit writes in the same transaction; failed outcomes and exceptions roll back before failure responses and audits are produced. Existing per-item partial-success behavior remains unchanged.
• Failure logging: Unexpected execution failures receive error-level logs with operation and correlation IDs while excluding original exception messages and inner exceptions from the logged wrapper.
• Regression verification: All 61 focused tests passed: 13 bulk-tool tests, 23 bulk-execution/executor tests, and 25 policy/confirmation-store tests. The executor suite passed on retry after an initial Segmentation fault (core dumped); the full solution suite and PostgreSQL concurrency behavior were not rerun in this review.

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

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
89.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[pullfrog]

✅ No new issues found.

Reviewed changes Reviewed the changes since 82226cc4, with the full PR diff and the policy, confirmation-store, audit, and unit-of-work paths as context.

• Preserved transaction exception identity: Kept unexpected exceptions unchanged inside the transaction delegate so retry classification and timeout conversion remain available before failure logging and auditing.
• Isolated per-attempt policy state: Extracted policy/tool execution and failure handling, resetting the stored decision before each evaluation so failure audits cannot reuse a previous retry's shadow decision.
• Extracted exception types: Moved rollback and redacted logging exceptions into public types in separate files without changing their payload or redaction behavior.
• Verified regressions: Passed all 77 focused parser, executor, bulk-execution, unit-of-work, policy, and confirmation-store tests after serial dependency restore resolved initial test-command timeouts. The full solution suite and PostgreSQL concurrency behavior were not rerun in this review.

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