diff --git a/apps/backend/internal/task/repository/sqlite/task_status_summary_projector_noop_test.go b/apps/backend/internal/task/repository/sqlite/task_status_summary_projector_noop_test.go new file mode 100644 index 0000000000..d8afcda76b --- /dev/null +++ b/apps/backend/internal/task/repository/sqlite/task_status_summary_projector_noop_test.go @@ -0,0 +1,120 @@ +package sqlite + +import ( + "context" + "testing" + "time" + + "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/events" + "github.com/kandev/kandev/internal/events/bus" + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/task/statussummary" +) + +// @covers AC-PLATFORM-BOUNDED-TASK-STATUS-DELIVERY-001.2 +// @covers AC-PLATFORM-BOUNDED-TASK-STATUS-DELIVERY-001.3 +func TestTaskStatusSummaryProjectorPRNoopWithRetainedError(t *testing.T) { + repo, db := newTaskStatusSummaryTestRepo(t) + seedTaskForStatusSummary(t, db, "task-summary-pr-noop", "workspace-summary-pr-noop") + eventBus := bus.NewMemoryEventBus(logger.Default()) + t.Cleanup(func() { eventBus.Close() }) + publicationCount := 0 + if _, err := eventBus.Subscribe(events.TaskStatusSummaryUpdated, func(context.Context, *bus.Event) error { + publicationCount++ + return nil + }); err != nil { + t.Fatalf("subscribe to summary updates: %v", err) + } + + loader := func(context.Context, string) (statussummary.SessionObservationSnapshot, error) { + return statussummary.SessionObservationSnapshot{ + ErrorsObserved: true, + Sessions: []statussummary.RebuildSession{{ + ID: "session-pr-noop", State: "WAITING_FOR_INPUT", IsPrimary: true, + ActiveError: &statussummary.ActiveErrorSummary{ + Scope: models.ErrorScopeSession, SessionID: "session-pr-noop", + Stamp: "retained-error", OccurredAt: time.Date(2026, 9, 17, 12, 0, 0, 0, time.UTC), + Preview: "agent failed", + }, + }}, + }, nil + } + newProjector := func() *statussummary.Projector { + return statussummary.NewProjector(statussummary.ProjectorConfig{ + Store: repo, EventBus: eventBus, LoadSessionObservations: loader, + Now: func() time.Time { return time.Date(2026, 9, 17, 12, 1, 0, 0, time.UTC) }, + }) + } + prEvent := func(checksState string, pendingReviewCount int) *bus.Event { + return bus.NewEvent(events.GitHubTaskPRUpdated, "test", map[string]interface{}{ + "task_id": "task-summary-pr-noop", "workspace_id": "workspace-summary-pr-noop", + "repository_id": "repo-a", "state": "open", "pr_number": 41, + "pr_url": "https://example.test/41", "checks_state": checksState, + "pending_review_count": pendingReviewCount, + }) + } + + projector := newProjector() + if err := projector.HandleEvent(context.Background(), prEvent("failure", 1)); err != nil { + t.Fatalf("initial PR projection: %v", err) + } + first, err := repo.LoadTaskStatusSummaries(context.Background(), []string{"task-summary-pr-noop"}) + if err != nil { + t.Fatalf("load initial summary: %v", err) + } + if err := projector.HandleEvent(context.Background(), prEvent("failure", 2)); err != nil { + t.Fatalf("aggregate-preserving PR update: %v", err) + } + second, err := repo.LoadTaskStatusSummaries(context.Background(), []string{"task-summary-pr-noop"}) + if err != nil { + t.Fatalf("load no-op summary: %v", err) + } + if second["task-summary-pr-noop"].Revision != first["task-summary-pr-noop"].Revision { + t.Fatalf("no-op changed revision from %d to %d", first["task-summary-pr-noop"].Revision, second["task-summary-pr-noop"].Revision) + } + if publicationCount != 1 { + t.Fatalf("no-op publication count = %d, want initial publication only", publicationCount) + } + + recreated := newProjector() + if err := recreated.HandleEvent(context.Background(), prEvent("failure", 3)); err != nil { + t.Fatalf("recreated projector no-op: %v", err) + } + third, err := repo.LoadTaskStatusSummaries(context.Background(), []string{"task-summary-pr-noop"}) + if err != nil { + t.Fatalf("load recreated no-op summary: %v", err) + } + if third["task-summary-pr-noop"].Revision != first["task-summary-pr-noop"].Revision { + t.Fatalf("recreated no-op changed revision to %d", third["task-summary-pr-noop"].Revision) + } + if publicationCount != 1 { + t.Fatalf("recreated no-op publication count = %d, want 1", publicationCount) + } + + if err := recreated.HandleEvent(context.Background(), prEvent("success", 0)); err != nil { + t.Fatalf("real PR aggregate change: %v", err) + } + fourth, err := repo.LoadTaskStatusSummaries(context.Background(), []string{"task-summary-pr-noop"}) + if err != nil { + t.Fatalf("load changed summary: %v", err) + } + if fourth["task-summary-pr-noop"].Revision != first["task-summary-pr-noop"].Revision+1 { + t.Fatalf("real change revision = %d, want %d", fourth["task-summary-pr-noop"].Revision, first["task-summary-pr-noop"].Revision+1) + } + if fourth["task-summary-pr-noop"].PullRequest == nil || fourth["task-summary-pr-noop"].PullRequest.AggregateState != "passing" { + t.Fatalf("real change PR summary = %+v, want passing aggregate", fourth["task-summary-pr-noop"].PullRequest) + } + if fourth["task-summary-pr-noop"].ActiveError == nil || fourth["task-summary-pr-noop"].ActiveError.Stamp != "retained-error" { + t.Fatalf("real change dropped retained error: %+v", fourth["task-summary-pr-noop"].ActiveError) + } + if publicationCount != 2 { + t.Fatalf("real change publication count = %d, want 2", publicationCount) + } + if err := recreated.HandleEvent(context.Background(), prEvent("success", 0)); err != nil { + t.Fatalf("replayed real PR state: %v", err) + } + if publicationCount != 2 { + t.Fatalf("replayed real change publication count = %d, want 2", publicationCount) + } +} diff --git a/apps/backend/internal/task/statussummary/model.go b/apps/backend/internal/task/statussummary/model.go index 378312ba19..41e3ff5191 100644 --- a/apps/backend/internal/task/statussummary/model.go +++ b/apps/backend/internal/task/statussummary/model.go @@ -4,9 +4,9 @@ package statussummary import ( + "bytes" "encoding/json" "fmt" - "reflect" "slices" "time" "unicode/utf8" @@ -138,13 +138,12 @@ type StoredTaskStatusSummary struct { Summary TaskStatusSummary } -// SemanticEqual compares only fields that task consumers observe as status. +// SemanticEqual compares valid summaries using the canonical payload stored by +// the repository, excluding transport metadata and omitted zero values. func (s TaskStatusSummary) SemanticEqual(other TaskStatusSummary) bool { - s.Revision = 0 - s.UpdatedAt = time.Time{} - other.Revision = 0 - other.UpdatedAt = time.Time{} - return reflect.DeepEqual(s, other) + left, leftErr := s.SemanticJSON() + right, rightErr := other.SemanticJSON() + return leftErr == nil && rightErr == nil && bytes.Equal(left, right) } // Validate enforces the bounded fields at the persistence boundary. Other diff --git a/apps/backend/internal/task/statussummary/model_semantic_equality_test.go b/apps/backend/internal/task/statussummary/model_semantic_equality_test.go new file mode 100644 index 0000000000..07ad2b79cc --- /dev/null +++ b/apps/backend/internal/task/statussummary/model_semantic_equality_test.go @@ -0,0 +1,83 @@ +package statussummary + +import ( + "encoding/json" + "strings" + "testing" + "time" + + "github.com/kandev/kandev/internal/task/models" +) + +// @covers AC-PLATFORM-BOUNDED-TASK-STATUS-DELIVERY-001.2 +func TestTaskStatusSummarySemanticEqualityMatchesPersistence(t *testing.T) { + occurredAt := time.Date(2026, 9, 17, 12, 0, 0, 0, time.UTC) + base := TaskStatusSummary{ + Revision: 7, + UpdatedAt: occurredAt.Add(time.Minute), + ActiveError: &ActiveErrorSummary{ + Scope: models.ErrorScopeSession, + SessionID: "session-1", + Stamp: "error-1", + OccurredAt: occurredAt, + Preview: "agent failed", + RecoveryActions: nil, + Causes: nil, + }, + TaskError: &ActiveErrorSummary{ + Scope: models.ErrorScopeTask, + Stamp: "task-error-1", + OccurredAt: occurredAt, + Preview: "task failed", + RecoveryActions: nil, + Causes: nil, + }, + } + + withEmptyCollections := base + activeEmpty := *base.ActiveError + activeEmpty.RecoveryActions = []string{} + activeEmpty.Causes = []models.AgentErrorCause{} + taskEmpty := *base.TaskError + taskEmpty.RecoveryActions = []string{} + taskEmpty.Causes = []models.AgentErrorCause{} + withEmptyCollections.ActiveError = &activeEmpty + withEmptyCollections.TaskError = &taskEmpty + withEmptyCollections.Revision = 99 + withEmptyCollections.UpdatedAt = occurredAt.Add(2 * time.Hour) + if !base.SemanticEqual(withEmptyCollections) { + t.Fatalf("nil and empty error collections must compare equal: %#v vs %#v", base, withEmptyCollections) + } + + encoded, err := base.SemanticJSON() + if err != nil { + t.Fatalf("encode base summary: %v", err) + } + var roundTrip TaskStatusSummary + if err := unmarshalSummary(encoded, &roundTrip); err != nil { + t.Fatalf("decode canonical summary: %v", err) + } + if !base.SemanticEqual(roundTrip) { + t.Fatalf("canonical persistence round trip changed semantics: %#v", roundTrip) + } + + changed := base + changed.PendingAction = pendingPermission + if base.SemanticEqual(changed) { + t.Fatal("a real semantic field change must not compare equal") + } +} + +// @covers AC-PLATFORM-BOUNDED-TASK-STATUS-DELIVERY-001.2 +func TestTaskStatusSummarySemanticEqualityRejectsInvalidValues(t *testing.T) { + invalid := TaskStatusSummary{ + ActiveError: &ActiveErrorSummary{Preview: strings.Repeat("x", MaxActiveErrorPreviewBytes+1)}, + } + if invalid.SemanticEqual(invalid) { + t.Fatal("invalid summaries must not be accepted as semantic no-ops") + } +} + +func unmarshalSummary(data []byte, summary *TaskStatusSummary) error { + return json.Unmarshal(data, summary) +} diff --git a/apps/backend/internal/task/statussummary/projector_test.go b/apps/backend/internal/task/statussummary/projector_test.go index a6791a06d0..0baad333da 100644 --- a/apps/backend/internal/task/statussummary/projector_test.go +++ b/apps/backend/internal/task/statussummary/projector_test.go @@ -45,8 +45,11 @@ func (s *projectorTestStore) CompareAndUpdateTaskStatusSummary(_ context.Context } s.mu.Lock() defer s.mu.Unlock() - if previous := s.rows[stored.TaskID]; previous != nil && previous.Summary.Revision >= stored.Summary.Revision { - return false, nil + if previous := s.rows[stored.TaskID]; previous != nil { + if previous.Summary.Revision >= stored.Summary.Revision || + (previous.WorkspaceID == stored.WorkspaceID && previous.Summary.SemanticEqual(stored.Summary)) { + return false, nil + } } copy := *stored copy.Summary = *cloneSummary(&stored.Summary) diff --git a/docs/plans/task-summary-semantic-equality/plan.md b/docs/plans/task-summary-semantic-equality/plan.md new file mode 100644 index 0000000000..3148e16435 --- /dev/null +++ b/docs/plans/task-summary-semantic-equality/plan.md @@ -0,0 +1,199 @@ +--- +created: 2026-09-17 +status: complete +requirements: + - REQ-PLATFORM-BOUNDED-TASK-STATUS-DELIVERY-001 +system_design: + - ../../specs/platform/system-design/bounded-task-status-delivery.md +legacy_specs: [] +--- + +# Implementation Plan: Task-summary semantic equality + +## Overview + +Stop false compare-and-set exhaustion errors when a source event changes a +detail but leaves the bounded task summary unchanged. Align the projector's +in-memory equality with its existing persisted semantic JSON, then prove the +complete event-to-storage-to-publication path with a real SQLite repository. +One sequential work order owns the correction and its regression coverage. + +## Evidence and confirmed root cause + +Investigation baseline: checkout `f32eb3737`, 2026-09-17. Today's retained +backend files contained 31 `exhausted CAS retries refreshing pending task +status` warnings paired with `github.task_pr.updated` handler errors between +11:19 and 15:16 Lisbon time. At 15:16:47, the source changed review counts. +Read-only inspection of three affected task summaries found retained session +errors with omitted `recovery_actions` and `causes` fields. These database +reads describe inspection-time state, not a historical snapshot. + +The deterministic cause is independent of concurrent writers: + +1. `restoreSessionObservations` / `normalizeRebuildError` rebuild error + observations. `NormalizeRecoveryActionsForCategory` and + `NormalizeAgentErrorCauses` can produce non-nil zero-length slices. +2. `deriveActiveError` retains those slices. `cloneSummary` and database JSON + round trips omit them through `omitempty` and restore nil slices. +3. `TaskStatusSummary.SemanticEqual` uses `reflect.DeepEqual`, which treats + those in-memory representations as different. +4. A PR detail change, such as pending review count 1 to 2 while checks remain + failing, changes the keyed observation but not the aggregate summary. +5. The projector attempts a write. The repository correctly rejects it because + `SemanticJSON` is identical. Reload/rebase restores the same mismatch, so + all three retries fail and the handler returns an error. + +A temporary test named `TestReproSummaryPRNoopWithRetainedError` drove +`Projector.HandleEvent` with a real SQLite repository and synthetic data. It +used one waiting primary session with an explicitly session-scoped retained +error and two PR events with checks `failure`, changing pending review count +from 1 to 2. The second event returned the exact exhaustion error. The before +and after stored JSON were identical and both revisions were 1. No competing +writer, running agent, external provider, or browser was involved. + +The temporary reproduction was removed after investigation. Its permanent +success assertion belongs to Task 01's red phase. An initial fixture omitted +the error's session scope and therefore introduced a separate summary change; +the final fixture explicitly sets scope and session identity. + +This establishes false errors for aggregate-preserving events, not a confirmed +loss of a meaningful PR status change. Existing projector tests missed it +because `projectorTestStore` rejects old revisions but accepts identical +semantic payloads with newer revisions, unlike the production repository. + +## Requirement conformance and assumptions + +Platform owns the bounded projection contract; Tasks and Integrations retain +the authoritative source records. This is an implementation defect under +[AC-PLATFORM-BOUNDED-TASK-STATUS-DELIVERY-001.2 and .3](../../specs/platform/requirements/bounded-task-status-delivery.md) +and the design's existing semantic-no-op and persistence guarantees. Reuse the +active requirement without adding a repair-specific requirement or new IDs. + +The intended behavior is settled: unchanged projections succeed silently; +real changes preserve monotonic revisions and workspace-scoped publication. +No new product behavior, schema, ownership boundary, or ADR is needed. The +design clarifies the shared equality rule. The accepted +[summary boundary ADR](../../decisions/2026-08-01-separate-task-summary-session-stream-traffic.md) +and [activity ADR](../../decisions/2026-08-17-separate-task-activity-from-summary-freshness.md) +remain authoritative. + +## Scope + +### In scope + +- Consistent equality for every valid bounded summary and its persisted form. +- Nil/empty error collections and JSON round-trip equivalence. +- Real SQLite projector regression, publication assertions, and faithful test + store no-op behavior. +- Preservation of genuine changes, validation failures, and contention errors. + +### Out of scope + +- Workflow import validation/input repair, task-deletion races, diff requests + during teardown, authentication settings, and agent-provider errors. +- Changes to error normalization throughout the model domain or error text. +- Retry-count increases, suppressing all CAS warnings, database rewrites, + schema migrations, new flags, and PR polling changes. +- Frontend layout, copy, interaction, and status precedence changes. + +## Technical approach + +In `apps/backend/internal/task/statussummary/model.go`, implement valid-value +equality using the canonical payload already provided by `SemanticJSON`. +Compare successful encodings with `bytes.Equal`; if either encoding fails, +return false so the projector continues into its existing validation/error +path. Do not equate two encoding failures by comparing nil byte slices. +This reuses the bounded persistence contract and automatically includes future +semantic fields. Do not change the JSON shape or broaden timestamp equivalence +beyond the existing serialized representation. + +Keep `persistAndPublishLocked`, its reload/rebase behavior, and the repository +SQL contract intact unless the regression demonstrates a necessary local +adjustment. Correct `projectorTestStore.CompareAndUpdateTaskStatusSummary` in +`projector_test.go` to reject an unchanged workspace and semantic payload as +well as stale revisions. Do not relax unrelated test assertions to compensate. + +Add focused model tests in `model_semantic_equality_test.go` and an integration +test in `internal/task/repository/sqlite/task_status_summary_projector_noop_test.go`. +Use the existing temporary-database helpers, synthetic identities, explicit +session error scope, fixed UTC times, and synchronous event handling. +The fixture must not open the developer's database or copy private log data. + +## Tests + +| Evidence | Acceptance mapping | +| --- | --- | +| `TestTaskStatusSummarySemanticEqualityMatchesPersistence`: nil/empty collections independently for active/task errors, JSON round trips, timestamp monotonic metadata, ignored revision/update time, and real semantic differences | AC-PLATFORM-BOUNDED-TASK-STATUS-DELIVERY-001.2 | +| `TestTaskStatusSummarySemanticEqualityRejectsInvalidValues`: invalid summaries cannot be classified as successful no-ops | AC-PLATFORM-BOUNDED-TASK-STATUS-DELIVERY-001.2 | +| `TestTaskStatusSummaryProjectorPRNoopWithRetainedError`: live and recreated projectors, real SQLite, unchanged payload/revision, zero extra publications, no exhaustion warning | AC-PLATFORM-BOUNDED-TASK-STATUS-DELIVERY-001.2, AC-PLATFORM-BOUNDED-TASK-STATUS-DELIVERY-001.3 | +| The real-change portion of `TestTaskStatusSummaryProjectorPRNoopWithRetainedError`: later aggregate change persists once and publishes the complete next revision while preserving error and unrelated fields | AC-PLATFORM-BOUNDED-TASK-STATUS-DELIVERY-001.2, AC-PLATFORM-BOUNDED-TASK-STATUS-DELIVERY-001.3 | +| Existing pending-authority and queue contention tests with the corrected test store | AC-PLATFORM-BOUNDED-TASK-STATUS-DELIVERY-001.2 | + +## E2E evidence + +The real-repository integration tests cover source event, normalization, +projection, SQL persistence/reload, and complete workspace-summary publication. +They exercise the shared payload consumed by desktop and mobile. No rendered +UI changes or new browser test are needed for this internal no-op correction. + +## Work orders + +- [x] [Task 01: Align semantic equality and prove persistence behavior](task-01-align-semantic-equality.md) + +Execution is sequential. No subagents are authorized or required. + +## Related implementation records + +The linked packages `bounded-task-status-delivery` (implemented), +`session-stream-overload-isolation` (complete), `backend-runtime-state-ownership` +(completed), and `deleted-session-error-summary` (done) were inventoried. +This repair does not reopen their work orders or alter their E2E matrices; +their historical verification results remain unchanged. + +## Verification results + +Investigation: the final temporary regression failed as expected with three +CAS retries, identical persisted JSON, and unchanged revision 1: + +```bash +(cd apps/backend && go test ./internal/task/repository/sqlite -run '^TestReproSummaryPRNoopWithRetainedError$' -count=1 -v) +``` + +Implementation and permanent regression verification passed on 2026-09-17: + +```text +(cd apps/backend && go test ./internal/task/statussummary ./internal/task/repository/sqlite \ + -run 'TestTaskStatusSummarySemanticEquality|TestTaskStatusSummaryProjectorPRNoopWithRetainedError' -count=1) + 2 packages passed. + +(cd apps/backend && go test -race ./internal/task/statussummary ./internal/task/repository/sqlite -count=1) + 2 packages passed. +``` + +The production equality path now uses the same canonical semantic JSON as the +repository. The SQLite projector regression proves no-op events keep the same +revision and publication count, recreated projectors converge, and a real PR +aggregate change advances once while preserving the retained error. +Design-package checks passed on 2026-09-17: + +- `python3 scripts/list-docs.py validate`: 285 decisions and 986 specifications. +- `python3 scripts/lint-spec-files.py --all`: all specification files passed. +- `python3 scripts/lint-spec-files.test.py`: 36 tests passed. +- `git diff --check`: passed. +- `git status --short -- docs/plans/task-summary-semantic-equality`: confirmed + the new package is present and untracked; it contains this plan and Task 01. + +The implementation is complete in Task 01. Commit and PR delivery evidence is +recorded outside this plan. + +## Risks + +- Comparing canonical JSON allocates bounded payloads; keep it on the existing + semantic-event path and do not introduce transcript or polling work. +- A faithful test store may expose pre-existing tests that rely on artificial + revision bumps. Preserve their behavioral intent and diagnose each failure. +- A no-op fix must not hide real field changes, invalid values, store failures, + or genuine contention exhaustion. +- The SQLite reproduction establishes this cause but does not prove every + historical CAS error had the same cause. No SQL changes are planned; if that + scope changes, add the repository-required PostgreSQL behavioral coverage. diff --git a/docs/plans/task-summary-semantic-equality/task-01-align-semantic-equality.md b/docs/plans/task-summary-semantic-equality/task-01-align-semantic-equality.md new file mode 100644 index 0000000000..f6e06d8029 --- /dev/null +++ b/docs/plans/task-summary-semantic-equality/task-01-align-semantic-equality.md @@ -0,0 +1,166 @@ +--- +id: "01-align-semantic-equality" +title: "Align semantic equality and prove persistence behavior" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-PLATFORM-BOUNDED-TASK-STATUS-DELIVERY-001 +acceptance_criteria: + - AC-PLATFORM-BOUNDED-TASK-STATUS-DELIVERY-001.2 + - AC-PLATFORM-BOUNDED-TASK-STATUS-DELIVERY-001.3 +system_design: + - ../../specs/platform/system-design/bounded-task-status-delivery.md +--- + +# Task 01: Align semantic equality and prove persistence behavior + +## Summary + +Make valid summary equality agree with persisted canonical JSON. Prove that a +PR detail change with an unchanged aggregate and a retained session error is +a successful no-op, while a later real change still persists and publishes. + +## In scope + +- Add model and real-SQLite regression tests before production edits. +- Replace representation-sensitive equality with successful canonical-payload + comparison, preserving the existing validation/error path. +- Match the test store's rejection semantics to the production repository. +- Record red/green command results and update the plan/work-order status. + +## Out of scope + +All exclusions in [the plan](plan.md#scope), especially repository SQL changes, +global normalization changes, UI work, log-level suppression, and retry tuning. + +## Acceptance + +1. Every tested valid summary compares equal to its canonical JSON round trip, + including nil/empty error actions and causes. Metadata-only differences + are equal; real semantic differences are unequal. Failed encodings are + never accepted as equality. +2. Against real SQLite, both live and recreated projectors process an + aggregate-preserving PR change with no handler error, exhaustion warning, + revision change, or extra summary publication. Preserve unrelated fields. +3. A subsequent true aggregate change advances exactly one revision and + publishes exactly one complete replacement with task/workspace identity. + Existing store-error and contention behavior stays observable and bounded. + +## TDD sequence and regression construction + +1. Mark this work order `in_progress`. Add + `TestTaskStatusSummaryProjectorPRNoopWithRetainedError` using + `newTaskStatusSummaryTestRepo` and `seedTaskForStatusSummary`. +2. Configure `LoadSessionObservations` with one primary waiting session, + `ErrorsObserved: true`, and an error with `Scope: "session"`, matching + `SessionID`, fixed UTC `OccurredAt`, stamp, and preview. Leave optional + collections absent so production normalization creates the relevant shape. +3. Call `Projector.HandleEvent` with `GitHubTaskPRUpdated`, fixed task/workspace + and repository identity, one open PR, `checks_state: "failure"`, and + `pending_review_count: 1`. Load the first summary, then send count 2. + The unchanged `failure` aggregate must succeed; current code instead returns + `exhausted CAS retries ... after 3 attempts`. Assert payload and revision + as well as event counts. Capture the projector logger for the warning check. +4. Include a recreated-projector case using the same temporary repository and + authoritative session/PR loaders. Keep loader keys aligned with event keys. + Register bus/logger/database cleanup immediately. No sleeps are needed. +5. Add model cases named in the plan. Independently exercise empty actions and + empty causes in both `ActiveError` and `TaskError`, plus valid populated + changes, removal, queue count, Git/PR state, and last activity changes. +6. Implement canonical equality and faithful fake-store semantics. Never + return equality when either `SemanticJSON` call fails. Do not swallow CAS + exhaustion or increase its attempt count. +7. In `TestTaskStatusSummaryProjectorPRNoopWithRetainedError`, after the + no-op cases, change aggregate status to a distinct state, assert one + write/event, then replay and assert no extra event. Verify the retained + error and complete replacement survive. Run the targeted packages and + record results. + +## Verification + +Run from the repository root. The first command is the red test before the +production edit and must be rerun green afterward. The second covers every +changed suite and the existing contention regressions. + +```bash +(cd apps/backend && go test ./internal/task/repository/sqlite -run '^TestTaskStatusSummaryProjectorPRNoopWithRetainedError$' -count=1 -v) +(cd apps/backend && go test -race ./internal/task/statussummary ./internal/task/repository/sqlite -count=1) +python3 scripts/list-docs.py validate +python3 scripts/lint-spec-files.py --all +git diff --check +git status --short -- docs/plans/task-summary-semantic-equality +``` + +Do not claim PostgreSQL was exercised by the SQLite suite. There is no SQL or +dialect change in this scope; if one becomes necessary, reconcile the package +and add the required real PostgreSQL check before implementing it. + +## Files likely touched + +- `apps/backend/internal/task/statussummary/model.go` +- `apps/backend/internal/task/statussummary/model_semantic_equality_test.go` (new) +- `apps/backend/internal/task/statussummary/projector_test.go` (test-store helper) +- `apps/backend/internal/task/repository/sqlite/task_status_summary_projector_noop_test.go` (new) +- This work order and `plan.md` (status and results). + +Read-only integration points: `projector.go`, `projector_events.go`, +`projector_derive.go`, `rebuild.go`, and `repository/sqlite/task_status_summary.go`. + +## Dependencies + +None. + +## Risks + +Invalid data must reach existing validation rather than being hidden by failed +serialization. The revised fake store can reveal tests that previously assumed +every higher revision was accepted. Diagnose those assumptions rather than +weakening the persistence contract. + +## Parallelism + +`sequential` + +## Inputs + +- [Plan and reproduction evidence](plan.md). +- [Requirements](../../specs/platform/requirements/bounded-task-status-delivery.md), AC .2 and .3. +- [Design](../../specs/platform/system-design/bounded-task-status-delivery.md), derivation and persistence guarantees. +- Existing `model_test.go`, `projector_pending_authority_test.go`, + `projector_queued_test.go`, and `task_status_summary_test.go` patterns. +- `.agents/skills/tdd/references/backend-tests.md`. + +## Results + +Implemented on 2026-09-17. + +- `TaskStatusSummary.SemanticEqual` compares successful canonical semantic JSON + encodings and rejects invalid summaries as equal. +- The projector test store now models both production rejection conditions: + stale revisions and unchanged workspace/semantic payloads. +- Added model coverage for nil/empty omitted collections, metadata-only changes, + real semantic changes, JSON round trips, and invalid values. +- Added a real SQLite projector regression covering live and recreated no-op PR + updates and one true aggregate change. + +Verification passed: + +```text +(cd apps/backend && go test ./internal/task/statussummary ./internal/task/repository/sqlite \ + -run 'TestTaskStatusSummarySemanticEquality|TestTaskStatusSummaryProjectorPRNoopWithRetainedError' -count=1) + 2 packages passed. + +(cd apps/backend && go test -race ./internal/task/statussummary ./internal/task/repository/sqlite -count=1) + 2 packages passed. + +python3 scripts/list-docs.py validate + Validated 285 decisions and 986 specifications. + +python3 scripts/lint-spec-files.py --all + All specification files passed. + +git diff --check + passed. +``` diff --git a/docs/specs/platform/system-design/bounded-task-status-delivery.md b/docs/specs/platform/system-design/bounded-task-status-delivery.md index 7fae2e7452..75781913c2 100644 --- a/docs/specs/platform/system-design/bounded-task-status-delivery.md +++ b/docs/specs/platform/system-design/bounded-task-status-delivery.md @@ -4,7 +4,7 @@ system: platform requirements: - REQ-PLATFORM-BOUNDED-TASK-STATUS-DELIVERY-001 created: 2026-08-01 -updated: 2026-08-29 +updated: 2026-09-17 owners: - kandev --- @@ -142,6 +142,12 @@ remain during migration, but switchers use the summary when present. Integration option updates refresh the authoritative keyed pull-request observations through the existing task-summary projector. - A semantic no-op does not increment `revision` or emit an update. +- For valid summaries, `TaskStatusSummary.SemanticEqual` uses the same + canonical semantic payload as `SemanticJSON` and the repository's no-op + comparison. Nil and empty omitted error collections are equivalent; + Go-only timestamp metadata does not create a change when serialized values + are identical. Revision and projection time remain excluded. Invalid values + must not short-circuit validation as successful no-ops. - Clients ignore a summary delta whose revision is not newer than the stored revision. - `last_activity_at` is separate from projection freshness. Task creation, @@ -189,6 +195,12 @@ remain during migration, but switchers use the summary when present. - Summary persistence and revision changes are serialized per task. A compare-and-update or equivalent transaction prevents concurrent source events from publishing duplicate revisions or losing a newer value. +- `CompareAndUpdateTaskStatusSummary` can return false for a stale revision + or an unchanged semantic payload. After reloading a rejected write, the + projector rebases authoritative observations and compares again using the + same payload semantics. An already-current value succeeds without another + write or publication; genuine contention retains the bounded retry and + error path. Test stores must preserve both repository rejection conditions. - Missing rows are rebuilt from authoritative records. List and boot loaders batch summary reads and may batch repairs; they do not perform an N+1 query. - Existing rows are repaired after startup recovery changes authoritative session state. Missing-row @@ -405,3 +417,5 @@ intermediate replacement. [`../../plans/backend-runtime-state-ownership/plan.md`](../../../plans/backend-runtime-state-ownership/plan.md) - Deleted-session error repair: [`../../plans/deleted-session-error-summary/plan.md`](../../../plans/deleted-session-error-summary/plan.md) +- Semantic no-op equality repair: + [Task-summary semantic equality](../../../plans/task-summary-semantic-equality/plan.md)