dreampiler/kandev
1
0
Fork 0
Code Issues 43 Pull Requests Actions 6 Packages Projects Releases Wiki Activity

fix: stabilize workflow move preview invalidation (#3787)

Browse Source
This commit is contained in:
Carlos Florêncio
2026-09-18 10:33:59 +01:00
committed by GitHub
parent 60ba2acddd
commit dd8216f542
12 changed files with 1711 additions and 146 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/plans/workflow-move-preview-stability/plan.md
+215
View File
@@ -0,0 +1,215 @@
---
created: 2026-09-17
status: complete
requirements:
- REQ-TASKS-WORKFLOW-MOVE-PREVIEW-001
- REQ-TASKS-WORKFLOW-MOVE-PREVIEW-002
system_design:
- ../../specs/tasks/system-design/workflow-move-preview.md
legacy_specs: []
---
# Implementation Plan: Stable workflow move previews
## Overview
Keep an open move preview stable when background updates cannot change its
prediction. Deliver one sequential work order covering semantic invalidation
and its desktop and touch regression evidence.
The tasks system owns this repair because it owns move prediction and recipient
selection. The existing requirement/design pair remains authoritative. This
package adds the missing stability criterion, AC-TASKS-WORKFLOW-MOVE-PREVIEW-001.8,
and refines the design without changing routing or execution policy.
Implementation followed the approved package in this work order. Existing .001.7
and .002.3 require real invalidation and prohibit stale predictions appearing
current. The correction retains results only when prediction inputs remain equal.
## Evidence and reproduction
`getWorkflowMovePreviewRevision` currently serializes complete task, session,
workflow, step, model, and profile objects. Any serialized metadata change
changes the request key. `useWorkflowMovePreview` then clears success, sets
loading, and requests again after 150 ms. This is an update-driven cycle;
the hook has no periodic polling timer.
The existing test `clears a successful result and fetches again when the
invalidation key changes` proves the reset path. The two focused hook suites
passed all eight tests on 2026-09-17. They lack a harmless-update regression.
No live trace established which event triggered the user's particular hover.
Smallest deterministic regression: resolve a preview, change only the task
description or session read cursor in the real revision input, and rerender.
Advance fake timers beyond 150 ms. Require unchanged success and one request.
Current code changes the revision, clears success, and issues a second request.
## Scope
### In scope
- Explicit semantic projections in the shared revision hook.
- Harmless-update stability in active and background task/session projections.
- Positive invalidation for routing, model, options, connection, and context changes.
- Session candidate-order semantics, mixed eligible/terminal inventories, and ties.
- Existing hover, keyboard, touch drawer, and next-step footer consumers.
### Out of scope
- Backend routing changes, new APIs, persistence, or TTL caches.
- Keeping known-stale results visible as current during a genuine refresh.
- Layout redesign, new copy, provider probes, and new phone navigation.
- Diagnosing unrelated timers or changing task/session store ownership.
## Technical approach
Follow [Semantic revision inputs](../../specs/tasks/system-design/workflow-move-preview.md#semantic-revision-inputs).
Replace whole-object serialization in
`apps/web/hooks/domains/kanban/use-workflow-move-preview-revision.ts` with typed
projections and deterministic serialization. Reuse existing configuration
helpers where available; do not create a generic cache framework.
Read `workflow_move_preview.go`, `workflow_move_preview_settings.go`, and the
session target helpers to verify each selected input. Session `updated_at`
cannot simply be dropped: it ranks reusable candidates. Represent meaningful
ordering without key churn when the winner/order remains unchanged. Preserve
backend tie behavior and original-session provenance.
Keep the 150 ms debounce, two-request limit, in-flight sharing, abort/cleanup,
retry, close/reopen, and late-generation guards in `use-workflow-move-preview.ts`.
Change that hook only if the composed regression identifies an additional
lifecycle defect. No stale-while-refreshing state or renderer rewrite is needed.
All existing profile records may remain candidates for projection to avoid
missing dynamic dependencies, but project only fields used by prediction.
Do not include `agentProfiles.version` as an independent refresh trigger.
Track source, destination, and an explicit referenced target step when available.
## ASCII UI preview
UI-01: Desktop hover/focus disclosure, resolved result plus harmless update.
AC-TASKS-WORKFLOW-MOVE-PREVIEW-001.8 and -002.1, .3.
```text
Current cycle: result -> Checking session... -> result
After: result -----------------------> same result
+-----------------------------+
| Move here |
| Options |
| [step capabilities] |
|-----------------------------|
| Reuse current session |
| Astra (i) |
+-----------------------------+
```
UI-02: Coarse-pointer drawer, opened by tapping the existing step trigger.
AC-TASKS-WORKFLOW-MOVE-PREVIEW-001.8 and -002.2, .3.
```text
+--------------------------------+
| Move to x | fixed header
|--------------------------------|
| Implement |
| [Options] [Move here] |
| Reuse current session | existing row alignment
| Astra | stable during harmless updates
| [expanded details if selected] |
|________________________________| safe-area clearance
```
Existing compact rows keep their shipped alignment; this repair does not apply
the centered desktop footer layout to drawer rows. Drawer content retains one
scroll owner and 44px touch actions. Nearest shipped surface is
`CompactWorkflowStepDisclosure`; the curated MobilePickerSheet pattern supplies
the temporary-choice drawer baseline. Full phone task routes keep their
existing composition; no desktop stepper is added to `SessionMobileLayout`.
UI-03: First open or meaningful input change still shows `Checking session...`.
A failed refresh shows `Preview unavailable` with `Retry`. Neither state alone
disables Move. Closing clears the result; reopening requests a fresh one.
These states also apply to the next-step options footer and touch rows.
Control order and stable feedback are structural requirements; names and
spacing are illustrative. Existing localization and accessibility remain intact.
## Tests
Add these named regressions to the existing suites; use deferred responses and
fake timers to assert both request count and status transitions.
| Criteria | File and planned tests |
| --- | --- |
| 001.8 | `use-workflow-move-preview-revision.test.ts`: `ignores non-predictive task and session updates`; `normalizes equivalent map key order`; cover each store source and missing initial-target profile provenance |
| 001.7, .8 | Same file: `tracks relevant configuration and routing changes`; `tracks candidate order without timestamp churn`; include ties, terminal and completion-follow-up candidates, primary fallback, and original provenance |
| 001.7, .8; 002.3 | `use-workflow-move-preview.test.ts`: `keeps success and request count stable across harmless store updates`; compose the real revision function with the request hook |
| 001.7; 002.3 | Same file: `refreshes after a predictive change amid harmless updates`; delayed stale response, reconnect, close/reopen, retry, two-request queue |
| 002.1-.4 | Existing renderer, stepper, and proceed-button component suites; retain loading/error/details and shared footer behavior |
Negative fixtures change task descriptions/titles, status-summary timestamps,
command counts, read cursors, unrelated metadata, and global profile version.
Positive fixtures change model/options, session name/profile/state/membership,
source end policy, destination start policy/target/actions, workflow default
profile, task profile fallback, reconnect, and workspace generation. Pair every
removed broad input with evidence that its meaningful fields remain observed.
## E2E tests
Extend `apps/web/e2e/tests/workflow/workflow-move-preview.spec.ts` in `chromium`:
hold one hover open after success, apply repeated harmless updates through the
existing `__KANDEV_E2E_STORE__` bridge, and assert no new POST or loading flash.
Observe the result during the updates, not only after it settles. Use the shared
negative-assertion dwell observer for the no-refresh window. Count requests by
task AND destination. Then change a real model with `apiClient.setSessionModel`,
hold the preview response, assert loading, and release it to verify the new model.
Do not mock the revision hook or add a production diagnostic endpoint.
Extend `mobile-workflow-move-preview.spec.ts` in `mobile-chrome` with the same
stability assertion in its existing `tabletTestPage` drawer. This is the shipped
coarse-pointer surface; the full Pixel 5 task route has no desktop stepper.
Keep Options/details open during updates and preserve the existing focus-return,
44px target, and overflow checks. Restore shared user settings in cleanup.
The shared hook regression covers the phone next-step footer without introducing
a new phone navigation surface. Preserve existing move-execution scenarios.
## Work orders
- [x] [Task 01: Stabilize preview invalidation](task-01-stabilize-invalidation.md)
One work order, sequential. No implementation subagents are authorized.
The [original delivery plan](../workflow-move-preview/plan.md) retains its
historical completion results; this repair owns its new tests and results.
## Verification results
Implementation completed on 2026-09-18. The focused unit suite passed 74 tests
across five files, including 9 revision tests covering the review remediation.
Typecheck and changed-file ESLint passed. The final desktop
Chromium run passed 3 tests, and the final mobile-chrome run passed 1 test.
Both browser flows counted one initial preview request, kept the resolved result
and selected details visible across three harmless updates without a loading
state, then observed request two and loading for a genuine model change before
the refreshed model appeared. The browser checks also passed existing move,
focus-return, 44px target, row-alignment, and overflow assertions.
`pnpm run build:e2e` passed after the final source changes. Package validation
passed on 2026-09-18:
- `python3 scripts/list-docs.py validate`: 286 decisions and 998 specifications.
- `python3 scripts/lint-spec-files.test.py`: 36 tests passed.
- `python3 scripts/lint-spec-files.py --all`: passed.
- Package links, acceptance references, and frontend command paths: passed.
- `git diff --check`: passed; scoped status confirms both new package files and
the updated requirements, design, and delivery-plan documents.
No backend policy, API, cache, or layout changes were made.
## Risks
- Over-narrow projection can hide a model, metadata, or recipient change.
- Raw timestamps cause churn; dropping selection order can leave stale recipients.
- Reordered rule arrays are meaningful even when object key order is not.
- Active/background stores can update separately; retain both invalidation paths.
- Live updates may legitimately cause checks. The fix promises stability only
when prediction inputs remain equivalent.
docs/plans/workflow-move-preview-stability/task-01-stabilize-invalidation.md
+189
View File
@@ -0,0 +1,189 @@
---
id: "01-stabilize-invalidation"
title: "Stabilize preview invalidation"
status: complete
wave: 1
depends_on: []
plan: "plan.md"
requirements:
- REQ-TASKS-WORKFLOW-MOVE-PREVIEW-001
- REQ-TASKS-WORKFLOW-MOVE-PREVIEW-002
acceptance_criteria:
- AC-TASKS-WORKFLOW-MOVE-PREVIEW-001.7
- AC-TASKS-WORKFLOW-MOVE-PREVIEW-001.8
- AC-TASKS-WORKFLOW-MOVE-PREVIEW-002.1
- AC-TASKS-WORKFLOW-MOVE-PREVIEW-002.2
- AC-TASKS-WORKFLOW-MOVE-PREVIEW-002.3
- AC-TASKS-WORKFLOW-MOVE-PREVIEW-002.4
system_design:
- ../../specs/tasks/system-design/workflow-move-preview.md
---
# Task 01: Stabilize preview invalidation
## Summary
Replace broad record invalidation with prediction-specific revisions. Prove
that harmless updates preserve visible success while real changes still refresh.
## In scope
- Typed revision projections, semantic candidate ordering, and map normalization.
- All regression cases in the [plan test matrix](plan.md#tests).
- Desktop and existing touch-drawer browser evidence, preserving move scenarios.
- Update this work order and plan with actual verification results.
## Out of scope
Backend policy/API changes, persistent caching, stale-result rendering, new copy,
layout changes, production instance mutation, and new navigation surfaces.
## Acceptance
1. The composed hook regression fails before the fix because a harmless update
clears success or issues another request. After the fix it retains success
and the request count across repeated updates in every existing store source.
2. Positive regressions prove relevant changes still invalidate, including
candidate-order changes and mixed eligible/terminal/completion-follow-up
inventories. Preserve cancellation, retry, queue, and generation guards.
3. Desktop and touch E2E keep the disclosure and selected details stable, then
prove a real model update refreshes. Existing move, focus, target-size, and
overflow assertions pass against freshly built assets.
## Implementation sequence
Use `/tdd`. First add the negative revision test and the composed request test
named in the plan; record the expected behavioral failures. Add positive field
and timestamp-order cases before removing broad inputs. Implement explicit
projections in the existing revision module, extracting a small adjacent helper
only if source-size limits require it. Follow the design's metadata and ordering
rules; use backend selectors as read-only references, not targets for edits.
Extend existing browser suites. The unit regression is the RED gate if the
browser fixture cannot reliably hold the pre-fix lifecycle; record that reason.
Run all commands below and record actual counts. Inspect desktop and touch
screenshots against UI-01/UI-02 without changing existing composition.
## ASCII UI preview
UI-01 and UI-02 excerpts; [full views and states](plan.md#ascii-ui-preview).
AC-TASKS-WORKFLOW-MOVE-PREVIEW-001.8 and -002.1 through .3.
```text
Desktop hover Touch drawer
+-------------------------+ +------------------------------+
| Move here | | Move to x |
| Options | | Implement |
| [step capabilities] | | [Options] [Move here] |
|-------------------------| | Reuse current session |
| Reuse current session | | Astra |
| Astra (i) | | [details stay open] |
+-------------------------+ +------------------------------+
Harmless update: both previews retain their current result.
Real input change: Checking session... -> fresh result or Retry.
```
Retain the existing drawer scroll owner, safe-area clearance, 44px touch targets,
and row alignment. Text is illustrative data or existing localized copy.
## Verification
Run from repository root. Install dependencies once if this worktree has no
workspace install. Run browser commands sequentially; managed runners rebuild.
```bash
(cd apps && pnpm install --frozen-lockfile)
(cd apps/web && pnpm exec vitest run hooks/domains/kanban/use-workflow-move-preview-revision.test.ts hooks/domains/kanban/use-workflow-move-preview.test.ts components/task/workflow-move-preview.test.tsx components/task/workflow-stepper.test.tsx components/task/workflow-move-proceed-button.test.tsx)
(cd apps/web && pnpm run typecheck)
(cd apps/web && pnpm exec eslint hooks/domains/kanban/use-workflow-move-preview-revision.ts hooks/domains/kanban/use-workflow-move-preview-revision.test.ts hooks/domains/kanban/use-workflow-move-preview.ts hooks/domains/kanban/use-workflow-move-preview.test.ts)
(cd apps/web && pnpm e2e:run --project chromium tests/workflow/workflow-move-preview.spec.ts)
(cd apps/web && pnpm e2e:run --project mobile-chrome tests/workflow/mobile-workflow-move-preview.spec.ts)
python3 scripts/list-docs.py validate
python3 scripts/lint-spec-files.test.py
python3 scripts/lint-spec-files.py --all
git diff --check
```
Add any extracted helper and changed suite to the lint/test commands before
completion. No new user-facing text is planned; if needed, localize it and run
the repository i18n checks. Record request counts, visible status, and screenshot
comparison. No live instance may be used for browser mutations.
## Files likely touched
- `apps/web/hooks/domains/kanban/use-workflow-move-preview-revision.ts`
- `apps/web/hooks/domains/kanban/use-workflow-move-preview-revision.test.ts`
- `apps/web/hooks/domains/kanban/use-workflow-move-preview.test.ts`
- `apps/web/hooks/domains/kanban/use-workflow-move-preview.ts` only if composed evidence requires it
- `apps/web/e2e/tests/workflow/workflow-move-preview.spec.ts`
- `apps/web/e2e/tests/workflow/mobile-workflow-move-preview.spec.ts`
- An adjacent shared E2E helper if both suites need the same stimulus/capture code
- This plan and work order for results
## Dependencies
None. Existing preview endpoint and consumers are already implemented.
## Risks
See [plan risks](plan.md#risks). Do not remove timestamp sensitivity without
proving candidate-order changes remain observable. Do not silence genuine
invalidations with a longer debounce or a persistent cache.
## Parallelism
`sequential`
## Inputs
- [Requirements](../../specs/tasks/requirements/workflow-move-preview.md).
- [Design](../../specs/tasks/system-design/workflow-move-preview.md#semantic-revision-inputs).
- Existing revision and request-hook suites, renderer/stepper/proceed-button suites.
- `workflow_move_preview.go`, `workflow_move_preview_settings.go`, and
`workflow_session_target.go` under `apps/backend/internal/orchestrator/`.
- Existing E2E specs and `apps/web/e2e/helpers/session-store.ts` for store access.
- `/tdd`, `/mobile-parity`, `/e2e`, and scoped frontend guidance.
## Results
Completed on 2026-09-18.
- TDD RED was confirmed for the new harmless-update and composed-hook cases.
- GREEN focused Vitest passed 74 tests across the revision, request-hook,
renderer, stepper, and proceed-button suites.
- The revision now projects prediction inputs, normalizes configuration maps,
and encodes current/original session identity plus reusable candidate order.
Task descriptions, status-summary bookkeeping, read cursors, command counts,
profile version churn, and timestamp changes that preserve candidate order do
not invalidate the preview. Routing, model, options, connection, context,
eligibility, and candidate-order changes still invalidate it.
- Touch disclosure hover and focus handlers are disabled while the existing
coarse-pointer drawer is active. This keeps the drawer and its details open
through harmless store rerenders while preserving explicit tap and Escape
behavior. Fine-pointer hover and focus behavior is unchanged.
- Desktop Chromium workflow move-preview E2E passed 3 tests. Mobile-chrome
workflow move-preview E2E passed 1 test. Each stability flow observed one
initial request, no request or loading state across three harmless updates,
then a second request and visible loading for a model change before the new
model appeared.
- Typecheck, changed-file ESLint, `pnpm run build:e2e`, `git diff --check`,
`python3 scripts/list-docs.py validate`, `python3 scripts/lint-spec-files.test.py`
(36 tests), and `python3 scripts/lint-spec-files.py --all` passed.
- Review remediation: task-session and indexed-session projections now calculate
current, original, and candidate-order dependencies per source. A duplicate
logical session is not counted twice, while timestamp changes in either source
remain observable when they change a real candidate order. The regression
covers separate timestamp updates, convergence, marked-original provenance,
and a genuine two-candidate reorder.
- Review remediation: profile revisions now include full settings-profile mode
and config options, with the normalized profile `updatedAt` as a scoped
fallback. The regression dispatches the real `agent.profile.updated` handler
for a mode/config-options-only edit, then proves an unrelated profile event
and global version bump do not invalidate the referenced preview.
- Review remediation: an initial-target profile from the durable
`workflow_initial_session` snapshot remains a revision dependency after the
historical session is deleted. The regression proves the missing-session
state and a real profile event invalidate the open preview.
- Review remediation: desktop and touch negative assertions use the shared
`dwell(..., "negative-assertion", ...)` observer window instead of treating a
request timeout as proof that no late refresh occurred.
docs/plans/workflow-move-preview/plan.md
+4
View File
@@ -256,6 +256,10 @@ Final post-remediation verification passed on 2026-09-15:
## Risks
Follow-up: [Preview stability repair](../workflow-move-preview-stability/plan.md)
narrows live invalidation. The results above describe the original delivery;
the follow-up package is complete and owns its implementation and verification.
- Selection helpers can write provenance or warnings; preview must isolate pure decisions.
- Live provider rejection and deferred moves cannot be guaranteed by a hover snapshot.
- Runtime configuration can differ from mutable profile rows.
docs/specs/tasks/requirements/workflow-move-preview.md
+7
View File
@@ -66,6 +66,12 @@ with a Luna profile can retain a later Astra model selection when reused.
options, known routing/configuration state, or the connection state shall
invalidate the preview. Late responses shall not overwrite a newer selection.
Actual moves shall retain execution-time validation, including deferred moves.
- **AC-TASKS-WORKFLOW-MOVE-PREVIEW-001.8:** While a disclosure remains open,
updates that do not affect its prediction shall preserve the displayed result
without another preview request. Examples include task descriptions, read
cursors, command counts, and activity-summary bookkeeping. Timestamp changes
that leave session selection unchanged shall also preserve the result.
Changes that alter candidate eligibility or selection shall still invalidate it.
### REQ-TASKS-WORKFLOW-MOVE-PREVIEW-002: Compact accessible disclosure
@@ -101,6 +107,7 @@ with a Luna profile can retain a later Astra model selection when reused.
- [Conditional settings](workflow-session-settings.md)
- [Design](../system-design/workflow-move-preview.md)
- [Implementation plan](../../../plans/workflow-move-preview/plan.md)
- [Preview stability repair](../../../plans/workflow-move-preview-stability/plan.md)
## Out of scope
docs/specs/tasks/system-design/workflow-move-preview.md
+62
View File
@@ -126,6 +126,68 @@ updates, profile updates, workflow-step updates, reconnect, and draft changes.
Clear on close and fetch fresh on reopen. Debounce draft edits; typing instruction
text need not refresh unless its presence changes prompt dispatch semantics.
### Semantic revision inputs
`getWorkflowMovePreviewRevision` in `use-workflow-move-preview-revision.ts`
shall serialize explicit prediction inputs, not complete store records. This
implements AC-TASKS-WORKFLOW-MOVE-PREVIEW-001.8 while preserving .7. The existing
request hook remains responsible for loading, cancellation, retry, and generation
checks. An irrelevant update retains the same key and successful result. A real
input change clears that result before requesting another prediction. Do not
retain a stale result as current, add polling, or cache completed requests.
Use these projection boundaries for active and background workflow snapshots:
| Input | Included meaning | Excluded bookkeeping |
| --- | --- | --- |
| Task | Identity, placement, primary session/state, archive eligibility, task profile fallback, initial-session and routing metadata | Title, description, position, unrelated metadata, status-summary counters and timestamps |
| Sessions belonging to the task | Membership, identity/name, profile identities, state, primary/passthrough flags, configuration and original-session provenance | Read cursor, command counts, error prose, activity counters, workspace paths |
| Configuration | Runtime configuration, overrides, original effective snapshot, profile snapshot fields used by preview, model/capability/config-option data | Unrelated session metadata and capability-fetch bookkeeping |
| Workflow and steps | Default profile; source end policy; destination profile, target, start policy, entry actions; referenced target-step inputs | Workflow/step colors and unrelated presentation fields |
| Profiles | Prediction-relevant identity, display name, family, model, mode, config options, passthrough and dynamic-routing configuration; scoped profile revision when the normalized option lacks a full field | Global profile version counters and capability error prose without a projected change |
| Context | Connection status, workspace generation, requested task/workflow/step, normalized move options | Unrelated store activity |
Keep both session stores and both task projections observable. Do not choose a
new authority or collapse conflicting snapshots as part of this repair.
Project metadata keys explicitly, including `agent_profile_id`,
`workflow_initial_session`, `workflow_session_route`, `runtime_config`,
`runtime_config_overrides`, `original_effective_config`, and
`completion_follow_up` where the backend reads them. Trace the preview's helper
calls when maintaining this list; arbitrary metadata is not an invalidation key.
Timestamps require semantic treatment. `selectReusableWorkflowSession` uses
`UpdatedAt` to rank eligible same-profile candidates, while `previewCurrentSession`
uses primary flags and then `StartedAt` with an ID tie-break. Encode candidate
ordering or selected identity dependencies instead of raw changing timestamps.
Preserve exclusion and tie behavior, including terminal and completion-follow-up
sessions. A timestamp update that changes selection must change the revision;
one that leaves candidate order unchanged must not. Task-session and indexed
session stores can contain two projections of the same logical session. Keep
their raw projections and sensitivity, but calculate current-session,
original-session, and candidate-order dependencies independently per store so
the duplicate is never treated as two candidates or two marked originals.
This is a freshness signal, not a frontend replacement for the server's move
decision.
Profile events update the full settings profile and a normalized picker option.
The revision must read `mode` and `configOptions` from the full settings
profile, and include the normalized profile's `updatedAt` as a scoped fallback
when those fields are unavailable. An unrelated profile event or global profile
version bump must not invalidate a preview that does not reference that profile.
When an initial-target snapshot names an `agent_profile_id`, include that profile
even if the historical session named by the snapshot is no longer present.
Normalize object key order in projected maps. Sort collections only when their
order is not meaningful; preserve rule order, selection order, and ties.
Retain explicit empty and missing configuration distinctions where the backend
distinguishes them. Never log revision contents.
The correction covers `StepMoveControls`, `StepDisclosureMoveControls`, and
`WorkflowMovePreviewFooter` through their shared hooks. It changes no drawer
geometry, controls, copy, or endpoint contract. The
[stability repair plan](../../../plans/workflow-move-preview-stability/plan.md)
defines the implementation and regression matrix.
Keep two-line loading/error states compact. Retry is explicit and does not move
the task. Preview failure does not introduce a new move barrier. Show predictions
as planned, not guaranteed; details explain that execution rechecks current state.