Skip to content

fix(review): name restores in card side-effect disclosures - #3018

Merged
Chris0Jeky merged 3 commits into
mainfrom
codex/issue-3008-restore-side-effect-copy
Sep 12, 2026
Merged

Chris0Jeky merged 3 commits into
mainfrom
codex/issue-3008-restore-side-effect-copy

Conversation

@Chris0Jeky

@Chris0Jeky Chris0Jeky commented Sep 11, 2026

Copy link
Copy Markdown
Owner

Summary

Closes #3008 once qualified and merged.

  • Include “restores” in the Cards side-effect sentence when a card-targeted restore-lifecycle operation is present.
  • Cover both card-only and mixed card/column rows, with the existing ordinal-case-insensitive classification.
  • Preserve all non-restore wording and all seven categories, tones, risk posture, webhook logic and execution behavior.

This is a bounded disclosure repair inside SideEffectAnalyzer, not an execution change or a side-effect redesign.

Regression coverage

12 new native cases cover restore-only action/target casing, mixed archive/restore with and without columns, unchanged create/move/archive-only wording, a non-card restore, empty operations, and parity of the public persisted-proposal and effective-snapshot overloads. Existing SideEffectAnalyzerTests are unchanged.

Verification / draft gate

Baseline source bytes were verified against blob 145652b98884f55a8eb4e79aeb356b72756213d9; the prepared implementation blob is 3cbedb99e1f78dc338b320c8ae4633dffd43a0af.

NOT RUN locally: .NET compilation/tests, full solution and repository docs/link checks; this environment has no .NET SDK or direct package/repository network. No synthetic C# pass is claimed. Keep draft until exact-head hosted CI and independent review are recorded.

Commands and scope: docs/analysis/2026-09-11-restore-side-effect-disclosure.md.

Coexistence

Independent branch from main 4c479a7ff311912ffb96da027f6e525521bb70df, not stacked on #3010 or #3014. No CI/control-path or shared canonical documentation edits. #2950's readable-diff vocabulary remains a separate issue. No merge, deployment, or human-action check-offs.


Claude lane qualification (2026-09-12, alpha-product-trust)

The Codex lane released this draft on 2026-09-11; this lane took it over for qualification and landing. The draft gate above is now satisfied, so the PR is marked ready for review.

Ran locally in an isolated worktree at head 7e48d31f6:

  • dotnet test backend/tests/Taskdeck.Application.Tests/Taskdeck.Application.Tests.csproj -c Release -m:1 --filter "FullyQualifiedName~SideEffectAnalyzer" - 64/64 passed (52 existing + 12 new).
  • Attack first: with SideEffectAnalyzer.cs reverted to the origin/main source and only the new test file kept, the lifecycle-disclosure class runs 6 failed / 6 passed - the restore-only casing cases, both mixed archive/restore cases and the persisted/effective parity case all go red without the fix, so the regression coverage genuinely pins the issue's failing case.
  • node scripts/check-docs-governance.mjs and node scripts/check-doc-links.mjs - both pass.

Added on landing: one bounded docs/STATUS.md delivery paragraph (the reviewer-facing copy is user-visible shipped reality) and the Last Updated date. No source or test change from the draft head.

Not run locally: the full backend solution - hosted Backend Unit and API Integration (both runners) were green at 7e48d31f6 and are re-running at the STATUS head; CI is the solution-level evidence for this docs-only delta.

Refs #3008. Preserve non-restore copy and add public-path lifecycle disclosure regression tests.

Copy link
Copy Markdown
Owner Author

Exact-head qualification update for 7e48d31f66c9d871208640b9a430c2472c6996ac: hosted application-test steps passed on both Linux and Windows (jobs 103384330649 and 103384330636). Docs governance and architecture jobs also passed. CI run: https://github.com/Chris0Jeky/Taskdeck/actions/runs/34636058063. CLI/API integration and other checks remained unfinished at observation; full CI and independent review are not claimed complete, so the PR stays draft. Local .NET execution remains unavailable; no synthetic C# pass is being substituted. Full lane release: #3008 (comment) .

@Chris0Jeky
Chris0Jeky marked this pull request as ready for review September 11, 2026 23:32
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Review round 1 - triage (Claude lane, alpha-product-trust)

Codex connector: its only comment on this PR is a usage-limit notice ("You have reached your Codex usage limits for code reviews"), posted 2026-09-11 23:32Z. There is no Codex review to triage. Under global law 2g the gate is therefore carried by one fresh-context independent adversarial review, run below.

Independent review (fresh context, read-only, given the full patch and issue #3008): no CRITICAL and no HIGH. It explicitly refuted four suspected defects rather than asserting them - the unstubbed IUnitOfWork in the test helper is safe because ProposalDto.BoardId is null so the webhook branch is never entered; hasCardRestore strictly implies hasCardMutation, so the new summary can never leak into the column-only branch; restore-lifecycle is the only card-restore verb the contract validator and handler registry accept, so keying on that exact string is complete; and the existing SideEffectAnalyzerTests negative assertions (:355, :584) and AutomationProposalsApiTests.cs:1140 only fire on column-only proposals or on "No board mutations", so nothing contradicts the new copy. It also confirmed ReviewSideEffects.vue:28 renders the value verbatim and no frontend spec or locale file carries the old sentence.

Fixed here (7cfbe7b40)

MEDIUM - the evidence note contradicted the STATUS paragraph in the same PR. docs/analysis/2026-09-11-restore-side-effect-disclosure.md still said native tests were NOT RUN and that "STATUS/MASTERPLAN were not modified", while the landing commit both ran them and added a STATUS paragraph. Real: STATUS is labelled Source of Truth and .claude/rules/docs.md tells later sessions to reconcile it against execution evidence, so two mutually exclusive same-commit records is a trap. The note now dates the NOT RUN sentence to the drafting session and records the executed run (64/64 green; six red against the pre-fix analyzer) and the satisfied draft gate. Docs-only; no source or test change.

Tracked, not fixed

MEDIUM - the Cards row omits delete (and update/bulk_move) exactly as it omitted restore, so a delete-only proposal reads "Creates, moves, or archives cards on the board" - naming the recoverable action and hiding the irreversible one. Pre-existing, not introduced by this diff, and outside issue #3008's explicit "no broad side-effect redesign" boundary. Filed as #3031, which also carries the related LOW: a restore-only proposal is still told the proposal may create and archive cards, which the per-action wording option #3008 offered would have avoided.

Declined

LOW - the new tests are coupled to BoardId staying null. Accurate: Analyze uses an unstubbed IUnitOfWork, so a future extension that sets a board id would throw NullReferenceException instead of failing an assertion. Maintenance quality on a test helper with no production path; the reviewer's own recommendation was not to respin for it. Declining rather than expanding the fix diff - it is one Setup line for whoever next extends that class.

Informational, no action: layer purity clean (no new usings, nothing for Architecture.Tests); the seven rows reach Spanish and Italian reviewers in English because the server string is interpolated, which is pre-existing for all seven and unchanged here.

Verification after the fix

The fix commit is documentation only and touches no governed doc, so the re-verification is scoped to it: node scripts/check-docs-governance.mjs and node scripts/check-doc-links.mjs pass (696 Markdown files, 0 broken relative links). The SideEffectAnalyzer source and tests are byte-identical to the reviewed head, so the 64/64 filtered Application run still stands; hosted CI is re-running at 7cfbe7b40.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Backend][Review] Cards side-effect row omits restores for restore-lifecycle proposals (from PR #3001 Codex review)

1 participant