diff --git a/autodoc/AGENT_INDEX.md b/autodoc/AGENT_INDEX.md index a262cd4b4..8d013323b 100644 --- a/autodoc/AGENT_INDEX.md +++ b/autodoc/AGENT_INDEX.md @@ -1,10 +1,12 @@ # Agent Index - Taskdeck (seam map) -Last-Verified: 2026-08-30 (Context Fabric seam re-verified after CF-01 `#2255`; the agent tooling / CI row re-verified the same day for the Smart CI measurement tool and the CI-02 scaffold PR, ADR-0066; other regions -unchanged — agent-inventory routing verified 2026-08-18; transcript/MCP/container and map-reduce seams -remain verified 2026-08-02; the rest retain the -2026-07-13 exploration). Re-verify when a seam moves; treat a stamp older than ~90 days as stale -(a wrong map misroutes — worse than no map). +Last-Verified: 2026-09-03 (PWA/offline seam added and the frontend, transcript, Smart CI and docs rows +re-verified against `main` `d629129f3`; Context Fabric verified 2026-08-30 after CF-01 `#2255`; +agent-inventory routing verified 2026-08-18; MCP/container and map-reduce seams remain verified +2026-08-02; the rest retain the 2026-07-13 exploration). Re-verify when a seam moves; treat a stamp +older than ~90 days as stale (a wrong map misroutes — worse than no map). **Any edit to this file +bumps the stamp**, including one that only adds a row: an unbumped stamp claims verification the edit +did not do. This is the repo's seam map — a fast orientation layer for coding agents. It points to interfaces, invariants, and verification commands; it does not duplicate implementation. @@ -15,10 +17,14 @@ It is the Taskdeck equivalent of the harness `AGENT_MAP.md` (grandfathered name) - **Start here** = this file. Find your region in the seams table, jump to its entry points. - Region rules auto-load: each major directory has a scoped `CLAUDE.md` (`backend/`, `frontend/taskdeck-web/`, `scripts/agent_hooks/`) — Claude Code loads it when you touch files - there. Read that, not the whole repo. + there. Two rule files load by path rather than directory: `.claude/rules/ci-control.md` + (`.github/**`, `ci/**`, `scripts/ci/**`) and `.claude/rules/docs.md` (`docs/**`, root `*.md`). + Read those, not the whole repo. - Current shipped state: `docs/STATUS.md` (source of truth) — read the relevant section, it is - ~1.5k lines; do not read it end-to-end. Roadmap: `docs/IMPLEMENTATION_MASTERPLAN.md` (~1.7k - lines — also section-read only, never bulk-read). + 935 lines; do not read it end-to-end. Roadmap: `docs/IMPLEMENTATION_MASTERPLAN.md` (2068 + lines — also section-read only, never bulk-read). Human-action file: `OUTSTANDING_TASKS.md`. + Strategy spine: `docs/strategy/PRODUCT_DIRECTION.md` → `docs/REVIVAL_PLAN.md`. Decisions: + `docs/decisions/INDEX.md`. - Contract: `AGENTS.md`. Invariants: `docs/GOLDEN_PRINCIPLES.md`. Skills: `.codex/skills/` for Codex and `.claude/skills/` for Claude. ## Do Not Read By Default @@ -34,15 +40,16 @@ It is the Taskdeck equivalent of the harness `AGENT_MAP.md` (grandfathered name) | Domain | Entry points | Invariants (load-bearing) | Verify | | --- | --- | --- | --- | -| Capture → review → board | `Api/Controllers/CaptureController.cs`, `AutomationProposalsController.cs`; `Application/Services/LlmCaptureTriageExtractor.cs` and `TranscriptTriageChunking.cs`; `views/InboxView.vue`, `ReviewView.vue`, `composables/useReviewProposals.ts` | **Preview == Apply** (#1235: diff + executor both materialize the latest `ProposalRevision`); approve & execute are two explicit calls; execute needs an Idempotency-Key; a missing/mismatched proposal deep link never substitutes another actionable proposal; Paper post-decision receipts are board-scoped, terminal receipts are keymap-nonactionable, and only Approved offers explicit Apply; provenance server-stamped, client identity fields rejected; transcript triage uses the LLM-backed extractor only after kill-switch/provider-health/quota gates and otherwise records deterministic fallback honestly. Under-budget transcripts stay one provider call; only a budget-forced split maps chunks, and any failed map leg falls back for the complete capture. Provider-call progress renews the transcript-worker heartbeat, so readiness permits one selected-provider timeout but not an entire stuck map-reduce batch. | `TranscriptTriageChunkingTests`, `LlmCaptureTriageExtractorTests`, `TranscriptTriageLlmGoldenPathIntegrationTests`, `HealthApiTests`; capture/review unit + `CaptureApiTests`, `ProposalRevisionApiTests`; E2E `capture-loop.spec.ts` | +| Capture → review → board | `Api/Controllers/CaptureController.cs`, `AutomationProposalsController.cs`; `Application/Services/LlmCaptureTriageExtractor.cs` and `TranscriptTriageChunking.cs`; `views/paper/PaperInboxView.vue` + `views/paper/inbox/*` and `views/paper/PaperReviewView.vue` + `views/paper/review/*` (Paper is the default skin, ADR-0038, `store/paperThemeStore.ts`); `views/InboxView.vue` and `views/ReviewView.vue` are 12-line theme switches, not the implementation, and `LegacyInboxView.vue` / `LegacyReviewView.vue` are the opt-out fallbacks; `composables/useReviewProposals.ts` | **Preview == Apply** (#1235: diff + executor both materialize the latest `ProposalRevision`); approve & execute are two explicit calls; execute needs an Idempotency-Key; a missing/mismatched proposal deep link never substitutes another actionable proposal; Paper post-decision receipts are board-scoped, terminal receipts are keymap-nonactionable, and only Approved offers explicit Apply; provenance server-stamped, client identity fields rejected; transcript triage uses the LLM-backed extractor only after kill-switch/provider-health/quota gates and otherwise records deterministic fallback honestly. Under-budget transcripts stay one provider call; only a budget-forced split maps chunks, and any failed map leg falls back for the complete capture. Provider-call progress renews the transcript-worker heartbeat, so readiness permits one selected-provider timeout but not an entire stuck map-reduce batch. | `TranscriptTriageChunkingTests`, `LlmCaptureTriageExtractorTests`, `TranscriptTriageLlmGoldenPathIntegrationTests`, `HealthApiTests`; capture/review unit + `CaptureApiTests`, `ProposalRevisionApiTests`; E2E `capture-loop.spec.ts` | | Artefact intake and local extraction | `backend/src/Taskdeck.Api/Controllers/ArtefactsController.cs`, `backend/src/Taskdeck.Application/Interfaces/IArtefactTextExtractor.cs`, `backend/src/Taskdeck.Application/Services/IArtefactExtractionService.cs` | `SourceArtefact` is the immutable user-owned source; extraction appends bounded, warning-bearing history and never mutates task state | `dotnet test backend/Taskdeck.sln -c Release -m:1 --filter "FullyQualifiedName~ArtefactExtraction"`; `MigrationBootstrapTests` | | Context Fabric (durable capture, processing contracts) — ADR-0065 | `backend/src/Taskdeck.Domain/Entities/Capture.cs`, `Entities/SourceAsset.cs`, `Entities/CaptureBackfillState.cs`, `Domain/Enums/CaptureTimeline.cs`, `Domain/Enums/CaptureSourceMapping.cs`, `Domain/Enums/CaptureLegacyStateMapping.cs`, `Domain/Processing/ProcessingCapability.cs`, `Application/Services/CaptureIntakeService.cs`, `Application/Services/CaptureBackfillService.cs`, `Application/Interfaces/ICaptureStore.cs`, `Application/Interfaces/ICaptureBackfillStore.cs` (+ `IRepresentationStore`, `IBlobStore` — contracts only), `Application/Processing/ProcessorManifest*.cs`, `Application/Processing/Protocol/WorkerProtocol.cs`, `Infrastructure/Repositories/EfCaptureStore.cs`, `EfCaptureBackfillStore.cs`, `Infrastructure/Persistence/ContextFabricBootstrap.cs`; map `docs/architecture/CONTEXT_FABRIC.md`, protocol `docs/architecture/WORKER_PROTOCOL_V1.md`; **v0.5/v0.6 head-start material (planning input, not authority)** `docs/analysis/2026-08-30-acceleration-bundles-v0.5-v0.6/` — one corrected file per v0.6 issue, `v0.5/CANDIDATE_MAP.md`, contract-draft schemas/fixtures under `contracts.manifest.json`, reference candidates; read it when a CF issue is admitted | A capture is valid the moment its source is stored and never becomes unreadable through job failure; `Capture.Id = LlmRequest.Id` (ID-preserving); capture state is three orthogonal axes (user disposition · processing summary · action state) and the timeline is a projection, never the only persisted truth; typed/pasted text is an immutable `SourceAsset`, never job state, and a post-intake edit appends a SUPERSEDING asset rather than rewriting one; every capture creation path writes through `CaptureIntakeService` (architecture-test enforced — nothing else constructs a `Capture` or writes the `Captures` set); processors declare capabilities from `ProcessingCapability` and hold no mutation tools, and sidecars and remote processors may declare only externalizable capabilities (no `context.resolve` / `change.plan` / `change.verify`); Worker Protocol is v1-alpha until PdfPig and WhisperX both pass conformance; `IBlobStore` deletes only on the last released reference; `IRepresentationStore` is a draft with a nullable `CaptureId` only for the migration window; **since CF-01 `#2255` `ContextFabric:DualWriteCaptures`, `BackfillCaptures` and `ReadCapturesFromStore` all default ON** — the startup pass (`ContextFabricBootstrap`, idempotent **divergence** join, marker in `CaptureBackfillStates`) both fills the aggregate and reconciles any capture whose queue row moved past it, and Inbox list/get read a capture's own material (source text, source snapshot, intake time) through `ICaptureStore` while job state stays on the queue row; the read switch has three guards — the marker, a per-item fallback, and a divergence check that prefers whichever writer moved last — so a capture can neither disappear from the Inbox nor go backwards in it; a listing reads the narrow `GetListMaterialForUserAsync` projection, never whole aggregates; no new `CaptureSource` values and no new request-type lane predicates — dimensions come from `CaptureSourceMapping`; router v1 = constraints + ordered preference + route receipt, no scoring before the CF-24A `#2319` corpus | `CaptureTests`, `SourceAssetTests`, `CaptureTimelineTests`, `CaptureSourceMappingTests`, `CaptureSourceSupersessionTests`, `CaptureLegacyStateMappingTests`, `ProcessingCapabilityTests` (Domain); `ProcessorManifestValidatorTests`, `WorkerProtocolSerializationTests`, `CaptureServiceDualWriteTests`, `LlmQueueServiceDualWriteTests`, `CaptureBackfillServiceTests`, `CaptureServiceReadSwitchTests`, `CaptureIntakeSourceTests` (Application); `ContextFabricCaptureBackfillTests` (Api — golden path over a seeded legacy queue), `MigrationBootstrapTests` (tables present + store round-trips); `CaptureIntakeSingleWriterTests` (Architecture); `py -3 -B scripts/context_fabric/check_contract_drafts.py` (contract drafts); tracker CF-00 `#2254` | -| Transcript persistence and evidence | `backend/src/Taskdeck.Domain/Entities/Transcript.cs`, `backend/src/Taskdeck.Application/Interfaces/ITranscriptRepository.cs`, `backend/src/Taskdeck.Infrastructure/Repositories/TranscriptRepository.cs`, migration `backend/src/Taskdeck.Infrastructure/Migrations/20260801173142_AddTranscripts.cs` | `Transcript.Text` is an independent normalized text record; the current capture path also persists transcript input in `LlmRequest.Payload` until `#1305` links triage to Transcript. Transcript reads/deletion/export are user-scoped; optional board/artefact deletion nulls references rather than deleting the transcript; `#1305` also owns evidence spans, provenance API, and Paper deep links | the three Transcript checkpoint commands in `docs/TESTING_GUIDE.md` | +| Transcript persistence and evidence | `backend/src/Taskdeck.Domain/Entities/Transcript.cs`, `backend/src/Taskdeck.Application/Interfaces/ITranscriptRepository.cs`, `backend/src/Taskdeck.Infrastructure/Repositories/TranscriptRepository.cs`, migration `backend/src/Taskdeck.Infrastructure/Migrations/20260801173142_AddTranscripts.cs` | `Transcript.Text` is an independent normalized text record; the current capture path also persists transcript input in `LlmRequest.Payload`. `#1305` closed 2026-08-27 (PR `#2144`): evidence spans are live and the never-mapped RFAI-02 intent-envelope vocabulary was deleted, so do not look for it. Transcript reads/deletion/export are user-scoped; optional board/artefact deletion nulls references rather than deleting the transcript; `#1305` also owns evidence spans, provenance API, and Paper deep links | the three Transcript checkpoint commands in `docs/TESTING_GUIDE.md` | | Proposal operation vocabulary | [`autodoc/interfaces/proposal-operation-vocabulary.md`](interfaces/proposal-operation-vocabulary.md), `ProposalOperationContractValidator`, `OperationHandlerRegistry`, `AutomationProposalService.GetProposalDiffAsync` | board-scoped preview/apply validation, card metadata handlers, chat executors, and `Taskdeck.Api/Mcp/WriteTools` | pipeline handler, proposal diff/revision, MCP/write-tool, and proposal API tests | | Backend API/application | `backend/Taskdeck.sln`, `Api/`, `Application/`; DI at `Infrastructure/DependencyInjection.cs` | Domain has no infra/framework refs; Application no Api/Infra refs (Architecture.Tests); claims-first identity; stable HTTP 400/401/403/404/409; no cross-user leak | `dotnet test backend/Taskdeck.sln -c Release -m:1` (see `backend/CLAUDE.md`) | -| Frontend workspace | `frontend/taskdeck-web/src/`: `router`, `views`, `store/board*`, `composables/`, `api/http.ts`, `components/ui` (17 `Td*`) | Review-first UI gating; per-board SignalR (`useBoardRealtime.ts`), not global; `boardStore` is a facade over `store/board/*`; all HTTP through `api/http.ts` | `npm run typecheck`, `npm run build`, `npx vitest --run` (OOM-prone: `--maxWorkers=2`/targeted), Playwright (see `frontend/taskdeck-web/CLAUDE.md`) | +| Frontend workspace | `frontend/taskdeck-web/src/`: `router`, `views`, `store/board*`, `composables/`, `api/http.ts`, `components/ui` (18 `Td*`) | Review-first UI gating; per-board SignalR (`useBoardRealtime.ts`), not global; `boardStore` is a facade over `store/board/*`; all HTTP through `api/http.ts` | `npm run typecheck`, `npm run build`, `npx vitest --run` (OOM-prone: `--maxWorkers=2`/targeted), Playwright (see `frontend/taskdeck-web/CLAUDE.md`). **`vitest --run` is not full coverage:** `vitest.config.ts:15` excludes `tests/pwa-generated-worker.spec.ts`, so a `vite.config.ts` workbox or `src/pwa/**` change is unproven until you also run `npm run test:pwa-generated-worker` (it builds first). See the PWA row. | +| PWA / offline / cache boundary | `frontend/taskdeck-web/vite.config.ts` (the `runtimeCaching` block), `frontend/taskdeck-web/src/pwa/` (`runtimeCachePolicy.ts`, `legacyApiCache.ts`, `legacyApiCacheWorker.ts`, `staleBundleRecovery.ts`), `frontend/taskdeck-web/public/api-cache-cleanup.js`, `backend/src/Taskdeck.Api/Middleware/ApiCacheControlMiddleware.cs`; behaviour contract `docs/platform/PWA_OFFLINE_BEHAVIOR.md` | **No runtime cache may ever admit an API response** (`#2350`, PR `#2381`): the service worker is network-only for the API and `ApiCacheControlMiddleware` stamps `no-store, private` server-side, so neither the worker nor the browser cache holds identity-bound data. Only two Workbox `runtimeCaching` handlers exist - `StaleWhileRevalidate` for lazy `it`/`es` locale chunks and `CacheFirst` for assets under `/assets/` and `/icons/`; the static route is anchored on the emitted directories, not on file extension, because `VITE_API_BASE_URL` may be prefixed and an authenticated `/taskdeck/api/users/by-username/alice.png` would otherwise land in the shared cross-identity `taskdeck-static-assets` cache. **The boundary is directory-anchored, not absolute:** the deny test matches only a path *starting* with `/api`, while the admit test accepts `/assets/` or `/icons/` followed by ANY nested path ending in a media or font extension. An API base nested under those two directories is therefore still admitted - `/assets/api/users/by-username/alice.png` is cached (tracked as `#2411`). It is **not origin-anchored** either: the predicates test `url.pathname` only and `cacheableResponse` is `{ statuses: [0, 200] }`, so an opaque third-party response under a matching path is admitted. Do not deploy an API base under `/assets/` or `/icons/`, and do not restate this invariant as unconditional. A pre-`#2350` installation still runs a NetworkFirst worker that repopulates the authenticated cache, so retirement is not left to the `registerType: 'prompt'` update banner: the page probes the controlling worker over a `MessageChannel`, treats silence as pre-`#2350`, then follows the replacement through `updatefound`/`statechange` to `installed` before messaging skip-waiting, because `registration.update()` resolves inside Install so a read taken *at the moment it resolves* finds `waiting` still null. The separate up-front `registration.waiting` read at `legacyApiCacheWorker.ts:191` is deliberate, covering a replacement already waiting because the user dismissed the update banner - **do not delete it**. The whole migration is **fail-closed** and gates identity: failure clears credentials (`router/index.ts:390`) and blocks session establishment (`sessionStore.ts:71-73,188-191`), bounded by `RETIREMENT_DEADLINE_MS = 12_000` | **`src/pwa/runtimeCachePolicy.ts` ships nothing** - Workbox serializes the `urlPattern` callbacks out of `vite.config.ts`, so the inlined copies there are what reaches `sw.js`, and a policy-module-only edit changes no shipped behaviour while its own spec stays green. `tests/runtime-cache-policy-parity.spec.ts` pins the two copies together; run it for any predicate change. Then `npx vitest --run --maxWorkers=2 src/tests/pwa/` and `src/tests/router/startupCacheBoundary.spec.ts`; **`npm run test:pwa-generated-worker`** for the generated worker, which `vitest.config.ts:15` excludes from the default run; `ApiCacheControlApiTests`; E2E `tests/e2e/pwa-api-cache.spec.ts` | | Agent runtime & MCP | `Application` (`AutomationPolicyEngine`), **MCP surface in `Api`** (`Program.cs` `--mcp` branch, `Api/Mcp/*`), `.codex/config.toml`, `.mcp.json`, `docs/MCP_TOOLING_GUIDE.md` | Policy evaluated before execute; egress/telemetry guards; tool registry | security tests, MCP inventory/egress tests | -| Agent tooling / CI / docs | `.claude/`, `.codex/`, `scripts/github/Invoke-TaskdeckReadOnlyInventory.ps1`, `scripts/agent_hooks/` (manual ledger projection only), `.github/workflows/` (`ci-required.yml` = the required CI evidence; `smart-ci-shadow.yml` = the shadow planner + observation-mode gate, landing with CI-02 `#2326`), `ci/policy.v1.json` + `scripts/ci/smart-ci/` (planner and gate evaluator from CI-02; estate measurement `measure-ci-estate.mjs` shipped with ADR-0066; map `docs/ci/SMART_CI.md`, tracker CI-00 `#2324`), `scripts/check-*.mjs` | Delegated shell-backed inventory enters through the opt-in read-only argv wrapper; direct Git/GitHub mutation stays coordinator-owned; review and merge disposition come from live authority plus the canonical global pipeline; no Taskdeck-owned runtime hooks or local command-deny list; Smart CI is in **shadow mode** — the planner and gate change no job selection until the recall report (CI-02 `#2326`) and the gate is registered only by the maintainer (CI-03 `#2327`); CI-control paths (`.github/**`, `ci/**`, `scripts/ci/**`) are R4/T2 and qualify hosted-only, never on a self-hosted runner; the repository goes private for v0.3.0 by maintainer action only (CI-13 `#2337`) and no self-hosted runner is attached while it is public | `powershell -NoProfile -ExecutionPolicy Bypass -File scripts/github/Invoke-TaskdeckReadOnlyInventory.ps1 -SelfTest`; failure-ledger synchronization unittest, settings/tier parsing, worktree helper suite when touched, then docs gates (see `scripts/agent_hooks/CLAUDE.md`); `node --test scripts/ci/smart-ci/*.test.mjs` when `ci/**` or `scripts/ci/smart-ci/**` change | +| Agent tooling / CI / docs | `.claude/`, `.codex/`, `scripts/github/Invoke-TaskdeckReadOnlyInventory.ps1`, `scripts/agent_hooks/` (manual ledger projection only), `.github/workflows/` (`ci-required.yml` = the required CI evidence; `smart-ci-shadow.yml` = the shadow planner + observation-mode gate, **landed** and now running `Smart CI / Plan`, `Smart CI / Planner Self-Test` and `Smart CI / Required Gate` on every PR), `ci/policy.v1.json` + `scripts/ci/smart-ci/` (planner `plan.mjs` and gate evaluator `evaluate-gate.mjs` from CI-02, plus the CLIs `measure-ci-estate.mjs`, `recall-report.mjs`, `action-pins.mjs`, `artifact-cleanup.mjs` and `resolve-merge-ref.mjs` (`#2401`, PR `#2404`); map `docs/ci/SMART_CI.md`, tracker CI-00 `#2324`), `scripts/check-*.mjs` | Delegated shell-backed inventory enters through the opt-in read-only argv wrapper; direct Git/GitHub mutation stays coordinator-owned; review and merge disposition come from live authority plus the canonical global pipeline; no Taskdeck-owned runtime hooks or local command-deny list; Smart CI is in **shadow mode** — the planner and gate change no job selection until the recall report (CI-02 `#2326`) and the gate is registered only by the maintainer (CI-03 `#2327`); CI-control paths (`.github/**`, `ci/**`, `scripts/ci/**`) are R4/T2 and qualify hosted-only, never on a self-hosted runner; the repository goes private for v0.3.0 by maintainer action only (CI-13 `#2337`) and no self-hosted runner is attached while it is public | `powershell -NoProfile -ExecutionPolicy Bypass -File scripts/github/Invoke-TaskdeckReadOnlyInventory.ps1 -SelfTest`; failure-ledger synchronization unittest, settings/tier parsing, worktree helper suite when touched, then docs gates (see `scripts/agent_hooks/CLAUDE.md`); `node --test scripts/ci/smart-ci/*.test.mjs` when `ci/**` or `scripts/ci/smart-ci/**` change | | Docs & planning | `docs/STATUS.md`, `docs/IMPLEMENTATION_MASTERPLAN.md`, `docs/ISSUE_EXECUTION_GUIDE.md`, `docs/TESTING_GUIDE.md` | STATUS is source of truth for shipped reality; keep governance line intact | `node scripts/check-docs-governance.mjs`, `node scripts/check-golden-principles.mjs` | ## Interface-On-Top Convention diff --git a/backend/src/Taskdeck.Application/Services/CaptureBackfillService.cs b/backend/src/Taskdeck.Application/Services/CaptureBackfillService.cs index 1efa591f0..0c487c8b0 100644 --- a/backend/src/Taskdeck.Application/Services/CaptureBackfillService.cs +++ b/backend/src/Taskdeck.Application/Services/CaptureBackfillService.cs @@ -275,8 +275,19 @@ private async Task ReconcileAsync( { var legacyState = ResolveLegacyState(request, payload); - // An archived capture is terminal: it rejects edits and projections alike, and the queue row - // has nothing left to teach it beyond the stamp. + // Archived is terminal for disposition, not evidence that source text agrees. The aggregate + // cannot accept a superseding asset once archived, so leave a mismatch outstanding and let + // the run's normal skip path retain queue-row fallback instead of stamping stale text away. + if (capture.Disposition == CaptureUserDisposition.Archived && + !string.Equals(capture.CurrentText, payload.Text, StringComparison.Ordinal)) + { + throw new DomainException( + ErrorCodes.ValidationError, + "Cannot reconcile source text on an archived capture"); + } + + // An archived capture whose text already agrees rejects the remaining projections; only its + // queue reconciliation stamp may move forward. if (capture.Disposition != CaptureUserDisposition.Archived) { if (!string.IsNullOrWhiteSpace(payload.Text) && diff --git a/backend/src/Taskdeck.Application/Services/CaptureService.cs b/backend/src/Taskdeck.Application/Services/CaptureService.cs index 4b6c8290b..18200e4df 100644 --- a/backend/src/Taskdeck.Application/Services/CaptureService.cs +++ b/backend/src/Taskdeck.Application/Services/CaptureService.cs @@ -890,12 +890,7 @@ public async Task> UpdateSuggestionAsync( { // The durable side must never be the reason an operation the queue row accepted fails. // The aggregate is now behind its queue row. The divergence guard on the read path - // detects that and the reconcile pass repairs it on the next start -- PROVIDED no - // disposition change intervenes first. Keep, Archive and Reactivate all Touch the - // aggregate, which stamps it newer than the queue row that moved past it and defeats - // both the read guard and the divergence join, masking the divergence indefinitely. - // Tracked as #2347; until that lands, a divergence followed by a disposition write stays - // hidden. + // detects that and the reconcile pass repairs it on the next start. _logger?.LogWarning( ex, "Context Fabric: could not record a superseding source for capture {CaptureId}; " + @@ -923,26 +918,74 @@ private async Task CancelInternalAsync( if (item.UserId != userId) return Result.Failure(ErrorCodes.Forbidden, "You do not have permission to modify this capture item"); - if (item.Status == RequestStatus.Cancelled) - return Result.Success(); - + var transactionOpen = false; try { + // Cancellation changes both the queue row and the durable aggregate. Use the same + // optimistic queue guard as Keep/Archive so a stale request cannot project an older + // payload into a capture that a concurrent edit has already corrected. + await _unitOfWork.BeginTransactionAsync(cancellationToken); + transactionOpen = true; + + var expectedStatus = item.Status; + var expectedUpdatedAt = item.UpdatedAt; item.Cancel(); + var updated = await _unitOfWork.LlmQueue.TrySetCaptureDispositionAsync( + item.Id, + expectedStatus, + expectedUpdatedAt, + item.Status, + item.Payload, + cancellationToken); + if (!updated) + { + await _unitOfWork.RollbackTransactionAsync(cancellationToken); + transactionOpen = false; + return Result.Failure( + ErrorCodes.Conflict, + "Capture item changed while it was being cancelled"); + } + + var persistedPayload = ParsePayload(item); // The user's disposition is a durable column now, not JSON on the queue row: putting a // capture away records Archived on the aggregate's disposition axis in the same unit of // work. Processing and action outcomes are deliberately left standing -- archiving is a // decision about the Inbox, not an erasure of what was produced (ADR-0065 Decision 1). - await ApplyDurableDispositionAsync(userId, item.Id, CaptureDisposition.Archived, cancellationToken); + var durable = await ApplyDurableDispositionAsync( + userId, + item.Id, + CaptureDisposition.Archived, + persistedPayload.Text, + item.UpdatedAt, + cancellationToken); + if (durable is not null) + { + await _unitOfWork.SaveChangesAsync(cancellationToken); + } - await _unitOfWork.SaveChangesAsync(cancellationToken); + await _unitOfWork.CommitTransactionAsync(cancellationToken); + transactionOpen = false; return Result.Success(); } catch (DomainException ex) { + if (transactionOpen) + { + await _unitOfWork.RollbackTransactionAsync(cancellationToken); + } + return Result.Failure(ex.ErrorCode, ex.Message); } + catch + { + if (transactionOpen) + { + await _unitOfWork.RollbackTransactionAsync(cancellationToken); + } + + throw; + } } /// @@ -954,6 +997,8 @@ private async Task CancelInternalAsync( Guid userId, Guid captureId, CaptureDisposition disposition, + string queueText, + DateTimeOffset queueUpdatedAt, CancellationToken cancellationToken) { // Not gated on DualWriteCaptures, for the same reason as SupersedeDurableTextAsync: the flag @@ -971,6 +1016,16 @@ private async Task CancelInternalAsync( try { + // CF-01c (#2347): this is the exact queue text the disposition CAS wrote. Repair any + // out-of-band divergence before Keep/Archive/Reactivate touches UpdatedAt; applying the + // disposition first would make stale durable text look newer than its queue row and + // hide it from both the read guard and reconcile backlog forever. Sources stay + // immutable: reconciliation appends a superseding asset. + if (!string.Equals(capture.CurrentText, queueText, StringComparison.Ordinal)) + { + capture.SupersedeInlineTextSource(queueText); + } + switch (CaptureUserDispositionMapping.FromLegacy(disposition)) { case CaptureUserDisposition.Archived: @@ -983,6 +1038,8 @@ private async Task CancelInternalAsync( capture.Reactivate(); break; } + + capture.RecordLegacyReconciliation(queueUpdatedAt); } catch (DomainException ex) { @@ -1019,43 +1076,29 @@ private async Task> SetDispositionAsync( var payload = ParsePayload(item); var status = ResolveCaptureStatus(item, payload); - - if (payload.Disposition?.Kind == disposition && + var isIdempotentDisposition = payload.Disposition?.Kind == disposition && (status is CaptureStatus.New or CaptureStatus.Failed || - disposition == CaptureDisposition.Archived && status == CaptureStatus.Ignored)) - { - // The idempotent return still repairs the aggregate. The queue row already carries this - // disposition, so a durable row that does not is the residue of an interrupted attempt -- - // and this early exit is the path every retry takes, so it has to be the path that heals. - var repaired = await ApplyDurableDispositionAsync(userId, item.Id, disposition, cancellationToken); - if (repaired is not null) - { - await _unitOfWork.SaveChangesAsync(cancellationToken); - } - - return Result.Success(MapToDetailDto( - item, - payload, - effectiveBoardId: null, - await ReadableMaterialAsync(repaired, cancellationToken))); - } + disposition == CaptureDisposition.Archived && status == CaptureStatus.Ignored); - if (status is not CaptureStatus.New and not CaptureStatus.Failed) + if (!isIdempotentDisposition && status is not CaptureStatus.New and not CaptureStatus.Failed) { return Result.Failure( ErrorCodes.Conflict, $"Capture item cannot be {disposition.ToString().ToLowerInvariant()} from {status}"); } - var existingProposal = await _unitOfWork.AutomationProposals.GetBySourceReferenceAsync( - ProposalSourceType.Queue, - item.Id.ToString(), - cancellationToken); - if (existingProposal?.Status is ProposalStatus.PendingReview or ProposalStatus.Approved or ProposalStatus.Applied) + if (!isIdempotentDisposition) { - return Result.Failure( - ErrorCodes.Conflict, - "Capture item already has a proposal in review or applied work"); + var existingProposal = await _unitOfWork.AutomationProposals.GetBySourceReferenceAsync( + ProposalSourceType.Queue, + item.Id.ToString(), + cancellationToken); + if (existingProposal?.Status is ProposalStatus.PendingReview or ProposalStatus.Approved or ProposalStatus.Applied) + { + return Result.Failure( + ErrorCodes.Conflict, + "Capture item already has a proposal in review or applied work"); + } } var transactionOpen = false; @@ -1069,14 +1112,16 @@ private async Task> SetDispositionAsync( var expectedStatus = item.Status; var expectedUpdatedAt = item.UpdatedAt; - var updatedPayload = payload with - { - Disposition = new CaptureDispositionV1( - disposition, - DateTimeOffset.UtcNow, - userId, - item.BoardId) - }; + var updatedPayload = isIdempotentDisposition + ? payload + : payload with + { + Disposition = new CaptureDispositionV1( + disposition, + DateTimeOffset.UtcNow, + userId, + item.BoardId) + }; var targetStatus = disposition == CaptureDisposition.Archived ? RequestStatus.Cancelled : item.Status; @@ -1101,9 +1146,17 @@ private async Task> SetDispositionAsync( "Capture item changed while its disposition was being recorded"); } + var persistedPayload = ParsePayload(item); + // Only after the conditional queue-row update actually won: a lost race must not leave // the durable disposition axis ahead of the row it describes. - var durable = await ApplyDurableDispositionAsync(userId, item.Id, disposition, cancellationToken); + var durable = await ApplyDurableDispositionAsync( + userId, + item.Id, + disposition, + persistedPayload.Text, + item.UpdatedAt, + cancellationToken); if (durable is not null) { await _unitOfWork.SaveChangesAsync(cancellationToken); @@ -1114,7 +1167,7 @@ private async Task> SetDispositionAsync( return Result.Success(MapToDetailDto( item, - updatedPayload, + persistedPayload, effectiveBoardId: null, await ReadableMaterialAsync(durable, cancellationToken))); } diff --git a/backend/src/Taskdeck.Application/Services/ContextFabricSettings.cs b/backend/src/Taskdeck.Application/Services/ContextFabricSettings.cs index a7764483f..8a8768b0c 100644 --- a/backend/src/Taskdeck.Application/Services/ContextFabricSettings.cs +++ b/backend/src/Taskdeck.Application/Services/ContextFabricSettings.cs @@ -21,11 +21,7 @@ public sealed class ContextFabricSettings /// durable write that fails is not retried inline. Re-enabling therefore does not simply resume: /// the next pass has to bring the window's captures in and /// reconcile anything that drifted, and until it has, the read path serves whichever of the two - /// wrote last. Nothing a user sees goes backwards at any point in that sequence - provided no - /// disposition change (keep, archive, reactivate) intervenes before the next reconcile pass. - /// Such a write stamps the aggregate newer than the queue row that moved past it, which defeats - /// both the divergence guard and the reconcile pass and masks the divergence until - /// #2347 lands. + /// wrote last. Nothing a user sees goes backwards at any point in that sequence. /// /// public bool DualWriteCaptures { get; set; } = true; diff --git a/backend/src/Taskdeck.Infrastructure/Repositories/EfCaptureBackfillStore.cs b/backend/src/Taskdeck.Infrastructure/Repositories/EfCaptureBackfillStore.cs index 4c563af6d..20348200a 100644 --- a/backend/src/Taskdeck.Infrastructure/Repositories/EfCaptureBackfillStore.cs +++ b/backend/src/Taskdeck.Infrastructure/Repositories/EfCaptureBackfillStore.cs @@ -1,3 +1,4 @@ +using System.Text.Json; using Microsoft.EntityFrameworkCore; using Taskdeck.Application.Interfaces; using Taskdeck.Domain.Entities; @@ -58,10 +59,12 @@ public async Task> GetLegacyCaptureBacklogAsync( // gives the Inbox listing. The NOT EXISTS clause is the divergence join: a row leaves the // backlog only once a capture exists for it AND that capture is at least as fresh as the // queue row. Oldest first, so the backlog drains in intake order. - // Rows this run has already failed on are excluded here rather than filtered afterwards, - // so a poisoned head cannot consume the whole batch on every iteration. - // FromSqlInterpolated parameterises every hole; the exclusion list is bounded by the - // number of distinct failures in one run, so it is fetched generously and trimmed below. + // Rows this run has already failed on are excluded by SQLite before LIMIT, so a poisoned + // head cannot consume the whole batch and no more than batchSize payloads are materialized. + // json_each carries every id in one collection parameter rather than one parameter per id, + // so a large poisoned set cannot hit SQLite's bound-variable ceiling. + var excludedJson = JsonSerializer.Serialize( + excluded.Select(id => id.ToString("D").ToUpperInvariant())); FormattableString sql = $""" SELECT * FROM LlmRequests @@ -70,19 +73,14 @@ AND NOT EXISTS ( SELECT 1 FROM Captures WHERE Captures.Id = LlmRequests.Id AND Captures.UpdatedAt >= LlmRequests.UpdatedAt) + AND Id NOT IN (SELECT value FROM json_each({excludedJson})) ORDER BY CreatedAt, Id - LIMIT {batchSize + excluded.Count} + LIMIT {batchSize} """; - var rows = await _context.LlmRequests + return await _context.LlmRequests .FromSqlInterpolated(sql) .AsNoTracking() .ToListAsync(cancellationToken); - return rows - .Where(request => !excluded.Contains(request.Id)) - .OrderBy(request => request.CreatedAt) - .ThenBy(request => request.Id.ToString(), StringComparer.Ordinal) - .Take(batchSize) - .ToList(); } var query = Backlog; diff --git a/backend/tests/Taskdeck.Api.Tests/CaptureBackfillStoreIntegrationTests.cs b/backend/tests/Taskdeck.Api.Tests/CaptureBackfillStoreIntegrationTests.cs new file mode 100644 index 000000000..0bbda02c6 --- /dev/null +++ b/backend/tests/Taskdeck.Api.Tests/CaptureBackfillStoreIntegrationTests.cs @@ -0,0 +1,160 @@ +using System.Collections.Concurrent; +using System.Data.Common; +using FluentAssertions; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Diagnostics; +using Taskdeck.Application.DTOs; +using Taskdeck.Domain.Common; +using Taskdeck.Domain.Entities; +using Taskdeck.Domain.Enums; +using Taskdeck.Infrastructure.Persistence; +using Taskdeck.Infrastructure.Repositories; +using Xunit; + +namespace Taskdeck.Api.Tests; + +/// Real-SQL regressions for the bounded CF-01 reconcile backlog (#2347). +public sealed class CaptureBackfillStoreIntegrationTests +{ + [Fact] + public async Task GetLegacyCaptureBacklogAsync_ShouldExcludeServerSideAndLimitMaterializationToBatchSize() + { + var dbPath = Path.Combine(Path.GetTempPath(), $"taskdeck-capture-backfill-{Guid.NewGuid():N}.db"); + var interceptor = new CapturingReaderInterceptor(); + var options = new DbContextOptionsBuilder() + .UseSqlite(TestSqlite.ConnectionString(dbPath)) + .AddInterceptors(interceptor) + .Options; + + try + { + await using var db = new TaskdeckDbContext(options); + await db.Database.MigrateAsync(); + var user = new User("capture-backfill-bound", "capture-backfill-bound@example.com", "hash"); + db.Users.Add(user); + + var oldestExcluded = AddQueueRow(db, user.Id, "excluded oldest", new DateTimeOffset(2026, 9, 1, 0, 0, 0, TimeSpan.Zero)); + var nextExcluded = AddQueueRow(db, user.Id, "excluded next", new DateTimeOffset(2026, 9, 1, 0, 1, 0, TimeSpan.Zero)); + var healthy = AddQueueRow(db, user.Id, "healthy", new DateTimeOffset(2026, 9, 1, 0, 2, 0, TimeSpan.Zero)); + await db.SaveChangesAsync(); + + // More than SQLite's traditional variable ceiling. A parameter-per-id implementation + // would fail, while one json_each collection parameter stays bounded. + var excluded = Enumerable.Range(0, 1_100) + .Select(_ => Guid.NewGuid()) + .Append(oldestExcluded.Id) + .Append(nextExcluded.Id) + .ToHashSet(); + interceptor.Clear(); + + var rows = await new EfCaptureBackfillStore(db) + .GetLegacyCaptureBacklogAsync(batchSize: 1, excluded); + + rows.Should().ContainSingle().Which.Id.Should().Be(healthy.Id, + "excluded head rows must not consume the database limit"); + var command = interceptor.Commands.Single( + captured => captured.Text.Contains("FROM LlmRequests", StringComparison.OrdinalIgnoreCase)); + command.Parameters.Should().HaveCount(3, + "request type, the whole excluded-id JSON array and batch size are the only parameters"); + command.Text.Should().ContainEquivalentOf("NOT IN (SELECT value FROM json_each(", + "exclusions must be evaluated by SQLite before materialization"); + + var exclusionParameter = command.Parameters.Single(parameter => + parameter.Value is string value && value.StartsWith("[", StringComparison.Ordinal)); + exclusionParameter.Value.Should().BeOfType() + .Which.Should().Contain(oldestExcluded.Id.ToString("D").ToUpperInvariant()); + + var limitParameter = command.Parameters.Single(parameter => parameter.Value is int value && value == 1); + var limitToken = limitParameter.Name.StartsWith('@') + ? limitParameter.Name + : $"@{limitParameter.Name}"; + command.Text.Should().Contain($"LIMIT {limitToken}", + "the SQL limit must be exactly batchSize, independent of excluded count"); + command.Parameters + .Where(parameter => parameter.Value is int value && value == excluded.Count + 1) + .Should().BeEmpty(); + } + finally + { + Cleanup(dbPath); + } + } + + private static LlmRequest AddQueueRow( + TaskdeckDbContext db, + Guid userId, + string text, + DateTimeOffset createdAt) + { + var payload = new CapturePayloadV1( + CaptureRequestContract.CurrentSchemaVersion, + CaptureSource.Typed, + text); + var request = new LlmRequest( + userId, + CaptureRequestContract.RequestTypeV1, + CaptureRequestContract.SerializePayload(payload)); + typeof(Entity).GetProperty(nameof(Entity.CreatedAt))!.SetValue(request, createdAt); + typeof(Entity).GetProperty(nameof(Entity.UpdatedAt))!.SetValue(request, createdAt); + db.LlmRequests.Add(request); + return request; + } + + private static void Cleanup(string dbPath) + { + foreach (var suffix in new[] { "", "-wal", "-shm", "-journal" }) + { + var path = dbPath + suffix; + if (!File.Exists(path)) + { + continue; + } + + try { File.Delete(path); } + catch (IOException) { /* best-effort temp cleanup */ } + } + } + + private sealed record CapturedCommand( + string Text, + IReadOnlyList Parameters); + + private sealed record CapturedParameter(string Name, object Value); + + private sealed class CapturingReaderInterceptor : DbCommandInterceptor + { + private readonly ConcurrentQueue _commands = new(); + + public IReadOnlyCollection Commands => _commands; + + public void Clear() => _commands.Clear(); + + public override InterceptionResult ReaderExecuting( + DbCommand command, + CommandEventData eventData, + InterceptionResult result) + { + Capture(command); + return base.ReaderExecuting(command, eventData, result); + } + + public override ValueTask> ReaderExecutingAsync( + DbCommand command, + CommandEventData eventData, + InterceptionResult result, + CancellationToken cancellationToken = default) + { + Capture(command); + return base.ReaderExecutingAsync(command, eventData, result, cancellationToken); + } + + private void Capture(DbCommand command) => + _commands.Enqueue(new CapturedCommand( + command.CommandText, + command.Parameters.Cast() + .Select(parameter => new CapturedParameter( + parameter.ParameterName, + parameter.Value ?? DBNull.Value)) + .ToList())); + } +} diff --git a/backend/tests/Taskdeck.Api.Tests/ContextFabricCaptureBackfillTests.cs b/backend/tests/Taskdeck.Api.Tests/ContextFabricCaptureBackfillTests.cs index 37e341ecc..b79d49f90 100644 --- a/backend/tests/Taskdeck.Api.Tests/ContextFabricCaptureBackfillTests.cs +++ b/backend/tests/Taskdeck.Api.Tests/ContextFabricCaptureBackfillTests.cs @@ -375,6 +375,116 @@ public async Task ADivergedAggregate_ShouldBeRepairedByTheReconcilePassAndNeverS second.Remaining.Should().Be(0); } + [Theory] + [InlineData("keep", CaptureUserDisposition.Kept)] + [InlineData("archive", CaptureUserDisposition.Archived)] + public async Task Disposition_ShouldPreserveAQueueTextCorrectionThroughResponseReadAndBackfill( + string action, + CaptureUserDisposition expectedDisposition) + { + // CF-01c (#2347) verifier repro against the real queue CAS and SQLite stores. + var user = await ApiTestHarness.AuthenticateAsync(_client, $"cf01-disposition-{action}"); + var created = (await (await _client.PostAsJsonAsync( + "/api/capture/items", + new CreateCaptureItemDto(null, "first draft", "typed"))) + .Content.ReadFromJsonAsync())!; + await RunBackfillAsync(); + + using (var scope = _factory.Services.CreateScope()) + { + var db = scope.ServiceProvider.GetRequiredService(); + var request = await db.LlmRequests.SingleAsync(row => row.Id == created.Id); + var payload = CaptureRequestContract.ParseStoredPayload(request.Payload); + request.UpdatePayload(CaptureRequestContract.SerializePayload(payload with { Text = "corrected draft" })); + await db.SaveChangesAsync(); + } + + var whileDiverged = await _client.GetFromJsonAsync($"/api/capture/items/{created.Id}"); + whileDiverged!.RawText.Should().Be("corrected draft", "the queue row is the newer writer before disposition"); + + var response = await _client.PostAsync($"/api/capture/items/{created.Id}/{action}", null); + response.StatusCode.Should().Be(HttpStatusCode.OK); + var mutated = await response.Content.ReadFromJsonAsync(); + mutated!.RawText.Should().Be("corrected draft"); + + var reread = await _client.GetFromJsonAsync($"/api/capture/items/{created.Id}"); + reread!.RawText.Should().Be("corrected draft"); + + var backfill = await RunBackfillAsync(); + backfill.Remaining.Should().Be(0, "the disposition repair must leave no masked divergence"); + + using var inspected = _factory.Services.CreateScope(); + var store = inspected.ServiceProvider.GetRequiredService(); + var capture = (await store.GetByIdForUserAsync(created.Id, user.UserId))!; + capture.CurrentText.Should().Be("corrected draft"); + capture.Disposition.Should().Be(expectedDisposition); + capture.SourceAssets.Should().HaveCount(2, "the original source remains as immutable lineage"); + capture.SourceAssets[0].TextPayload!.Text.Should().Be("first draft"); + capture.SourceAssets[0].IsActive.Should().BeFalse(); + capture.SourceAssets[1].SupersedesAssetId.Should().Be(capture.SourceAssets[0].Id); + } + + [Fact] + public async Task Backfill_ShouldKeepAnArchivedTextMismatchReadableAndOutstandingWithoutStampingIt() + { + var user = await ApiTestHarness.AuthenticateAsync(_client, "cf01-archived-divergence"); + Guid captureId = Guid.Empty; + try + { + var created = (await (await _client.PostAsJsonAsync( + "/api/capture/items", + new CreateCaptureItemDto(null, "first draft", "typed"))) + .Content.ReadFromJsonAsync())!; + captureId = created.Id; + await RunBackfillAsync(); + + var archive = await _client.PostAsync($"/api/capture/items/{captureId}/archive", null); + archive.StatusCode.Should().Be(HttpStatusCode.OK); + + DateTimeOffset archivedStamp; + using (var before = _factory.Services.CreateScope()) + { + var store = before.ServiceProvider.GetRequiredService(); + archivedStamp = (await store.GetByIdForUserAsync(captureId, user.UserId))!.UpdatedAt; + } + + using (var scope = _factory.Services.CreateScope()) + { + var db = scope.ServiceProvider.GetRequiredService(); + var request = await db.LlmRequests.SingleAsync(row => row.Id == captureId); + var payload = CaptureRequestContract.ParseStoredPayload(request.Payload); + request.UpdatePayload(CaptureRequestContract.SerializePayload(payload with { Text = "corrected draft" })); + await db.SaveChangesAsync(); + } + + var result = await RunBackfillAsync(); + + result.Skipped.Should().BeGreaterThan(0); + result.Remaining.Should().BeGreaterThan(0); + result.Complete.Should().BeFalse(); + var reread = await _client.GetFromJsonAsync($"/api/capture/items/{captureId}"); + reread!.RawText.Should().Be("corrected draft", "the queue fallback owns the unrepaired mismatch"); + + using var inspected = _factory.Services.CreateScope(); + var inspectedStore = inspected.ServiceProvider.GetRequiredService(); + var capture = (await inspectedStore.GetByIdForUserAsync(captureId, user.UserId))!; + capture.CurrentText.Should().Be("first draft"); + capture.UpdatedAt.Should().Be(archivedStamp, "the failed reconciliation earned no stamp"); + capture.SourceAssets.Should().ContainSingle(); + } + finally + { + if (captureId != Guid.Empty) + { + // The intentionally outstanding row must not poison this shared fixture's later tests. + using var cleanup = _factory.Services.CreateScope(); + var db = cleanup.ServiceProvider.GetRequiredService(); + await db.Captures.Where(capture => capture.Id == captureId).ExecuteDeleteAsync(); + await db.LlmRequests.Where(row => row.Id == captureId).ExecuteDeleteAsync(); + } + } + } + [Fact] public async Task Backfill_ShouldStepOverUnmappableRowsAndReportAnHonestMarker() { diff --git a/backend/tests/Taskdeck.Application.Tests/Services/CaptureBackfillServiceTests.cs b/backend/tests/Taskdeck.Application.Tests/Services/CaptureBackfillServiceTests.cs index dd4a9090c..6c49d0e94 100644 --- a/backend/tests/Taskdeck.Application.Tests/Services/CaptureBackfillServiceTests.cs +++ b/backend/tests/Taskdeck.Application.Tests/Services/CaptureBackfillServiceTests.cs @@ -295,6 +295,33 @@ public async Task RunAsync_ShouldReconcileACaptureWhoseQueueRowWasEditedWhileDua capture.SourceAssets[1].SupersedesAssetId.Should().Be(capture.SourceAssets[0].Id); } + [Fact] + public async Task RunAsync_ShouldLeaveAnArchivedTextDivergenceOutstandingWithoutAdvancingItsStamp() + { + // CF-01c (#2347): archived is terminal for disposition, not proof that stale source text was + // repaired. If the aggregate cannot accept the correction, the row must remain in the + // backlog and readable through the queue fallback rather than receive a false stamp. + var request = SeedLegacyRow(text: "first draft"); + await CreateService().RunAsync(); + var capture = _captureStore.All.Should().ContainSingle().Subject; + capture.Archive(); + var archivedStamp = capture.UpdatedAt; + + EditQueuePayloadOnly(request, "corrected draft"); + + var result = await CreateService().RunAsync(); + + result.Migrated.Should().Be(0); + result.Reconciled.Should().Be(0); + result.Skipped.Should().Be(1); + result.Remaining.Should().Be(1, "the unrepaired row must retain queue-text fallback safety"); + result.Complete.Should().BeFalse(); + capture.CurrentText.Should().Be("first draft"); + capture.UpdatedAt.Should().Be(archivedStamp, "no reconciliation stamp was earned"); + capture.SourceAssets.Should().ContainSingle("immutable lineage must not be rewritten"); + _backfillStore.SavedState!.SkippedCount.Should().Be(1); + } + [Fact] public async Task RunAsync_ShouldReconcileATitleAndAnExternalReferenceAddedAfterTheCaptureWasBuilt() { diff --git a/backend/tests/Taskdeck.Application.Tests/Services/CaptureIntakeSourceTests.cs b/backend/tests/Taskdeck.Application.Tests/Services/CaptureIntakeSourceTests.cs index 5c3d452a3..367572095 100644 --- a/backend/tests/Taskdeck.Application.Tests/Services/CaptureIntakeSourceTests.cs +++ b/backend/tests/Taskdeck.Application.Tests/Services/CaptureIntakeSourceTests.cs @@ -178,6 +178,14 @@ public async Task CancelAsync_ShouldArchiveTheDurableCaptureWithoutErasingItsOut processingSummary: CaptureProcessingSummary.Ready, actionState: CaptureActionState.Acted); _queue.Setup(repository => repository.GetByIdAsync(request.Id, It.IsAny())).ReturnsAsync(request); + _queue.Setup(repository => repository.TrySetCaptureDispositionAsync( + request.Id, + It.IsAny(), + It.IsAny(), + RequestStatus.Cancelled, + It.IsAny(), + It.IsAny())) + .ReturnsAsync(true); _captureStore .Setup(store => store.GetByIdForUpdateAsync(request.Id, _userId, It.IsAny())) .ReturnsAsync(durable); diff --git a/backend/tests/Taskdeck.Application.Tests/Services/CaptureServiceReadSwitchTests.cs b/backend/tests/Taskdeck.Application.Tests/Services/CaptureServiceReadSwitchTests.cs index 728007f6c..fa4a470a7 100644 --- a/backend/tests/Taskdeck.Application.Tests/Services/CaptureServiceReadSwitchTests.cs +++ b/backend/tests/Taskdeck.Application.Tests/Services/CaptureServiceReadSwitchTests.cs @@ -296,6 +296,244 @@ public async Task KeepAsync_ShouldStillRecordTheDurableDispositionWhenTheReadFla _unitOfWork.Verify(unit => unit.CommitTransactionAsync(It.IsAny()), Times.Once); } + [Theory] + [InlineData(CaptureDisposition.Kept)] + [InlineData(CaptureDisposition.Archived)] + public async Task SetDispositionAsync_ShouldReconcileQueueTextBeforeStampingTheDurableCapture( + CaptureDisposition disposition) + { + // CF-01c (#2347): the queue row can be newer after an out-of-band edit. Keep/Archive both + // Touch the aggregate, so applying the disposition first would mask the stale durable text + // from both the read guard and the reconcile backlog forever. + var row = QueueRow("first draft"); + var durable = DurableFor(row, "first draft"); + var originalAsset = durable.SourceAssets.Should().ContainSingle().Subject; + var correctedPayload = CaptureRequestContract.ParseStoredPayload(row.Payload) with + { + Text = "corrected draft" + }; + row.UpdatePayload(CaptureRequestContract.SerializePayload(correctedPayload)); + + _queue.Setup(repository => repository.GetByIdAsync(row.Id, It.IsAny())) + .ReturnsAsync(row); + _queue.Setup(repository => repository.TrySetCaptureDispositionAsync( + row.Id, + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .ReturnsAsync(true); + _proposals + .Setup(repository => repository.GetBySourceReferenceAsync( + It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((AutomationProposal?)null); + _captureStore + .Setup(store => store.GetByIdForUpdateAsync(row.Id, _userId, It.IsAny())) + .ReturnsAsync(durable); + _captureStore + .Setup(store => store.GetByIdForUserAsync(row.Id, _userId, It.IsAny())) + .ReturnsAsync(durable); + + var service = CreateService(); + var mutation = disposition == CaptureDisposition.Kept + ? await service.KeepAsync(_userId, row.Id) + : await service.ArchiveAsync(_userId, row.Id); + var reread = await service.GetByIdAsync(_userId, row.Id); + + mutation.IsSuccess.Should().BeTrue(mutation.ErrorMessage); + mutation.Value!.RawText.Should().Be("corrected draft"); + reread.IsSuccess.Should().BeTrue(reread.ErrorMessage); + reread.Value!.RawText.Should().Be("corrected draft"); + durable.CurrentText.Should().Be("corrected draft"); + durable.Disposition.Should().Be(CaptureUserDispositionMapping.FromLegacy(disposition)); + durable.SourceAssets.Should().HaveCount(2, "the correction supersedes rather than rewriting lineage"); + originalAsset.IsActive.Should().BeFalse(); + durable.SourceAssets[1].SupersedesAssetId.Should().Be(originalAsset.Id); + } + + [Fact] + public async Task ArchiveAsync_WhenDurableTextCannotBeRepaired_ShouldKeepTheQueueFallbackAndStampUnchanged() + { + var row = QueueRow("first draft"); + var durable = DurableFor(row, "first draft"); + durable.Archive(); + var archivedStamp = durable.UpdatedAt; + var correctedPayload = CaptureRequestContract.ParseStoredPayload(row.Payload) with + { + Text = "corrected draft" + }; + row.UpdatePayload(CaptureRequestContract.SerializePayload(correctedPayload)); + + _queue.Setup(repository => repository.GetByIdAsync(row.Id, It.IsAny())) + .ReturnsAsync(row); + _queue.Setup(repository => repository.TrySetCaptureDispositionAsync( + row.Id, + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .ReturnsAsync(true); + _proposals + .Setup(repository => repository.GetBySourceReferenceAsync( + It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((AutomationProposal?)null); + _captureStore + .Setup(store => store.GetByIdForUpdateAsync(row.Id, _userId, It.IsAny())) + .ReturnsAsync(durable); + _captureStore + .Setup(store => store.GetByIdForUserAsync(row.Id, _userId, It.IsAny())) + .ReturnsAsync(durable); + + var service = CreateService(); + var mutation = await service.ArchiveAsync(_userId, row.Id); + var reread = await service.GetByIdAsync(_userId, row.Id); + + mutation.IsSuccess.Should().BeTrue(mutation.ErrorMessage); + mutation.Value!.RawText.Should().Be("corrected draft", "the accepted queue write remains readable"); + reread.Value!.RawText.Should().Be("corrected draft"); + durable.CurrentText.Should().Be("first draft"); + durable.UpdatedAt.Should().Be(archivedStamp, "a failed repair must not fabricate freshness"); + _captureStore.Verify( + store => store.UpdateAsync(It.IsAny(), It.IsAny()), + Times.Never); + } + + [Fact] + public async Task KeepAsync_IdempotentRetryThatLosesTheQueueCas_ShouldNotOverwriteNewerDurableText() + { + var row = QueueRow("stale retry text"); + var keptPayload = CaptureRequestContract.ParseStoredPayload(row.Payload) with + { + Disposition = new CaptureDispositionV1( + CaptureDisposition.Kept, + DateTimeOffset.UtcNow, + _userId) + }; + row.UpdatePayload(CaptureRequestContract.SerializePayload(keptPayload)); + var durable = DurableFor(row, "stale retry text"); + durable.Keep(); + durable.SupersedeInlineTextSource("newer correction"); + var newerStamp = durable.UpdatedAt; + + _queue.Setup(repository => repository.GetByIdAsync(row.Id, It.IsAny())) + .ReturnsAsync(row); + _queue.Setup(repository => repository.TrySetCaptureDispositionAsync( + row.Id, + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .ReturnsAsync(false); + _captureStore + .Setup(store => store.GetByIdForUpdateAsync(row.Id, _userId, It.IsAny())) + .ReturnsAsync(durable); + + var result = await CreateService().KeepAsync(_userId, row.Id); + + result.IsSuccess.Should().BeFalse("a stale retry must lose before touching the aggregate"); + result.ErrorCode.Should().Be(Taskdeck.Domain.Exceptions.ErrorCodes.Conflict); + durable.CurrentText.Should().Be("newer correction"); + durable.UpdatedAt.Should().Be(newerStamp); + _captureStore.Verify( + store => store.UpdateAsync(It.IsAny(), It.IsAny()), + Times.Never); + } + + [Fact] + public async Task CancelAsync_WhenTheQueueCasLoses_ShouldNotOverwriteOrArchiveNewerDurableText() + { + var row = QueueRow("stale cancel text"); + var durable = DurableFor(row, "stale cancel text"); + durable.SupersedeInlineTextSource("newer correction"); + var newerStamp = durable.UpdatedAt; + + _queue.Setup(repository => repository.GetByIdAsync(row.Id, It.IsAny())) + .ReturnsAsync(row); + _queue.Setup(repository => repository.TrySetCaptureDispositionAsync( + row.Id, + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .ReturnsAsync(false); + _captureStore + .Setup(store => store.GetByIdForUpdateAsync(row.Id, _userId, It.IsAny())) + .ReturnsAsync(durable); + + var result = await CreateService().CancelAsync(_userId, row.Id); + + result.IsSuccess.Should().BeFalse("a stale cancellation must lose before touching the aggregate"); + result.ErrorCode.Should().Be(Taskdeck.Domain.Exceptions.ErrorCodes.Conflict); + durable.CurrentText.Should().Be("newer correction"); + durable.Disposition.Should().Be(CaptureUserDisposition.Active); + durable.UpdatedAt.Should().Be(newerStamp); + _captureStore.Verify( + store => store.UpdateAsync(It.IsAny(), It.IsAny()), + Times.Never); + } + + [Fact] + public async Task CancelAsync_WhenAlreadyCancelled_ShouldRefreshQueueFallbackWithoutMutatingArchivedText() + { + var row = QueueRow("first draft"); + var durable = DurableFor(row, "first draft"); + var correctedPayload = CaptureRequestContract.ParseStoredPayload(row.Payload) with + { + Text = "corrected draft" + }; + row.UpdatePayload(CaptureRequestContract.SerializePayload(correctedPayload)); + row.Cancel(); + var cancelledStamp = new DateTimeOffset(2026, 9, 1, 12, 0, 0, TimeSpan.Zero); + typeof(Entity).GetProperty(nameof(Entity.UpdatedAt))!.SetValue(row, cancelledStamp); + + durable.Archive(); + var maskedStamp = cancelledStamp.AddMinutes(1); + typeof(Entity).GetProperty(nameof(Entity.UpdatedAt))!.SetValue(durable, maskedStamp); + var refreshedQueueStamp = maskedStamp.AddMinutes(1); + + _queue.Setup(repository => repository.GetByIdAsync(row.Id, It.IsAny())) + .ReturnsAsync(row); + _queue.Setup(repository => repository.TrySetCaptureDispositionAsync( + row.Id, + RequestStatus.Cancelled, + cancelledStamp, + RequestStatus.Cancelled, + It.Is(payload => string.Equals(payload, row.Payload, StringComparison.Ordinal)), + It.IsAny())) + .Callback(() => + typeof(Entity).GetProperty(nameof(Entity.UpdatedAt))!.SetValue(row, refreshedQueueStamp)) + .ReturnsAsync(true); + _captureStore + .Setup(store => store.GetByIdForUpdateAsync(row.Id, _userId, It.IsAny())) + .ReturnsAsync(durable); + _captureStore + .Setup(store => store.GetByIdForUserAsync(row.Id, _userId, It.IsAny())) + .ReturnsAsync(durable); + + var service = CreateService(); + var result = await service.CancelAsync(_userId, row.Id); + var reread = await service.GetByIdAsync(_userId, row.Id); + + result.IsSuccess.Should().BeTrue(result.ErrorMessage); + reread.IsSuccess.Should().BeTrue(reread.ErrorMessage); + reread.Value!.RawText.Should().Be( + "corrected draft", + "the successful idempotent CAS must make the truthful queue fallback newer"); + row.UpdatedAt.Should().Be(refreshedQueueStamp); + durable.CurrentText.Should().Be("first draft", "archived source lineage cannot be superseded inline"); + durable.UpdatedAt.Should().Be(maskedStamp, "a failed archived repair must not earn a stamp"); + durable.SourceAssets.Should().ContainSingle(); + _captureStore.Verify( + store => store.UpdateAsync(It.IsAny(), It.IsAny()), + Times.Never); + _unitOfWork.Verify(unit => unit.CommitTransactionAsync(It.IsAny()), Times.Once); + _unitOfWork.Verify(unit => unit.RollbackTransactionAsync(It.IsAny()), Times.Never); + } + [Fact] public async Task GetByIdAsync_ShouldReadTheTextAndSourceFromTheAggregate() { diff --git a/ci/schemas/ci-run.v1.schema.json b/ci/schemas/ci-run.v1.schema.json index 2f1c45b7b..063aa99e5 100644 --- a/ci/schemas/ci-run.v1.schema.json +++ b/ci/schemas/ci-run.v1.schema.json @@ -19,7 +19,7 @@ "additionalProperties": false, "required": ["code", "detail"], "properties": { - "code": { "enum": ["plan-missing", "plan-invalid", "planner-error", "plan-job-failed", "head-sha-mismatch", "base-sha-mismatch", "policy-digest-mismatch", "trust-mismatch", "labels-mismatch", "mode-mismatch", "evidence-unavailable", "selected-evidence-missing", "selected-not-success", "evidence-sha-missing", "evidence-wrong-sha", "skipped-without-reason"] }, + "code": { "enum": ["plan-missing", "plan-invalid", "planner-error", "plan-job-failed", "plan-job-cancelled", "head-sha-mismatch", "base-sha-mismatch", "policy-digest-mismatch", "trust-mismatch", "labels-mismatch", "mode-mismatch", "evidence-unavailable", "selected-evidence-missing", "selected-not-success", "evidence-sha-missing", "evidence-wrong-sha", "skipped-without-reason"] }, "detail": { "type": "string" } } } diff --git a/docs/STATUS.md b/docs/STATUS.md index 084f29a91..188b463d8 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -67,7 +67,7 @@ Post-v0.2.0 fixes on `main` (since 2026-08-29): - **The single-turn `RequestProposal` path now preserves a degraded outcome instead of building a proposal from the user's own text (`#1628`, PR `#2294`, merge `42aec57e2`).** On the non-tool-calling `ChatService` proposal path, when a terminal provider outcome is degraded and the turn requests a proposal, the service preserves the returned and persisted `degraded` classification and reason, returns no proposal id, and calls neither planner nor direct proposal creation. If that degraded fallback itself promises a proposal for an actionable turn, the user instead receives explicit copy that no proposal was created and no board was changed; generic degraded non-action content remains untouched. Non-degraded proposal behavior is unchanged. Tool-calling orchestration is outside this guard: a `propose_*` tool can persist a proposal before a later orchestration round becomes degraded, and `#1628` did not change that path or its response-shaping residual. Evidence: 69 focused `ChatService` tests on the final base, the full backend solution at the feature head (Domain 1,520; Application 4,036; API 2,733 passed / 4 skipped; CLI 147; Architecture 26 passed / 1 skipped; Integration 36), one clean current-base independent review, a clean current-head Codex review, and hosted CI with 21 successes plus 11 declared skips including E2E Smoke. **Not verified:** no live-provider or browser journey; the behavior is proven at the application-service seam. - **Automatic proposal expiry now respects the archived-board guard (`#2197`).** PR `#2168` gave the interactive decision lanes an archived-board guard (ADR-0063), but both automatic expiry paths — `ProposalHousekeepingWorker.ExpireStaleProposalsAsync` and `AutomationProposalService.ExpireProposalsAsync` — still called `Expire()` and saved without it, so the scheduled worker silently decided pending proposals on archived boards. The guard now lives in the single query both paths read (`IAutomationProposalRepository.GetExpiredAsync`), which returns an `ExpiredProposalSweep` partition: archived-board rows are withheld as a count instead of entities, so no save, notification, or audit row is produced for them, and each path logs a non-secret count of what it declined. Board-less and dangling-board rows stay expirable, and restoring a board makes its withheld proposals expirable again. Verified by Application.Tests 3892/3892, Api.Tests 2538 passed / 4 pre-existing skips, and Architecture.Tests 26 passed / 1 skip, with the query guard and the skip log each mutation-checked. This defect was authored on the v0.3 lane and reached `main` in the `#2196` merge; **it is not present in the published v0.2.0.** - **Partial transcript dates no longer resolve to a hallucinated year (`#2193`, PR `#2206`).** The extraction prompt now states the capture's calendar day (ADR-0058) and tells the model to resolve a day/month with no year to the first such date on or after it, never to guess a year, and never to emit a date outside the contract's plausibility window; `CaptureTriageOutputContract` gained that window (2 years before / 5 years after the reference) plus a `ReviewDueDateHint` that drops an unparseable or implausible hint and returns an honest note, and `LlmCaptureTriagePrompt.TryParseTasks` applies it so one bad date costs the hint rather than the whole extraction. Found by the 2026-08-29 acceptance run of the published v0.2.0 build, where `gpt-4o-mini` answered "Monday 1 September" (spoken in August 2026) with `2023-09-01` and a format-only check carried it onto a card. Evidence at the PR head: 184 triage-filtered, 3,915 Application, 26 Architecture, and 5 transcript-triage golden-path API tests passed, with the window and the rendered reference date both mutation-checked. **Not verified:** no live-provider run against a real model on this head; the notes the parser now produces are returned at the contract boundary and are not yet surfaced to the reviewer, and the reference date is the server's UTC day at triage rather than the capture row's own `CreatedAt`, which a retry-after-failure re-enqueue (`CaptureService.cs:342`) can push days later — both tracked on `#2210`. The prompt's derivation semantics changed without a `llm-triage.v3` prompt-version bump; that bump is a 14-file change across backend, the frontend provenance classifier, the Windows release-acceptance gate (`scripts/ci/windows_desktop_archive.py:949`), and four docs, and is tracked on `#2211`. -- **The triage degradation notice now renders on degraded successful captures (`#2202`).** Both Inbox surfaces gated their message on a *failed* status, so the notice `#2192` stores was API-visible and shown nowhere; this supersedes the "not yet rendered" sentence in the v0.2.0 block below. A capture whose triage completed carrying an `errorMessage` now renders a caution — that a model reading could not be confirmed, the server's notice **verbatim**, what that means for review, and to check the provider settings — in the Paper triage row and the Legacy detail panel, localized in en/it/es. Neither failure gate was widened: the Paper row keeps its clamped `role="alert"` failure span inside the row button and gains a separate `role="status"` block outside it on the note palette, and the Legacy panel adds a plain block above its unchanged error banner rather than reusing `TdInlineAlert`, which hardcodes `role="alert"`. The shared predicate is a positive allowlist over the three statuses that mean the run completed (`Triaged`/`ProposalCreated`/`Converted`) rather than `!== Failed`, because `LlmRequest.Cancel()` accepts a Failed request and clears nothing — a failed-then-ignored capture surfaces as `Ignored` still carrying its failure text, which a negative rule would re-dress as a friendly degradation notice. The copy never asserts WHICH engine authored the result: `CaptureTriageService.ResolveReuseDegradedNotice` (crash recovery) itself reports the author as unknown, so every mention of the deterministic extractor is conditional, and telling the two apart would need a machine-readable degradation kind from the backend (`#2212`) rather than parsing the server sentence, which the frontend never does. The review sentence is status-specific, because "read it before you apply it" is impossible on `Triaged` and stale on `Converted`. Legacy batch triage also reconciles a cached detail against the list snapshot it just fetched when the row reached a terminal status, so an open panel no longer shows a pre-batch `New`/`Triaging` capture without its notice; this is not a poll, so an item finishing after that fetch still needs the existing refresh paths. Evidence: 38 tests in the three degradation spec files (both surfaces plus the predicate) and 3 new capture-store tests for the batch reconciliation, 239 neighbouring Inbox/capture-store/i18n tests, `npm run typecheck`, `npm run build`, ESLint on the touched files, and two measured mutation checks (suppressing the notice fails 12 tests, all of them the degraded cases, with every clean-success and failed case still green; substituting `!== Failed` fails 5, exactly the failed-then-cancelled, pre-completion and out-of-contract cases). **Still not rendered:** the notice is capture-side only — the Review surface discloses the deterministic producer through proposal provenance but carries no pointer back to this capture-side notice, and the outcome remains prose rather than a machine-readable field, so no surface groups or filters by degradation cause. **Not verified:** no live-provider run and no browser or packaged pass; proven by component tests only. +- **The triage degradation notice now renders on degraded successful captures (`#2202`).** Both Inbox surfaces gated their message on a *failed* status, so the notice `#2192` stores was API-visible and shown nowhere; this supersedes the "not yet rendered" sentence in the v0.2.0 block below. A capture whose triage completed carrying an `errorMessage` now renders a caution — that a model reading could not be confirmed, the server's notice **verbatim**, what that means for review, and to check the provider settings — in the Paper triage row and the Legacy detail panel, localized in en/it/es. Neither failure gate was widened: the Paper row keeps its clamped `role="alert"` failure span inside the row button and gains a separate `role="status"` block outside it on the note palette, and the Legacy panel adds a plain block above its unchanged error banner rather than reusing `TdInlineAlert`, which hardcodes `role="alert"`. The shared predicate is a positive allowlist over the three statuses that mean the run completed (`Triaged`/`ProposalCreated`/`Converted`) rather than `!== Failed`, because `LlmRequest.Cancel()` accepts a Failed request and clears nothing — a failed-then-ignored capture surfaces as `Ignored` still carrying its failure text, which a negative rule would re-dress as a friendly degradation notice. The copy never asserts WHICH engine authored the result: `CaptureTriageService.ResolveReuseDegradedNotice` (crash recovery) itself reports the author as unknown, so every mention of the deterministic extractor is conditional, and telling the two apart would need a machine-readable degradation kind from the backend (`#2212`) rather than parsing the server sentence, which the frontend never does. The review sentence is status-specific, because "read it before you apply it" is impossible on `Triaged` and stale on `Converted`. Legacy batch triage also reconciles a cached detail against the list snapshot it just fetched when the row reached a terminal status, so an open panel no longer shows a pre-batch `New`/`Triaging` capture without its notice. **Superseded 2026-09-03 (`#2230`, PR `#2299`, merge `4c6ce5f83`): that reconciliation is now a poll.** The sentence that stood here said it was not one; see the batch-triage polling entry below for what shipped and what remains. Evidence: 38 tests in the three degradation spec files (both surfaces plus the predicate) and 3 new capture-store tests for the batch reconciliation, 239 neighbouring Inbox/capture-store/i18n tests, `npm run typecheck`, `npm run build`, ESLint on the touched files, and two measured mutation checks (suppressing the notice fails 12 tests, all of them the degraded cases, with every clean-success and failed case still green; substituting `!== Failed` fails 5, exactly the failed-then-cancelled, pre-completion and out-of-contract cases). **Still not rendered:** the notice is capture-side only — the Review surface discloses the deterministic producer through proposal provenance but carries no pointer back to this capture-side notice, and the outcome remains prose rather than a machine-readable field, so no surface groups or filters by degradation cause. **Not verified:** no live-provider run and no browser or packaged pass; proven by component tests only. - **The planner validates permissions before it persists a proposal (`#1433`, PR `#2219`, merge `17ed7d560`).** `AutomationPlannerService` checked `BoardAccessBar.Write` *after* creating the proposal on both the single-instruction and batch planning paths, so a denied request could leave a persisted proposal behind while returning `Forbidden`. Validation now runs first on both paths; the existing `Forbidden` code/message contract is unchanged, and one regression per path pins a denied request to zero `CreateProposalAsync` calls. Verified by the focused Application planner tests — 86/86 passed, re-run after merging `origin/main`. **Not verified:** the full backend solution and hosted CI were not run locally. **Not in the published v0.2.0.** - **The unscoped CLI card move validates the target column's board (`#2125`, PR `#2218`, merge `e81fb0f92`).** `CardService.MoveCardAsync` — the path the CLI uses, not the board-scoped HTTP path — checked neither that the target column belonged to the same board nor whether the target board was archived. A foreign active target now returns `NotFound` and an archived target board `InvalidOperation`; neither path saves or relocates the card, and board-scoped HTTP and proposal behaviour are unchanged. Verified by Application `CardServiceTests` 39 passed and CLI `CardsCommandTests` 13 passed (error code, exit code, unchanged source-card position). **Not verified:** the full backend suite and hosted CI locally; the target-board lookup assumes normal column-to-board referential integrity. **Not in the published v0.2.0.** - **The Windows launcher no longer reports a false transient-cleanup failure after a completed npm stage (`#2161`, PR `#2221`, merge `ae38d9f43`).** `Invoke-NpmStage` re-read `Process.HasExited` in its `finally` block; that second query can race on Windows and set `TransientStageCleanupUnproved`, retaining `dev-up.pids` after the API port was already released. `scripts/dev-up.ps1` now records the completion established by its bounded `WaitForExit(100)` loop, so only an incomplete — including cancellation-interrupted — wait enters the identity-proven transient cleanup path. Verified by the focused PowerShell transactional suite 6/6 (9.25 s) and the full source launcher suite 85/85 (303.15 s); hosted CI had not run for the PR. **The flake family recurred on `main` at `aadbe6eb2` after this merge, so `#2161` stays open** — this fix removed one proven race, not the family. **Not in the published v0.2.0.** @@ -88,8 +88,10 @@ Post-v0.2.0 Review-queue liveness (2026-08-29): - **Hardened in review before merge.** A stale read could revert a decision: every single-proposal path (approve/reject/defer/execute) patches the row locally with a new array and never touches the load counter, so a read issued before the click could land after it and write `PendingReview` back over a row whose receipt already said approved. The poll now snapshots the queue's array identity before its await and discards its answer if the reference moved, and it re-evaluates the surface guard **after** the await so a read cannot land under a dialog opened while it was in flight (the Reject dialog is bound to a computed that closes itself when its proposal leaves the list, discarding a half-typed reason). A `403` is now distinguished from a transient failure - board access was revoked, so the poll stops rather than hammering, unauthorised rows are dropped, and both skins say why, because clearing the queue silently would swap a permission failure for a fresh "Nothing waiting. Good."; a `500` leaves the queue alone and keeps polling. The deep-link pin is restored at its `createdAt` position rather than appended, so the record under review no longer jumps to the bottom of a rail that renders array order. The poll opts out of the shared three-retry interceptor and carries an `AbortController` cancelled by `stopQueueRefresh`. - **What is both skins, and what is Paper only.** The bounded poll, the badge refresh, the polite `role="status"` live region for the awaiting count, and the revoked-access message are wired in **both** Paper and Legacy - Legacy had no badge wiring at all and no tests for any of it, and it renders no translated strings, so its copy is plain text like the rest of that file. The badge trigger is the queue **array**, not the awaiting count: watching the count missed a board-scoped view, where another board's proposal never moves the scoped number, and a decision that swaps one pending row for another. The "This week" cadence bars are **Paper only** (`ReviewMiniCadence`), and they follow the refresh with no extra wiring because they project the same proposals array. - **Evidence:** 563 tests across the 25 review, Legacy-Review, i18n-catalog and source-guard spec files, `npm run typecheck`, `npm run build`, and ESLint over the eleven touched files, all green. `scripts/ci/check-bundle-size.mjs` passes: eager JS **385.90 KB** against the 1250 KB hard gate (`origin/main` measures 385.61 KB); its total-JS report-ceiling warning is warning-only and **pre-existing on `origin/main`** at 1314.76 KB, which this change moves to 1318.61 KB. Seven mutation checks each killed their target test and only that test - dropping the decision-identity guard, skipping the post-await guard re-check, appending rather than positioning the deep-link pin, treating 403 as transient, dropping the deep-link pin entirely, ignoring the visibility guard, and removing the badge watch. Introducing the poll first broke three existing expiry specs, whose `mountView` fixture answered the queue with a single `...Once`; a real server keeps answering, so the fixture now does too. -- **Polling restarts after permission recovery (`#2214` item 2, PR `#2298`, merge `bd2541991`).** A current-scope `403` now suspends the Review interval without discarding the owning surface's refresh predicate or queue-replacement hook. The first successful explicit load that proves access has returned clears the revoked-access state and rearms exactly one interval; a permanent stop or scope disposal remains final, so a late recovery response cannot resurrect polling after route leave. Both Review skins inherit the repair through the shared composable. Evidence: 97 focused and 148 broader Review tests, the full frontend suite (357 files / 5,270 tests), typecheck, scoped zero-warning ESLint, build, diff hygiene, clean current-base review, and hosted CI with 21 successes plus 11 declared skips. **This is a bounded delivery, not closure of `#2214`:** deep-link re-authorization, stale/degraded queue disclosure, count-neutral replacement announcements, per-request deadlines, and the recorded low-impact accessibility/diagnostic residuals remain open. **Not verified:** no live ACL revoke/re-grant browser journey or screen-reader pass. +- **Polling restarts after permission recovery (`#2214` item 2, PR `#2298`, merge `bd2541991`).** A current-scope `403` now suspends the Review interval without discarding the owning surface's refresh predicate or queue-replacement hook. The first successful explicit load that proves access has returned clears the revoked-access state and rearms exactly one interval; a permanent stop or scope disposal remains final, so a late recovery response cannot resurrect polling after route leave. Both Review skins inherit the repair through the shared composable. Evidence: 97 focused and 148 broader Review tests, the full frontend suite (357 files / 5,270 tests), typecheck, scoped zero-warning ESLint, build, diff hygiene, clean current-base review, and hosted CI with 21 successes plus 11 declared skips. **This is a bounded delivery, not closure of `#2214`:** deep-link re-authorization, count-neutral replacement announcements, and the recorded low-impact accessibility/diagnostic residuals remain open. **Two items on this list were delivered later by PR `#2388` (merge `4b8075632`) and are struck here so the two entries do not contradict each other:** per-request deadlines (`REVIEW_QUEUE_REQUEST_DEADLINE_MS = 8_000`, `useReviewProposals.ts:53`, raced at `:590`) and the degraded queue disclosure (`consecutiveQueueRefreshFailures` against `REVIEW_QUEUE_CONSECUTIVE_FAILURE_THRESHOLD`, `:543-560`, cleared at `:519-520`). The **stale** copy's Paper localization is still open as `#2214` item 4, since it is hardcoded English. **Not verified:** no live ACL revoke/re-grant browser journey or screen-reader pass. - **Not verified:** no live browser or packaged run - the 115 s repro was not re-measured against a running stack, the revoked-access path was not exercised against a real 403, and no screen reader was used to hear the live region; the fix is proven by component and composable tests only. The empty-state copy is unchanged: with the refresh in place "Nothing waiting" can no longer be minutes stale while the page is being looked at, so the acceptance bullet's "or is refreshed" branch is taken rather than its copy branch. +- **Background Review queue requests are deadline-bound and degrade politely (`#2214`, PR `#2388`, merge `4b8075632`).** Each background queue request is bound to an **8-second deadline**; three consecutive transient failures preserve the last trustworthy queue and expose a polite degraded notice rather than blanking the list, and any successful poll or explicit load clears it. `403` authority, teardown and supersession semantics are unchanged, and both Legacy and Paper render the state without hiding retained actions or disturbing the Paper three-column layout. Evidence: 293 focused tests across three spec files, 180 fix-diff affected specs, typecheck, build, scoped ESLint, docs and GitHub-ops governance, diff hygiene. Initial fresh-context review found two HIGH layout/conditional defects; one bounded fix commit resolved both and the fix-diff review found no blocker. **Known MEDIUM, deferred:** the Paper stale copy is hardcoded English, carried on `#2214` item 4. **Recorded 2026-09-03 by the coordinator, not by PR `#2388`:** that PR's body claimed it had updated this file, and it did not - merge `4b8075632` changed nothing in `docs/STATUS.md`. **Not verified:** full frontend suite, Playwright or manual browser, live degraded backend, screen reader, translation quality. +- **Legacy batch triage polls to a terminal detail (`#2230`, PR `#2299`, merge `4c6ce5f83`).** `pollBatchTriageCompletion` will not accept cached terminal state as completion before its first authoritative read: `observedPostEnqueueList` starts false, the completion predicate's summary branch returns false while it is unset, and it is set true only after a successful `captureApi.listItems` inside a tick. That was the defect the PR parked on twice. Merged with full exact-head CI green including `E2E Smoke`, both Windows legs and all three branch-protection contexts, after one fresh-context adversarial pass returned no blocker. **`#2230` is deliberately NOT closed.** The poll's own 60-second wall (`BATCH_TRIAGE_POLL_MAX_DURATION_MS`) is shorter than the server's worst case: the worker drains at most `MaxBatchSize` items total per tick and then sleeps `QueuePollIntervalSeconds` (shipped: 5 and 5, shared between capture and non-capture work), while the server accepts batches up to 50, so a 50-item batch needs at least 50 seconds of pure queue delay before any LLM latency. Expiry is silent - no toast, no error state, no exposed ref - so a large batch can expire back into the cached `Triaging` detail that `#2230` exists to remove. The single-item poll allows 15 minutes for strictly less work. Remaining acceptance is on the issue. Two Codex P2s about `403` handling were declined as unreachable: `GET /capture/items` is user-scoped with no board authorization and `GET /capture/items/{id}` returns `403` only on a user mismatch, so board-membership revocation produces no `403` on either read. **Not verified:** the timing above is arithmetic over committed configuration, not an observed batch run. - **The active proposal no longer changes under a live reviewer (`#2215` parts A and B, the `#2208` residual).** A poll that dropped or reordered away the row on screen used to slide the selection onto the next pending proposal: `ReviewMain` is keyed on the proposal id, so it was re-created and focus left the decision controls, while the window-level review keymap stayed enabled for the newly selected record - the next Enter/Backspace/D/E decided a proposal the reviewer never chose. `useReviewProposals` now signals a landed poll (`startQueueRefresh`'s `onQueueReplaced` hook, fired synchronously after the queue is replaced and cleared by `stopQueueRefresh`); Paper arms a one-shot flag from it, so an active-proposal change observed while armed is by definition not one the reviewer made. It pins the taken proposal, renders an explicit "This proposal left the review queue" notice in place of the decision column (localized en/it/es, `role="status"` + `aria-live="polite"`), and moves focus to the notice's own control. What silences the keymap is `activeProposal` being null in that state - `useReviewKeymap`'s `enabled` predicate requires a non-null active proposal - while the focus move is what stops focus falling to the document body and gives the reviewer a safe target; the two are independent. The copy states only what was observed - the row left the queue - because one poll answer is not proof of a decision: the list endpoint also omits a PendingReview proposal whose `deferredUntil` is in the future, and it is capped at 200 rows. The notice's action re-reads the queue authoritatively rather than only dismissing itself, and no fetch-by-id was added. A deep-linked target is unaffected: `refreshProposals` already pins it. The notice is dropped by any explicit choice - selecting a row, changing filter, changing board scope, following a deep link, or its own "Back to the queue" button, which also clears the stale explicit selection so the ordinary first-pending default resumes. **Part B:** the rendered diff and the revision state are keyed on `latestRevisionId` as well as the proposal id, in **both** skins. Keying on the id alone left a diff on screen that had been computed for the previous revision once a poll brought in one another session had saved, while Approve pins and Apply executes the server's latest; and `useProposalRevisions` kept `revisionCount` and the cached `latestRevision` from page entry, so `editablePayload` offered superseded operations to the next save. The revision resync is deliberately a read, not the full reset the proposal-change path does - closing an open composer over somebody else's change would discard a half-written edit. **Evidence:** 933 tests across all 46 composable and i18n spec files plus 502 across the 21 review/Legacy-Review/i18n/source-guard files, `npm run typecheck` and `npm run build`, all green. Seventeen mutation checks, each killing its target specs and only those: removing the selection guard (4), disarming the poll signal (4), removing the focus move (1), disabling and then forcing the Paper diff's revision comparison (1 each), dropping `latestRevisionId` from the revision-state key (1), disabling and then widening the resync branch (1 and 4), disabling then forcing the Legacy pane's revision comparison (1 and 3), and - from review round 1 - republishing a failed resync as authoritative (1), making that resync loud (1), deleting the no-op key guard (1), and dropping the notice's queue re-read (1). Round 2 adds four more: dropping the null-transition rule (1), dropping the `approvedRevisionId` fallback (1), dropping both (4), and the three round-2 specs staying green against the restored rule. Round 1 also fixed a real defect the first pass shipped: the resync left `revisionsLoaded` true, so ONE transient GET failure under a poll would have published a zero count as authoritative - a false zero-op Apply toast and a fallback to pre-revision operations - and it toasted from a background poll, against `refreshProposals`' silent doctrine. It now clears `revisionsLoaded` first and loads `{ silent: true }`. **Round 2 caught the sharper one, verified against the backend:** `latestRevisionId` is a PendingReview-ONLY value on the wire - `AutomationProposalService.BuildEffectiveProposalDto` sets it to `Status == PendingReview ? effectiveRevision?.Id : null` - so approving a revised proposal moves it `rev-X` -> `null` in the very next read, and all three keys read the reviewer's OWN approval as a collaborator revision change. In Paper that wiped the open diff mid-decision, and because the key watcher runs before the read-only conversion watcher it wiped the pane rather than letting it convert to the decision-time stored presentation. All three now key on an EFFECTIVE identity - `latestRevisionId ?? approvedRevisionId`, shared as `proposalRevisionIdentity` in `utils/proposalIdentity.ts` - and clear only on a genuine move (`proposalRevisionMoved`): an identity reaching null is a status transition, never a revision change, because revisions are append-only and of the exits only `Approve` pins a replacement while `Reject`/`Expire`/`Dismiss` pin nothing. Both halves of the rule are separately load-bearing: dropping the null rule reddens the rejected-proposal conversion spec, and dropping the `approvedRevisionId` fallback reddens the stale-pre-approval-read spec. **Not verified:** no live browser or packaged run; no two-session concurrency repro. **Out of scope, still open on `#2215`:** part C beyond the revision-state resync delivered here, and the deep-review selectors (provenance, side effects, conflicts, history), which still key on the proposal id alone. - **Split residual:** the queue rail's **Apply rate** is a hardcoded empty, not a staleness bug - `PaperReviewView` never binds `ReviewQueueRail`'s optional `applyRate` prop, so `applyRatePct` is always `null` and "No decisions yet" renders unconditionally however many decisions were made. No refresh can fix it. Tracked as [`#2205`](https://github.com/Chris0Jeky/Taskdeck/issues/2205); `#2194` stays open until it is dispositioned. diff --git a/docs/platform/CONFIGURATION_REFERENCE.md b/docs/platform/CONFIGURATION_REFERENCE.md index 841d2e603..4e7da5ea7 100644 --- a/docs/platform/CONFIGURATION_REFERENCE.md +++ b/docs/platform/CONFIGURATION_REFERENCE.md @@ -425,7 +425,7 @@ rather than changing what a user sees. | Key | Type | Default | Description | Required? | | --- | --- | --- | --- | --- | -| `ContextFabric:DualWriteCaptures` | `bool` | `true` | When true, every capture admitted through the canonical `CaptureIntakeService` is written to the durable `Captures` aggregate under the queue row's own id (ID-preserving), staged in the same unit of work as the queue row. `CaptureIntakeService` is the single writer of the aggregate, and it serves **both** creation paths — `CaptureService.CreateAsync` and the `POST /api/llm-queue` enqueue path (`LlmQueueService.AddToQueueAsync`). The typed or pasted text becomes an immutable inline `SourceAsset` (verbatim, hashed over UTF-8) and a payload `externalRef` becomes an `ExternalReference` asset. **Turning this off is not consequence-free.** Captures created while it is off never reach the aggregate; captures that already have one are still kept in step (an edit, a keep or an archive still writes the aggregate — a flag about *new* rows never licenses an existing row to rot). Re-enabling does not simply resume: the next backfill pass must bring the window's captures in and reconcile anything that drifted. Nothing a user sees goes backwards at any point — the read path defers to whichever writer moved last (see `ReadCapturesFromStore`) — **provided no disposition change (keep, archive, reactivate) intervenes before the next reconcile pass**: such a write stamps the aggregate newer than the queue row that moved past it, defeating both the read guard and the divergence join and masking the divergence until `#2347` lands. The durable table is behind until that pass runs. | No | +| `ContextFabric:DualWriteCaptures` | `bool` | `true` | When true, every capture admitted through the canonical `CaptureIntakeService` is written to the durable `Captures` aggregate under the queue row's own id (ID-preserving), staged in the same unit of work as the queue row. `CaptureIntakeService` is the single writer of the aggregate, and it serves **both** creation paths — `CaptureService.CreateAsync` and the `POST /api/llm-queue` enqueue path (`LlmQueueService.AddToQueueAsync`). The typed or pasted text becomes an immutable inline `SourceAsset` (verbatim, hashed over UTF-8) and a payload `externalRef` becomes an `ExternalReference` asset. **Turning this off is not consequence-free.** Captures created while it is off never reach the aggregate; captures that already have one are still kept in step (an edit, a keep or an archive still writes the aggregate — a flag about *new* rows never licenses an existing row to rot). Re-enabling does not simply resume: the next backfill pass must bring the window's captures in and reconcile anything that drifted. Nothing a user sees goes backwards at any point — the read path defers to whichever writer moved last (see `ReadCapturesFromStore`). The durable table is behind until that pass runs. | No | | `ContextFabric:BackfillCaptures` | `bool` | `true` | When true, the ID-preserving backfill **and reconcile pass** runs after migrations at startup, on every host that applies them: the web API, the standalone MCP stdio and HTTP hosts, and the CLI. The backlog is a **divergence join**, not an anti-join: a capture-shaped queue row (`inbox.capture.%`) qualifies when it has no `Captures` row *or* when the queue row has been written since its capture last was. That second half is what repairs drift after a `DualWriteCaptures` window. It is idempotent and resumable — a row leaves the backlog the moment its capture agrees with it, so re-running creates nothing twice and a crash mid-way resumes at the next outstanding row — and it costs one marker read plus one indexed count on a database whose captures all agree. Progress, the distinct skip count and completion are recorded in `CaptureBackfillStates`. A row that cannot be mapped is logged by id, excluded for the rest of the run so the healthy rows behind it are still reached, and left readable through its queue row; while any row is outstanding the marker does not complete. A failure never blocks startup. Turning this off leaves an unfinished marker unfinished (Inbox reads stay on the queue row) and leaves drift unrepaired. | No | | `ContextFabric:ReadCapturesFromStore` | `bool` | `true` | When true, Inbox list / get resolve a capture's own material — its immutable source text, its capture source snapshot and its server intake time — from the durable aggregate through `ICaptureStore` instead of parsing the queue row's payload JSON. Mutation responses (keep, archive, cancel, edit) obey the same gate, so turning this off never leaks aggregate material through a write. Job state (queue status, processed-at, retry count, error message) and the fields that have no column yet (triage provenance, suggestion metadata, the disposition receipt's who/when/where) keep their shipped source, so the DTOs are byte-identical across the switch. **Three guards:** the switch arms only once the backfill marker records completion; it degrades per item, so a capture with no durable row is still read from its queue row; and it defers to the queue row for any capture whose text disagrees with an aggregate the queue row has been written past. A capture can neither disappear from the Inbox nor go backwards in it. Set to false to force every read onto the queue row without touching the dual-write. | No | diff --git a/docs/platform/PWA_OFFLINE_BEHAVIOR.md b/docs/platform/PWA_OFFLINE_BEHAVIOR.md index 17ee9426e..eb15a6f40 100644 --- a/docs/platform/PWA_OFFLINE_BEHAVIOR.md +++ b/docs/platform/PWA_OFFLINE_BEHAVIOR.md @@ -14,15 +14,26 @@ All app shell assets (JS, CSS, HTML, icons, fonts) are precached on first load. | Resource | Strategy | TTL | Notes | |----------|----------|-----|-------| -| API responses (`/api/*`) | Network only | N/A | Never stored by the service worker or browser cache because responses may be identity-bound | +| API responses (`/api/*`) | Network only | N/A | Not stored by the service worker: no runtime route matches a path starting with `/api`, and `ApiCacheControlMiddleware` stamps `no-store, private` on every `/api` response so the browser cache does not hold it either. See the boundary caveat below - this is not unconditional for an API base nested under `/assets/` or `/icons/` | | Lazy `it`/`es` locale chunks | StaleWhileRevalidate | Content-versioned | Cached after first use so the selected language remains available offline | | Static assets under `/assets/` and `/icons/` | CacheFirst | 30 days | Served from cache after a miss; there is no Google Fonts runtime route | The static-asset route is anchored on the directories the build emits, not on the file extension. The API base is a deployment choice - `VITE_API_BASE_URL` may be prefixed, such as `/taskdeck/api` - so denying `/api` alone would not stop an authenticated `GET /taskdeck/api/users/by-username/alice.png` -from being stored in the shared, cross-identity static cache. An unrecognised layout therefore loses -runtime caching for a static asset; it never admits an API response. +from being stored in the shared, cross-identity static cache. Anchoring on the emitted directories +closes that case: an unrecognised layout loses runtime caching for a static asset. + +**The boundary is directory-anchored, not absolute, and it is not origin-anchored.** Two limits are +load-bearing and were previously overstated here: + +- The deny test matches only a path *starting* with `/api`, while the admit test accepts `/assets/` or + `/icons/` followed by any nested path ending in a media or font extension. An API base nested under + either directory is therefore still admitted: `/assets/api/users/by-username/alice.png` is cached. + Do not deploy an API base under `/assets/` or `/icons/`. Tracked as `#2411`. +- The predicates test `url.pathname` only, and `cacheableResponse` is `{ statuses: [0, 200] }`, so an + opaque third-party response under a matching path is admitted too. Nothing identity-bound leaks, but + `taskdeck-static-assets` is not first-party-only. ### Retiring the pre-#2350 worker @@ -37,7 +48,9 @@ identifies it. The page then calls `registration.update()` and follows the repla `updatefound` and `statechange` until it reaches `installed`, at which point it is messaged to skip waiting. Following it matters: `registration.update()` resolves inside Install, *before* the install event's lifetime promises settle, so `registration.waiting` is normally still null when it returns and -a one-shot read there would never deliver the message. The replacement claims open clients on +a single read taken *at that moment* would not deliver the message. The code still reads +`registration.waiting` once up front, deliberately, for the case where a replacement is already +waiting because the user dismissed the update banner; that read is not the anti-pattern. The replacement claims open clients on activation, so the switch does not need a reload. Every step is bounded, and the whole migration has a hard 12-second ceiling, because session restore diff --git a/frontend/taskdeck-web/src/api/boardsApi.ts b/frontend/taskdeck-web/src/api/boardsApi.ts index 63053df6b..5289abac9 100644 --- a/frontend/taskdeck-web/src/api/boardsApi.ts +++ b/frontend/taskdeck-web/src/api/boardsApi.ts @@ -1,9 +1,9 @@ -import http from './http' +import http, { type BoardReadOptions } from './http' import type { Board, BoardDetail, CreateBoardDto, UpdateBoardDto, PaginatedBoards } from '../types/board' export const boardsApi = { - async getBoards(search?: string, includeArchived = false): Promise { - const result = await boardsApi.getBoardsPaginated(search, includeArchived, 0, 200) + async getBoards(search?: string, includeArchived = false, options?: BoardReadOptions): Promise { + const result = await boardsApi.getBoardsPaginated(search, includeArchived, 0, 200, options) return result.items }, @@ -12,6 +12,7 @@ export const boardsApi = { includeArchived = false, offset = 0, limit?: number, + options?: BoardReadOptions, ): Promise { const params = new URLSearchParams() if (search) params.append('search', search) @@ -19,12 +20,14 @@ export const boardsApi = { if (offset > 0) params.append('offset', String(offset)) if (limit !== undefined) params.append('limit', String(limit)) - const { data } = await http.get(`/boards?${params}`) + const url = `/boards?${params}` + const { data } = options ? await http.get(url, options) : await http.get(url) return data }, - async getBoard(id: string): Promise { - const { data } = await http.get(`/boards/${id}`) + async getBoard(id: string, options?: BoardReadOptions): Promise { + const url = `/boards/${id}` + const { data } = options ? await http.get(url, options) : await http.get(url) return data }, diff --git a/frontend/taskdeck-web/src/api/cardsApi.ts b/frontend/taskdeck-web/src/api/cardsApi.ts index 7c44474d4..5c18ee344 100644 --- a/frontend/taskdeck-web/src/api/cardsApi.ts +++ b/frontend/taskdeck-web/src/api/cardsApi.ts @@ -1,14 +1,19 @@ -import http from './http' +import http, { type BoardReadOptions } from './http' import type { Card, CardCaptureProvenance, CreateCardDto, UpdateCardDto, MoveCardDto } from '../types/board' export const cardsApi = { - async getCards(boardId: string, params?: { search?: string; labelId?: string; columnId?: string }): Promise { + async getCards( + boardId: string, + params?: { search?: string; labelId?: string; columnId?: string }, + options?: BoardReadOptions, + ): Promise { const searchParams = new URLSearchParams() if (params?.search) searchParams.append('search', params.search) if (params?.labelId) searchParams.append('labelId', params.labelId) if (params?.columnId) searchParams.append('columnId', params.columnId) - const { data } = await http.get(`/boards/${boardId}/cards?${searchParams}`) + const url = `/boards/${boardId}/cards?${searchParams}` + const { data } = options ? await http.get(url, options) : await http.get(url) return data }, diff --git a/frontend/taskdeck-web/src/api/http.ts b/frontend/taskdeck-web/src/api/http.ts index c1affcc34..b6a92d4fe 100644 --- a/frontend/taskdeck-web/src/api/http.ts +++ b/frontend/taskdeck-web/src/api/http.ts @@ -18,6 +18,8 @@ import { // sites (a misspelled key becomes a compile error instead of silently no-op). declare module 'axios' { interface AxiosRequestConfig { + /** Opt out of the shared retry interceptor for bounded read operations. */ + skipRetry?: boolean /** * Error statuses that are an expected part of this endpoint's contract * (e.g. a 404 from the optional card-provenance lookup for manual cards). @@ -30,6 +32,20 @@ declare module 'axios' { const REQUEST_ID_HEADER = 'X-Request-Id' +/** + * Maximum time a board load read may occupy the UI before the caller receives + * a terminal error. The board-load caller supplies this explicitly so nested + * board APIs such as provenance and comments retain their normal defaults. + */ +export const BOARD_REQUEST_TIMEOUT_MS = 10_000 + +/** Optional cancellation controls for board-scoped read wrappers. */ +export interface BoardReadOptions { + signal?: AbortSignal + timeout?: number + skipRetry?: boolean +} + function ensureRequestIdHeader(config: InternalAxiosRequestConfig): void { const headers = AxiosHeaders.from(config.headers) if (!headers.get(REQUEST_ID_HEADER)) { diff --git a/frontend/taskdeck-web/src/api/labelsApi.ts b/frontend/taskdeck-web/src/api/labelsApi.ts index 4a4d4a344..f4c82f284 100644 --- a/frontend/taskdeck-web/src/api/labelsApi.ts +++ b/frontend/taskdeck-web/src/api/labelsApi.ts @@ -1,9 +1,10 @@ -import http from './http' +import http, { type BoardReadOptions } from './http' import type { Label, CreateLabelDto, UpdateLabelDto } from '../types/board' export const labelsApi = { - async getLabels(boardId: string): Promise { - const { data } = await http.get(`/boards/${boardId}/labels`) + async getLabels(boardId: string, options?: BoardReadOptions): Promise { + const url = `/boards/${boardId}/labels` + const { data } = options ? await http.get(url, options) : await http.get(url) return data }, diff --git a/frontend/taskdeck-web/src/locales/en/review.ts b/frontend/taskdeck-web/src/locales/en/review.ts index 93c053fca..d8ef2edb5 100644 --- a/frontend/taskdeck-web/src/locales/en/review.ts +++ b/frontend/taskdeck-web/src/locales/en/review.ts @@ -69,10 +69,6 @@ export default { empty: 'Nothing in this filter.', cadence: { heading: 'This week', - // Rendered as `{label} {percentage}` — the value carries its own - // emphasis in the template, so this label must not end with punctuation. - applyRateLabel: 'Apply rate', - applyRateEmpty: 'No decisions yet', }, }, diff --git a/frontend/taskdeck-web/src/locales/es/review.ts b/frontend/taskdeck-web/src/locales/es/review.ts index cc69d170e..96edd342d 100644 --- a/frontend/taskdeck-web/src/locales/es/review.ts +++ b/frontend/taskdeck-web/src/locales/es/review.ts @@ -38,8 +38,6 @@ export default { empty: 'Nada en este filtro.', cadence: { heading: 'Esta semana', - applyRateLabel: 'Tasa de aplicación', - applyRateEmpty: 'Aún sin decisiones', }, }, diff --git a/frontend/taskdeck-web/src/locales/it/review.ts b/frontend/taskdeck-web/src/locales/it/review.ts index a3752be68..a15435a72 100644 --- a/frontend/taskdeck-web/src/locales/it/review.ts +++ b/frontend/taskdeck-web/src/locales/it/review.ts @@ -39,8 +39,6 @@ export default { empty: 'Niente in questo filtro.', cadence: { heading: 'Questa settimana', - applyRateLabel: 'Tasso di applicazione', - applyRateEmpty: 'Ancora nessuna decisione', }, }, diff --git a/frontend/taskdeck-web/src/store/board/boardCrudStore.ts b/frontend/taskdeck-web/src/store/board/boardCrudStore.ts index b87ac9adf..46e53bea3 100644 --- a/frontend/taskdeck-web/src/store/board/boardCrudStore.ts +++ b/frontend/taskdeck-web/src/store/board/boardCrudStore.ts @@ -4,6 +4,8 @@ import { boardsApi } from '../../api/boardsApi' import { cardsApi } from '../../api/cardsApi' import { labelsApi } from '../../api/labelsApi' +import axios from 'axios' +import { BOARD_REQUEST_TIMEOUT_MS, type BoardReadOptions } from '../../api/http' import { buildDemoBoardList, buildDemoBoardDetail } from '../../utils/demoData' import type { CreateBoardDto, UpdateBoardDto } from '../../types/board' import type { BoardState } from './boardState' @@ -17,6 +19,7 @@ const FETCH_BOARDS_THROTTLE_MS = 5000 export function createBoardCrudActions(state: BoardState, helpers: BoardHelpers) { let lastFetchBoardsAt = 0 let boardFetchGeneration = 0 + let activeBoardFetchController: AbortController | null = null async function fetchBoards(search?: string, includeArchived = false) { const now = Date.now() @@ -59,6 +62,9 @@ export function createBoardCrudActions(state: BoardState, helpers: BoardHelpers) async function fetchBoard(id: string): Promise { const requestGeneration = ++boardFetchGeneration + activeBoardFetchController?.abort() + const controller = new AbortController() + activeBoardFetchController = controller if (helpers.isDemoMode) { state.loading.value = true @@ -70,6 +76,7 @@ export function createBoardCrudActions(state: BoardState, helpers: BoardHelpers) state.currentBoardLabels.value = [] state.cardCommentsByCardId.value = {} state.loading.value = false + activeBoardFetchController = null return true } @@ -79,10 +86,15 @@ export function createBoardCrudActions(state: BoardState, helpers: BoardHelpers) try { state.loading.value = true state.error.value = null + const readOptions: BoardReadOptions = { + signal: controller.signal, + timeout: BOARD_REQUEST_TIMEOUT_MS, + skipRetry: true, + } const [board, cards, labels] = await Promise.all([ - boardsApi.getBoard(id), - cardsApi.getCards(id), - labelsApi.getLabels(id), + boardsApi.getBoard(id, readOptions), + cardsApi.getCards(id, undefined, readOptions), + labelsApi.getLabels(id, readOptions), ]) if (requestGeneration !== boardFetchGeneration) { @@ -103,15 +115,21 @@ export function createBoardCrudActions(state: BoardState, helpers: BoardHelpers) state.cardCommentsByCardId.value = {} return true } catch (e: unknown) { - if (requestGeneration !== boardFetchGeneration) { + if (requestGeneration !== boardFetchGeneration || axios.isCancel(e)) { return false } + // Ensure held-open siblings are cancelled before exposing the failure to + // the caller. The next explicit Retry starts a new generation. + controller.abort() helpers.handleApiError(e, 'Failed to fetch board') throw e } finally { if (requestGeneration === boardFetchGeneration) { state.loading.value = false + if (activeBoardFetchController === controller) { + activeBoardFetchController = null + } } } } diff --git a/frontend/taskdeck-web/src/tests/api/http.spec.ts b/frontend/taskdeck-web/src/tests/api/http.spec.ts index df9eb426b..acefe9fec 100644 --- a/frontend/taskdeck-web/src/tests/api/http.spec.ts +++ b/frontend/taskdeck-web/src/tests/api/http.spec.ts @@ -506,6 +506,28 @@ describe('http interceptors (#725)', () => { expect(response.status).toBe(201) expect(response.data).toEqual({ id: 'new-1' }) }) + + it('does not apply fetchBoard bounds to unrelated nested board reads', async () => { + vi.spyOn(tokenStorage, 'getToken').mockReturnValue(null) + mock.onGet('/boards/board-1/cards/card-1/provenance').reply(200, { sourceType: 'manual' }) + + const response = await http.get('/boards/board-1/cards/card-1/provenance') + + expect(response.config.timeout).toBe(0) + expect((response.config as { skipRetry?: boolean }).skipRetry).toBeUndefined() + }) + + it('does not automatically retry a failed fetchBoard transport request', async () => { + mock.onGet('/boards/board-1').reply(503, { message: 'unavailable' }) + + await expect( + http.get('/boards/board-1', { timeout: 10_000, skipRetry: true }), + ).rejects.toMatchObject({ + response: { status: 503 }, + }) + + expect(mock.history.get).toHaveLength(1) + }) }) // ── Retry interceptor (#854) ─────────────────────────────────────────── diff --git a/frontend/taskdeck-web/src/tests/store/board/boardCrudStore.spec.ts b/frontend/taskdeck-web/src/tests/store/board/boardCrudStore.spec.ts index 3b54880af..3f22e165e 100644 --- a/frontend/taskdeck-web/src/tests/store/board/boardCrudStore.spec.ts +++ b/frontend/taskdeck-web/src/tests/store/board/boardCrudStore.spec.ts @@ -1,5 +1,7 @@ import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest' import { ref } from 'vue' +import axios from 'axios' +import { BOARD_REQUEST_TIMEOUT_MS } from '../../../api/http' const { mockBoardsApi } = vi.hoisted(() => ({ mockBoardsApi: { @@ -264,7 +266,31 @@ describe('boardCrudStore', () => { const { fetchBoard } = createBoardCrudActions(state as any, helpers as any) const committed = await fetchBoard('board-1') - expect(mockBoardsApi.getBoard).toHaveBeenCalledWith('board-1') + expect(mockBoardsApi.getBoard).toHaveBeenCalledWith( + 'board-1', + expect.objectContaining({ + signal: expect.any(AbortSignal), + timeout: BOARD_REQUEST_TIMEOUT_MS, + skipRetry: true, + }), + ) + expect(mockCardsApi.getCards).toHaveBeenCalledWith( + 'board-1', + undefined, + expect.objectContaining({ + signal: expect.any(AbortSignal), + timeout: BOARD_REQUEST_TIMEOUT_MS, + skipRetry: true, + }), + ) + expect(mockLabelsApi.getLabels).toHaveBeenCalledWith( + 'board-1', + expect.objectContaining({ + signal: expect.any(AbortSignal), + timeout: BOARD_REQUEST_TIMEOUT_MS, + skipRetry: true, + }), + ) expect(state.currentBoard.value).toEqual(boardDetail) expect(state.currentBoardCards.value).toEqual(cards) expect(state.currentBoardLabels.value).toEqual(labels) @@ -308,6 +334,9 @@ describe('boardCrudStore', () => { }) it('handles error and rethrows', async () => { + state.currentBoard.value = { id: 'existing', name: 'Existing' } + state.currentBoardCards.value = [{ id: 'existing-card' }] + state.currentBoardLabels.value = [{ id: 'existing-label' }] mockBoardsApi.getBoard.mockRejectedValueOnce(new Error('not found')) const { fetchBoard } = createBoardCrudActions(state as any, helpers as any) @@ -317,6 +346,89 @@ describe('boardCrudStore', () => { expect.any(Error), 'Failed to fetch board', ) + expect(state.currentBoard.value).toEqual({ id: 'existing', name: 'Existing' }) + expect(state.currentBoardCards.value).toEqual([{ id: 'existing-card' }]) + expect(state.currentBoardLabels.value).toEqual([{ id: 'existing-label' }]) + expect(state.loading.value).toBe(false) + }) + + it('aborts still-pending siblings when one board read fails', async () => { + let cardsAborted = false + let labelsAborted = false + const abortable = (onAbort: () => void) => + (_id: string, _params: unknown, options: { signal: AbortSignal }) => + new Promise((_resolve, reject) => { + options.signal.addEventListener('abort', () => { + onAbort() + reject(new axios.CanceledError('sibling aborted')) + }) + }) + + mockBoardsApi.getBoard.mockRejectedValueOnce(new Error('board failed')) + mockCardsApi.getCards.mockImplementationOnce(abortable(() => (cardsAborted = true))) + mockLabelsApi.getLabels.mockImplementationOnce( + (_id: string, options: { signal: AbortSignal }) => + new Promise((_resolve, reject) => { + options.signal.addEventListener('abort', () => { + labelsAborted = true + reject(new axios.CanceledError('sibling aborted')) + }) + }), + ) + + const { fetchBoard } = createBoardCrudActions(state as any, helpers as any) + await expect(fetchBoard('board-1')).rejects.toThrow('board failed') + + expect(cardsAborted).toBe(true) + expect(labelsAborted).toBe(true) + expect(state.loading.value).toBe(false) + }) + + it('aborts the previous generation before starting a new board read', async () => { + let oldCardsAborted = false + let oldLabelsAborted = false + let boardCall = 0 + mockBoardsApi.getBoard.mockImplementation((_id: string, options: { signal: AbortSignal }) => { + boardCall += 1 + if (boardCall === 1) { + return new Promise((_resolve, reject) => { + options.signal.addEventListener('abort', () => reject(new axios.CanceledError('stale board'))) + }) + } + return Promise.resolve({ id: 'board-2', name: 'Board 2', columns: [] }) + }) + mockCardsApi.getCards.mockImplementation((_id: string, _params: unknown, options: { signal: AbortSignal }) => { + if (boardCall === 1) { + return new Promise((_resolve, reject) => { + options.signal.addEventListener('abort', () => { + oldCardsAborted = true + reject(new axios.CanceledError('stale cards')) + }) + }) + } + return Promise.resolve([]) + }) + mockLabelsApi.getLabels.mockImplementation((_id: string, options: { signal: AbortSignal }) => { + if (boardCall === 1) { + return new Promise((_resolve, reject) => { + options.signal.addEventListener('abort', () => { + oldLabelsAborted = true + reject(new axios.CanceledError('stale labels')) + }) + }) + } + return Promise.resolve([]) + }) + + const { fetchBoard } = createBoardCrudActions(state as any, helpers as any) + const first = fetchBoard('board-1') + const second = fetchBoard('board-2') + + await expect(second).resolves.toBe(true) + await expect(first).resolves.toBe(false) + expect(oldCardsAborted).toBe(true) + expect(oldLabelsAborted).toBe(true) + expect(state.currentBoard.value).toMatchObject({ id: 'board-2' }) expect(state.loading.value).toBe(false) }) diff --git a/frontend/taskdeck-web/src/tests/views/paper/review/PaperReviewView.spec.ts b/frontend/taskdeck-web/src/tests/views/paper/review/PaperReviewView.spec.ts index 3c8bee766..1e2cee2bc 100644 --- a/frontend/taskdeck-web/src/tests/views/paper/review/PaperReviewView.spec.ts +++ b/frontend/taskdeck-web/src/tests/views/paper/review/PaperReviewView.spec.ts @@ -3355,10 +3355,9 @@ describe('PaperReviewView', () => { const wrapper = await mountView([makeProposal({ id: 'pending' })]) expect(wrapper.find('[data-testid="paper-review-mini-cadence"]').exists()).toBe(false) - // The rail still shows the honest apply-rate empty state beneath the heading. - expect(wrapper.find('[data-testid="paper-review-apply-rate-empty"]').text()).toBe( - 'No decisions yet', - ) + expect(wrapper.find('[data-testid="paper-review-apply-rate"]').exists()).toBe(false) + expect(wrapper.find('[data-testid="paper-review-apply-rate-empty"]').exists()).toBe(false) + expect(wrapper.text()).not.toContain('No decisions yet') }) it('scopes the cadence to the active board filter', async () => { diff --git a/frontend/taskdeck-web/src/tests/views/paper/review/ReviewQueueRail.spec.ts b/frontend/taskdeck-web/src/tests/views/paper/review/ReviewQueueRail.spec.ts index 3c41bc493..3f0d1a9e0 100644 --- a/frontend/taskdeck-web/src/tests/views/paper/review/ReviewQueueRail.spec.ts +++ b/frontend/taskdeck-web/src/tests/views/paper/review/ReviewQueueRail.spec.ts @@ -28,7 +28,6 @@ function mountRail(props?: Partial<{ busy: boolean batchSelectedCount: number batchExecutableCount: number - applyRate: number cadence: number[] scopeLabel: string scopeClearLabel: string @@ -44,7 +43,6 @@ function mountRail(props?: Partial<{ batchSelectedCount: props?.batchSelectedCount ?? 0, batchExecutableCount: props?.batchExecutableCount ?? 0, recentlyApplied: props?.recentlyApplied ?? [], - ...(props?.applyRate !== undefined ? { applyRate: props.applyRate } : {}), ...(props?.cadence !== undefined ? { cadence: props.cadence } : {}), ...(props?.scopeLabel ? { scopeLabel: props.scopeLabel, scopeClearLabel: props.scopeClearLabel } : {}), }, @@ -245,40 +243,15 @@ describe('ReviewQueueRail', () => { expect(wrapper.emitted('select')?.at(-1)).toEqual(['recent-2']) }) - describe('This week apply-rate stat', () => { - it('shows the empty state and never a fabricated percentage when no apply rate is provided', () => { - // A fresh account with zero decision history: the rail must not invent a - // statistic. This is the mutation guard for the old `applyRate: 0.71` - // default — restoring it would render "71%" and hide the empty state, - // failing both assertions below. - const wrapper = mountRail() - const empty = wrapper.find('[data-testid="paper-review-apply-rate-empty"]') - expect(empty.exists()).toBe(true) - expect(empty.text()).toBe('No decisions yet') - expect(wrapper.find('[data-testid="paper-review-apply-rate"]').exists()).toBe(false) - expect(wrapper.text()).not.toContain('Apply rate') - expect(wrapper.text()).not.toContain('71%') - expect(wrapper.text()).not.toMatch(/\d+%/) - }) - - it('renders the real apply rate as a rounded percentage when provided', () => { - const wrapper = mountRail({ applyRate: 0.5 }) - const stat = wrapper.find('[data-testid="paper-review-apply-rate"]') - expect(stat.exists()).toBe(true) - expect(stat.text()).toContain('Apply rate') - expect(stat.text()).toContain('50%') - expect(wrapper.find('[data-testid="paper-review-apply-rate-empty"]').exists()).toBe(false) - }) + describe('This week cadence', () => { + it('renders real cadence without an apply-rate metric or empty claim', () => { + const wrapper = mountRail({ cadence: [1, 2, 3, 4, 5, 6, 7] }) - it('renders a real apply rate of zero as 0%, not the empty state', () => { - // 0 is a real decision-history value (nothing applied yet), distinct from - // "no history at all" — it must render honestly, not fall through to the - // empty state or a fabricated default. - const wrapper = mountRail({ applyRate: 0 }) - const stat = wrapper.find('[data-testid="paper-review-apply-rate"]') - expect(stat.exists()).toBe(true) - expect(stat.text()).toContain('0%') + expect(wrapper.findAll('.paper-review-cadence__bar')).toHaveLength(7) + expect(wrapper.find('[data-testid="paper-review-apply-rate"]').exists()).toBe(false) expect(wrapper.find('[data-testid="paper-review-apply-rate-empty"]').exists()).toBe(false) + expect(wrapper.text()).not.toContain('Apply rate') + expect(wrapper.text()).not.toContain('No decisions yet') }) it('hides the mini-cadence bars when no real cadence data is provided', () => { diff --git a/frontend/taskdeck-web/src/views/paper/review/ReviewQueueRail.vue b/frontend/taskdeck-web/src/views/paper/review/ReviewQueueRail.vue index 1ae6042db..575c19eb8 100644 --- a/frontend/taskdeck-web/src/views/paper/review/ReviewQueueRail.vue +++ b/frontend/taskdeck-web/src/views/paper/review/ReviewQueueRail.vue @@ -49,12 +49,6 @@ const props = withDefaults( * decision history — the mini-cadence bars are hidden rather than invented. */ cadence?: number[] - /** - * Real apply rate (0–1) derived from decision history. Omit when there is - * no decision history so the rail shows an honest empty state instead of a - * fabricated percentage. There is intentionally no default value. - */ - applyRate?: number /** * Whether the author partition — the "All" vs "Mine" split — can mean * anything for this viewer. It comes from the server-computed @@ -100,24 +94,11 @@ const visible = computed(() => { } }) -function asPct(value: number): string { - return `${Math.round(value * 100)}%` -} - /** Real 7-day cadence to render; null hides the mini-cadence bars entirely. */ const hasCadence = computed( () => Array.isArray(props.cadence) && props.cadence.length > 0, ) -/** - * Formatted apply-rate percentage, or null when there is no decision history. - * Never falls back to a fabricated constant — an absent value renders the - * "No decisions yet" empty state instead of an invented percentage. - */ -const applyRatePct = computed(() => - typeof props.applyRate === 'number' ? asPct(props.applyRate) : null, -) - function setFilter(next: QueueFilter) { filter.value = next emit('filter-change', next) @@ -281,20 +262,6 @@ function onFilterPillClick(key: QueueFilter) { {{ $t('review.queueRail.cadence.heading') }} -
- {{ $t('review.queueRail.cadence.applyRateLabel') }} {{ applyRatePct }} -
-
- {{ $t('review.queueRail.cadence.applyRateEmpty') }} -
@@ -433,12 +400,4 @@ function onFilterPillClick(key: QueueFilter) { .paper-review-rail__cadence-heading { margin-bottom: 6px; } -.paper-review-rail__cadence-meta { - font-size: 10px; - margin-top: 6px; -} -.paper-review-rail__cadence-meta b { - color: var(--ink); - font-weight: 500; -} diff --git a/scripts/ci/smart-ci/lib/plan.mjs b/scripts/ci/smart-ci/lib/plan.mjs index eb94c1b85..1769d8493 100644 --- a/scripts/ci/smart-ci/lib/plan.mjs +++ b/scripts/ci/smart-ci/lib/plan.mjs @@ -449,14 +449,23 @@ export function evaluateGate(plan, context) { const mode = requestedMode ?? planMode ?? 'shadow'; const failures = []; const notes = []; + const shadowPlanCancellation = mode === 'shadow' && context.planJobResult === 'cancelled'; + const recordShadowPlanCancellation = () => { + failures.push({ code: 'plan-job-cancelled', detail: 'plan job was cancelled by a superseding control-plane run' }); + notes.push('superseded plan cancellation is excluded from observation evidence and remains blocking in enforce mode'); + }; if (plan && requestedMode && planMode && requestedMode !== planMode) failures.push({ code: 'mode-mismatch', detail: `gate invoked in ${requestedMode} mode but the plan was produced under policy mode ${planMode}` }); if (!plan) { - failures.push({ code: 'plan-missing', detail: 'no plan receipt was produced' }); + if (shadowPlanCancellation) recordShadowPlanCancellation(); + else failures.push({ code: 'plan-missing', detail: 'no plan receipt was produced' }); } else { const planErrors = validatePlan(plan, context.policy ?? null); if (planErrors.length > 0) failures.push({ code: 'plan-invalid', detail: planErrors.join('; ') }); if (plan.plannerError) failures.push({ code: 'planner-error', detail: `${plan.plannerError.name}: ${plan.plannerError.message}` }); - if (context.planJobResult && context.planJobResult !== 'success') failures.push({ code: 'plan-job-failed', detail: `plan job result: ${context.planJobResult}` }); + if (context.planJobResult && context.planJobResult !== 'success') { + if (shadowPlanCancellation) recordShadowPlanCancellation(); + else failures.push({ code: 'plan-job-failed', detail: `plan job result: ${context.planJobResult}` }); + } if (context.expectedHeadSha && plan.headSha !== context.expectedHeadSha) failures.push({ code: 'head-sha-mismatch', detail: `plan ${plan.headSha} vs event ${context.expectedHeadSha}` }); if (context.expectedBaseSha && plan.baseSha !== context.expectedBaseSha) failures.push({ code: 'base-sha-mismatch', detail: `plan ${plan.baseSha} vs event ${context.expectedBaseSha}` }); if (context.expectedPolicyDigest && plan.policyDigest !== context.expectedPolicyDigest) failures.push({ code: 'policy-digest-mismatch', detail: `plan ${plan.policyDigest} vs policy ${context.expectedPolicyDigest}` }); diff --git a/scripts/ci/smart-ci/plan.test.mjs b/scripts/ci/smart-ci/plan.test.mjs index 45f3e3471..0e678c0bf 100644 --- a/scripts/ci/smart-ci/plan.test.mjs +++ b/scripts/ci/smart-ci/plan.test.mjs @@ -25,6 +25,7 @@ import { observeMergeRef } from './resolve-merge-ref.mjs'; const policyText = readFileSync(new URL('../../../ci/policy.v1.json', import.meta.url), 'utf8'); const policy = JSON.parse(policyText); +const runReceiptSchema = JSON.parse(readFileSync(new URL('../../../ci/schemas/ci-run.v1.schema.json', import.meta.url), 'utf8')); const digest = policyDigest(policyText); const BASE = 'a'.repeat(40); const HEAD = 'b'.repeat(40); @@ -234,6 +235,39 @@ test('the gate fails closed on missing, mismatched or errored plans in both mode assert.equal(planJobFailed.ok, false); }); +test('a superseded cancelled plan is non-red only in shadow mode', () => { + const plan = buildPlan(ownerInput(['docs/x.md']), policy, digest); + + const shadow = evaluateGate(plan, { mode: 'shadow', planJobResult: 'cancelled' }); + assert.equal(shadow.ok, true); + assert.equal(shadow.wouldFail, true); + assert.deepEqual(shadow.plannerFailures, []); + assert.ok(shadow.failures.some((failure) => failure.code === 'plan-job-cancelled')); + const receiptFailureCodes = new Set(runReceiptSchema.properties.failures.items.properties.code.enum); + assert.ok(shadow.failures.every((failure) => receiptFailureCodes.has(failure.code)), 'every emitted failure code must satisfy ci-run.v1'); + assert.ok(shadow.notes.some((note) => note.includes('superseded'))); + const summary = renderGateSummary(shadow, plan); + assert.match(summary, /would FAIL in enforce mode/); + assert.match(summary, /superseded plan cancellation is excluded from observation evidence/); + + const cancelledBeforeReceipt = evaluateGate(null, { mode: 'shadow', planJobResult: 'cancelled' }); + assert.equal(cancelledBeforeReceipt.ok, true); + assert.equal(cancelledBeforeReceipt.wouldFail, true); + assert.deepEqual(cancelledBeforeReceipt.plannerFailures, []); + assert.ok(cancelledBeforeReceipt.failures.some((failure) => failure.code === 'plan-job-cancelled')); + + const enforcePlan = { ...plan, mode: 'enforce' }; + const enforce = evaluateGate(enforcePlan, { mode: 'enforce', planJobResult: 'cancelled', results: {} }); + assert.equal(enforce.ok, false); + assert.ok(enforce.failures.some((failure) => failure.code === 'plan-job-failed')); + + for (const planJobResult of ['failure', 'timed_out', 'skipped']) { + const ordinaryFailure = evaluateGate(plan, { mode: 'shadow', planJobResult }); + assert.equal(ordinaryFailure.ok, false, `${planJobResult} must stay red in shadow mode`); + assert.ok(ordinaryFailure.failures.some((failure) => failure.code === 'plan-job-failed')); + } +}); + test('the gate distinguishes shadow (report) from enforce (block) for job evidence', () => { const plan = buildPlan(ownerInput(['docs/x.md']), policy, digest); const noEvidence = evaluateGate(plan, { mode: 'shadow', expectedHeadSha: HEAD, expectedBaseSha: BASE, expectedPolicyDigest: digest, planJobResult: 'success' });