fix: keep watcher save state visible (#2037)
This commit is contained in:
Carlos Florêncio
@@ -0,0 +1,123 @@
|
||||
---
|
||||
spec: docs/specs/ui/settings-manual-save.md
|
||||
created: 2026-07-29
|
||||
status: complete
|
||||
---
|
||||
|
||||
# Implementation Plan: Integration Watcher Save State
|
||||
|
||||
## Overview
|
||||
|
||||
Watcher enabled-state saves pass through a shared frontend draft hook. GitHub
|
||||
review-watch updates currently violate their frontend contract by returning an
|
||||
acknowledgement instead of the updated watch, so the store does not advance and
|
||||
the cleared draft reveals stale state. The repair aligns that endpoint with all
|
||||
other watcher updates, makes the shared hook retain a clean saved baseline until
|
||||
authoritative items reconcile, and proves the user-visible flow without a
|
||||
refresh.
|
||||
|
||||
## Confirmed root cause
|
||||
|
||||
- `updateReviewWatch()` is typed as returning `ReviewWatch`, but
|
||||
`httpUpdateReviewWatch` returns `{"updated": true}`.
|
||||
- `useReviewWatches.update` forwards that acknowledgement to
|
||||
`updateReviewWatch` in the Zustand slice. Because the response has no `id`,
|
||||
the cached row is not replaced.
|
||||
- `useWatcherEnabledDrafts.save` removes the successful local draft immediately.
|
||||
When its `items` input is still stale, the rendered row falls back to the
|
||||
pre-save `enabled` value until a reload fetches the persisted record.
|
||||
- The shared hook is used by GitHub, GitLab, Jira, Linear, and Sentry watcher
|
||||
settings, so it must tolerate delayed authoritative-list reconciliation.
|
||||
|
||||
## Backend
|
||||
|
||||
### GitHub review-watch update response
|
||||
|
||||
- Update `apps/backend/internal/github/controller.go` so
|
||||
`httpUpdateReviewWatch` loads and returns the updated `ReviewWatch`, matching
|
||||
the existing issue-watch handler and the TypeScript client contract.
|
||||
- Add a focused HTTP regression in
|
||||
`apps/backend/internal/github/controller_test.go` that updates `enabled` and
|
||||
asserts the response contains the same watch `id` with the new value.
|
||||
|
||||
No database, authorization, or public route changes are required.
|
||||
|
||||
## Frontend
|
||||
|
||||
### Shared watcher saved baseline
|
||||
|
||||
- Update
|
||||
`apps/web/components/integrations/use-watcher-enabled-drafts.ts` to distinguish
|
||||
unsaved drafts from successfully persisted values that have not yet appeared
|
||||
in `items`.
|
||||
- Render and toggle against the latest saved baseline, keep the contributor
|
||||
clean after a successful save, preserve edits made during an in-flight save,
|
||||
and remove the temporary saved baseline once the authoritative item matches
|
||||
or disappears.
|
||||
- Extend
|
||||
`apps/web/components/integrations/use-watcher-enabled-drafts.test.tsx` with
|
||||
regressions for stale post-save items and later authoritative reconciliation.
|
||||
|
||||
### Mobile design contract
|
||||
|
||||
This is state reconciliation inside existing watcher tables and cards. It does
|
||||
not change composition, navigation, overlays, scrolling, touch targets, or
|
||||
viewport behavior. Desktop and mobile continue sharing the same watcher state
|
||||
hook and action handlers. The nearest mobile exemplar is the existing GitLab
|
||||
watch card in `mobile-gitlab-parity.spec.ts`, whose touch-sized pause control
|
||||
already covers the shared mobile interaction.
|
||||
|
||||
## Tests
|
||||
|
||||
- **What:** GitHub review-watch update returns the authoritative updated row.
|
||||
**File:** `apps/backend/internal/github/controller_test.go`.
|
||||
**How:** SQLite-backed Gin handler test through the real controller/service/store.
|
||||
- **What:** A successful watcher save stays visually changed and clean while
|
||||
incoming items are stale, then reconciles when they catch up.
|
||||
**File:**
|
||||
`apps/web/components/integrations/use-watcher-enabled-drafts.test.tsx`.
|
||||
**How:** Vitest hook harness under `SettingsSaveProvider`.
|
||||
- **What:** Failed saves and in-flight newer edits remain dirty.
|
||||
**File:**
|
||||
`apps/web/components/integrations/use-watcher-enabled-drafts.test.tsx`.
|
||||
**How:** retain and extend the existing failure/retry coverage as needed.
|
||||
|
||||
## E2E Tests
|
||||
|
||||
- **Scenario:** GIVEN an active GitHub review watch, WHEN the user pauses it and
|
||||
saves, THEN the row stays Paused without reload; WHEN the user enables and
|
||||
saves again, THEN it stays Active without reload.
|
||||
**File:** `apps/web/e2e/tests/integrations/github-workspace-settings.spec.ts`.
|
||||
**What to verify:** status text, clean Save changes state, and persisted API
|
||||
value after each save.
|
||||
- Run the existing mobile GitLab watcher pause scenario as change-aware proof
|
||||
that the shared hook retains mobile capability and touch behavior.
|
||||
|
||||
## Implementation Tasks
|
||||
|
||||
1. [x] [Task 01: Return the updated GitHub review watch](task-01-github-review-watch-response.md)
|
||||
2. [x] [Task 02: Reconcile shared watcher saved state](task-02-shared-watcher-saved-baseline.md)
|
||||
3. [x] [Task 03: Prove pause and resume without refresh](task-03-watcher-save-e2e.md)
|
||||
|
||||
Execution is sequential in the primary conversation. No task is delegated
|
||||
without explicit user authorization.
|
||||
|
||||
## Verification
|
||||
|
||||
After each task's targeted checks:
|
||||
|
||||
```bash
|
||||
make fmt
|
||||
make typecheck test lint
|
||||
```
|
||||
|
||||
The E2E managed runner rebuilds backend and frontend production artifacts before
|
||||
the focused Playwright checks.
|
||||
|
||||
## Risks
|
||||
|
||||
- Clearing a saved overlay too early recreates the flicker; retaining it forever
|
||||
can mask a later authoritative update. Reconciliation therefore clears only
|
||||
when the matching item reaches the saved value or the item disappears.
|
||||
- Concurrent edits during Save must remain dirty and must not be replaced by
|
||||
the completed request.
|
||||
@@ -0,0 +1,54 @@
|
||||
---
|
||||
id: "01-github-review-watch-response"
|
||||
title: "Return the updated GitHub review watch"
|
||||
status: done
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/ui/settings-manual-save.md"
|
||||
---
|
||||
|
||||
# Task 01: Return the updated GitHub review watch
|
||||
|
||||
## Acceptance
|
||||
|
||||
- `PUT /api/v1/github/watches/review/:id` returns the complete updated
|
||||
`ReviewWatch`, including its `id` and new `enabled` value.
|
||||
- The route keeps its existing workspace authorization and error behavior.
|
||||
- A controller regression fails on the acknowledgement-only response and passes
|
||||
on the authoritative response.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
make -C apps/backend test
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/internal/github/controller.go`
|
||||
- `apps/backend/internal/github/controller_test.go`
|
||||
|
||||
## Dependencies
|
||||
|
||||
None.
|
||||
|
||||
## Parallelism
|
||||
|
||||
Sequential.
|
||||
|
||||
## Inputs
|
||||
|
||||
- `docs/specs/ui/settings-manual-save.md`
|
||||
- `docs/plans/integration-watcher-save-state/plan.md`
|
||||
- Existing `httpUpdateIssueWatch` response pattern
|
||||
|
||||
## Risks
|
||||
|
||||
The test must exercise the real handler/service/store path so the frontend
|
||||
contract cannot drift back to an acknowledgement shape.
|
||||
|
||||
## Output contract
|
||||
|
||||
Report the red and green command results, files changed, blockers, risks, and
|
||||
update this task plus `plan.md` status in the primary conversation.
|
||||
@@ -0,0 +1,56 @@
|
||||
---
|
||||
id: "02-shared-watcher-saved-baseline"
|
||||
title: "Reconcile shared watcher saved state"
|
||||
status: done
|
||||
wave: 2
|
||||
depends_on: ["01-github-review-watch-response"]
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/ui/settings-manual-save.md"
|
||||
---
|
||||
|
||||
# Task 02: Reconcile shared watcher saved state
|
||||
|
||||
## Acceptance
|
||||
|
||||
- A successful watcher enabled-state save becomes the clean rendered baseline
|
||||
even when the incoming integration list still contains the old value.
|
||||
- An edit made while Save is in flight remains visible and dirty after the
|
||||
earlier request succeeds.
|
||||
- The temporary saved baseline is removed after authoritative items match it or
|
||||
the watcher disappears.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
cd apps && pnpm --filter @kandev/web test -- --run components/integrations/use-watcher-enabled-drafts.test.tsx
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/web/components/integrations/use-watcher-enabled-drafts.ts`
|
||||
- `apps/web/components/integrations/use-watcher-enabled-drafts.test.tsx`
|
||||
|
||||
## Dependencies
|
||||
|
||||
Task 01 documents the authoritative endpoint contract the hook reconciles
|
||||
against.
|
||||
|
||||
## Parallelism
|
||||
|
||||
Sequential.
|
||||
|
||||
## Inputs
|
||||
|
||||
- `docs/specs/ui/settings-manual-save.md`
|
||||
- `docs/plans/integration-watcher-save-state/plan.md`
|
||||
- `docs/decisions/0046-settings-route-save-coordinator.md`
|
||||
|
||||
## Risks
|
||||
|
||||
React may batch the saved-baseline and draft updates. The test must assert the
|
||||
user-visible state, not update ordering or setter calls.
|
||||
|
||||
## Output contract
|
||||
|
||||
Report the red and green command results, files changed, blockers, risks, and
|
||||
update this task plus `plan.md` status in the primary conversation.
|
||||
@@ -0,0 +1,59 @@
|
||||
---
|
||||
id: "03-watcher-save-e2e"
|
||||
title: "Prove pause and resume without refresh"
|
||||
status: done
|
||||
wave: 3
|
||||
depends_on:
|
||||
- "01-github-review-watch-response"
|
||||
- "02-shared-watcher-saved-baseline"
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/ui/settings-manual-save.md"
|
||||
---
|
||||
|
||||
# Task 03: Prove pause and resume without refresh
|
||||
|
||||
## Acceptance
|
||||
|
||||
- The GitHub workspace settings E2E pauses and resumes a review watch through
|
||||
the floating Save action without reloading between either saved state.
|
||||
- The test asserts the status, action availability, and persisted API value.
|
||||
- The existing mobile GitLab pause scenario remains green against the shared
|
||||
hook with its touch and viewport assertions.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
cd apps && pnpm install --frozen-lockfile
|
||||
cd apps/web && pnpm e2e:run tests/integrations/github-workspace-settings.spec.ts -- --grep "keeps review watch pause and resume visible after save"
|
||||
cd apps/web && pnpm e2e:run tests/gitlab/mobile-gitlab-parity.spec.ts -- --project=mobile-chrome --grep "watch controls remain touch sized and persist a pause"
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/web/e2e/tests/integrations/github-workspace-settings.spec.ts`
|
||||
- `apps/web/e2e/tests/gitlab/mobile-gitlab-parity.spec.ts`
|
||||
|
||||
## Dependencies
|
||||
|
||||
Tasks 01 and 02.
|
||||
|
||||
## Parallelism
|
||||
|
||||
Sequential.
|
||||
|
||||
## Inputs
|
||||
|
||||
- `docs/specs/ui/settings-manual-save.md`
|
||||
- `docs/plans/integration-watcher-save-state/plan.md`
|
||||
- `apps/web/e2e/helpers/api-client.ts`
|
||||
- Existing GitLab watcher desktop/mobile E2E patterns
|
||||
|
||||
## Risks
|
||||
|
||||
Use stable role/test-id selectors and the managed runner's production rebuild.
|
||||
Do not add timeouts to hide a missed state transition.
|
||||
|
||||
## Output contract
|
||||
|
||||
Report the red and green command results, files changed, blockers, risks, and
|
||||
update this task plus `plan.md` status in the primary conversation.
|
||||
@@ -1,6 +1,7 @@
|
||||
---
|
||||
status: shipped
|
||||
created: 2026-07-14
|
||||
updated: 2026-07-29
|
||||
owner: kandev
|
||||
supersedes: docs/specs/workflow-settings-autosave/spec.md
|
||||
---
|
||||
@@ -44,7 +45,11 @@ The manual-save rule applies to route-level persistent configuration, including:
|
||||
- appearance, resource metrics, keyboard shortcuts, terminal and shell, editors, notifications, voice mode, prompts, secrets, and changelog-notification configuration;
|
||||
- utility-agent selection/enabled state and the config-chat profile;
|
||||
- runtime feature-toggle overrides;
|
||||
- integration configuration forms, GitHub repository scope/action presets/default queries, and GitHub/Jira/Linear/Sentry watcher enabled state.
|
||||
- integration configuration forms, GitHub repository scope/action presets/default queries, and GitHub/GitLab/Jira/Linear/Sentry watcher enabled state.
|
||||
- After a watcher enabled-state save succeeds, every integration keeps the saved
|
||||
Active or Paused state visible without requiring a page refresh. A delayed
|
||||
integration-list refresh may reconcile the saved baseline but must not make
|
||||
the row revert to its pre-save state.
|
||||
|
||||
### Immediate actions
|
||||
|
||||
@@ -96,6 +101,10 @@ Route navigation with dirty contributors opens an in-app confirmation with `Save
|
||||
- **GIVEN** a new workflow draft, **WHEN** the user reloads before saving, **THEN** no workflow was created and the draft is discarded.
|
||||
- **GIVEN** two dirty settings contributors and one save fails, **WHEN** Save completes, **THEN** the successful contributor is clean, the failed contributor remains dirty, and the action reports `Couldn't save`.
|
||||
- **GIVEN** a save is in flight, **WHEN** the user changes another field, **THEN** the completed request does not clear the newer dirty revision.
|
||||
- **GIVEN** an integration watcher is Active or Paused, **WHEN** the user changes
|
||||
its enabled state and Save succeeds, **THEN** the row immediately shows the
|
||||
saved state, the contributor becomes clean, and the state does not revert
|
||||
while the integration list catches up.
|
||||
- **GIVEN** a dirty keyboard shortcut, **WHEN** the user presses Reset, **THEN** the default shortcut is shown locally and no persistence request occurs until Save.
|
||||
- **GIVEN** a dirty feature-toggle override, **WHEN** the user presses Reset to default, **THEN** the inherited value is staged and the runtime flag does not change until Save succeeds.
|
||||
- **GIVEN** a dirty color-theme draft, **WHEN** the user previews another theme and leaves with Discard, **THEN** the previously saved theme is restored.
|
||||
|
||||
Reference in New Issue
Block a user