feat: improved workspace ux and state identity separation. (#103)

Author: knightedcodemonkey

docs/idb-workspace-state.md

@@ -17,13 +17,14 @@ Each workspace record may include:
   - `id`
   - `createdAt`
   - `lastModified`
+  - `workspaceScope` (`local` | `repository`)
 - Repository and PR context:
   - `repo`
   - `base`
   - `head`
   - `prTitle`
   - `prNumber`
-  - `prContextState` (`inactive` | `active` | `disconnected` | `closed`)
+  - `prContextState` (`inactive` | `active` | `closed`)
 - Runtime/editor state:
   - `renderMode`
   - `activeTabId`
@@ -37,10 +38,34 @@ IDB supports that by storing:
 
 - Full workspace snapshots
 - Repo-scoped context records
-- Historical transitions such as disconnected or closed PR context
+- Historical transitions such as closed PR context
 
 ## Design Rule
 
 If a value is required to accurately restore PR/workspace behavior after reload, it must live in IDB records.
 
 `localStorage` should only mirror user preferences and lightweight bootstrap values.
+
+## Post-Push Baseline Invariant
+
+After a successful Push Commit action for an active PR workspace:
+
+- The active workspace record must persist immediately in IDB.
+- Any committed tab path returned by push updates must persist with:
+  - `isDirty = false`
+  - `syncedContent = content`
+  - `syncedAt` updated to the push/reconcile time
+  - `lastSyncedRemoteSha` set when a commit SHA is available
+- The same clean baseline must survive a full page reload.
+
+Dirty-state note:
+
+- When `syncedContent` is present for a tab, canonical dirty state is derived from
+  `content !== syncedContent`.
+- This prevents stale UI-only dirty flags from overriding persisted sync baseline truth.
+
+## Behavioral Spec
+
+For action-level drawer semantics and state machine behavior, see:
+
+- `docs/workspaces-behavior-algorithm.md`

docs/issue-99-workspaces-drawer-plan.md

@@ -0,0 +1,71 @@
+# Issue #99 + Workspaces Drawer UX Plan
+
+## Goal
+
+Simplify PR/workspace lifecycle and the Workspaces drawer UX by:
+
+- removing disconnected state and Disconnect action paths
+- using workspace terminology in the drawer (not context)
+- separating new workspace initialization from workspace selection
+- preserving strict explicit intent semantics (no implicit apply/mutation from select changes)
+
+## Decisions
+
+- New workspace is an explicit direct action via a `New workspace` button.
+- `New workspace` must work for both repository scopes and `Local`.
+- `Open` remains the explicit action for applying an existing stored workspace.
+- If the selected repository has no stored workspaces, hide the workspace select and show the new-workspace path.
+
+## Implementation Steps
+
+1. Remove disconnected model paths (Issue #99)
+
+- Remove Disconnect control from UI.
+- Remove disconnected event wiring and runtime callbacks.
+- Remove disconnected public action paths.
+- Normalize legacy `disconnected` records to `inactive` during restore/normalization.
+
+2. Redesign drawer flow
+
+- Replace starter option-in-select behavior with a dedicated `New workspace` button adjacent to repository select.
+- Keep workspace select for stored workspaces only.
+- Hide workspace select when no stored workspaces exist for the selected scope.
+- Keep strict explicit selection semantics (no auto-apply from select/filter changes).
+
+3. Update copy and accessibility
+
+- Replace "Stored contexts" and related "context" wording with "Workspace" wording.
+- Update status and aria labels consistently.
+
+4. Remove obsolete code paths
+
+- Remove starter prefix constants and parsing.
+- Remove disconnected-only logic and stale styling/branches.
+
+5. Update tests
+
+- Remove/replace disconnected-focused scenarios.
+- Update helpers/selectors to new workspace labels and `New workspace` action.
+- Add/adjust scenarios for empty repository scope (select hidden) and explicit Open behavior for existing workspaces.
+
+6. Update docs
+
+- Update storage/state docs to remove disconnected semantics.
+- Update drawer UX docs to reflect repository row + new workspace action flow.
+
+## Verification
+
+1. `npm run lint`
+2. Targeted Playwright (Chromium first):
+
+- `playwright/github-pr-drawer/active-context-switch.spec.ts`
+- `playwright/github-pr-drawer/open-pr-create.spec.ts`
+
+3. Broader Playwright run for workspace/PR drawer flows.
+4. Manual verification in dev server for:
+
+- repository + new workspace row
+- local new workspace creation
+- hidden workspace select when no stored entries
+- explicit Open required for existing entries
+- no Disconnect control

docs/localstorage-state.md

@@ -19,7 +19,7 @@ Do not store pull request context in `localStorage`.
 Examples that must stay out of `localStorage`:
 
 - Selected repository preference (`owner/repo`)
-- PR context state (`active`, `disconnected`, `closed`, `inactive`)
+- PR context state (`active`, `closed`, `inactive`)
 - PR number and URL
 - PR base/head/title/body
 - PR drawer repository-scoped workflow state
@@ -32,3 +32,7 @@ Examples that must stay out of `localStorage`:
 If data is needed to restore workspace or pull request workflow state, it belongs in IndexedDB workspace records.
 
 Repository selection is derived from in-memory BYOT controls and IndexedDB-backed workspace records, not from a dedicated localStorage key.
+
+For the Workspaces drawer action/state algorithm, see:
+
+- `docs/workspaces-behavior-algorithm.md`

docs/pr-context-storage-matrix.md

@@ -20,7 +20,7 @@ See the full storage ownership docs for non-PR keys:
 - Database: `knighted-develop-workspaces`
 - Object store: `prWorkspaces`
 - Relevant fields in each workspace record:
-  - `prContextState`: `inactive` | `active` | `disconnected` | `closed`
+  - `prContextState`: `inactive` | `active` | `closed`
   - `prNumber`: `number | null`
   - `prTitle`, `base`, `head`
   - `repo`
@@ -34,12 +34,12 @@ See the full storage ownership docs for non-PR keys:
 
 Use this matrix as the source of truth when debugging UI/storage mismatch.
 
-| Scenario                                      | IDB `prContextState` | IDB `prNumber`                    | localStorage PR fields | Notes                                                                                                          |
-| --------------------------------------------- | -------------------- | --------------------------------- | ---------------------- | -------------------------------------------------------------------------------------------------------------- |
-| A. Local workspace only, no PR context        | `inactive`           | `null`                            | none                   | No connected PR context.                                                                                       |
-| B. Workspace is for an active, open PR        | `active`             | PR number                         | none                   | Push mode in PR controls.                                                                                      |
-| C. Workspace is for a disconnected PR context | `disconnected`       | last known PR number if available | none                   | Opening this workspace from Workspaces restores PR runtime context and verifies open/closed state with GitHub. |
-| D. Workspace is for a PR closed on GitHub     | `closed`             | closed PR number                  | none                   | Historical context retained for debugging/reference.                                                           |
+| Scenario                                   | IDB `prContextState` | IDB `prNumber`   | localStorage PR fields | Notes                                                                                                           |
+| ------------------------------------------ | -------------------- | ---------------- | ---------------------- | --------------------------------------------------------------------------------------------------------------- |
+| A. Local workspace only, no PR context     | `inactive`           | `null`           | none                   | No connected PR context.                                                                                        |
+| B. Workspace is for an active, open PR     | `active`             | PR number        | none                   | Push mode in PR controls.                                                                                       |
+| C. Workspace is for a PR closed on GitHub  | `closed`             | closed PR number | none                   | Historical context retained for debugging/reference.                                                            |
+| D. Active PR immediately after push commit | `active`             | PR number        | none                   | Committed tabs persist clean baseline (`isDirty=false`, `syncedContent=content`) and remain clean after reload. |
 
 ## Current Workspace Selection On Load
 
@@ -81,10 +81,12 @@ When the UI does not match expected PR state:
    - `prContextState`
    - `prNumber`
    - `repo`, `head`, `prTitle`
+
+- committed tab fields: `isDirty`, `syncedContent`, `content`, `syncedAt`, `lastSyncedRemoteSha`
+
 2. Compare against the matrix above.
-3. If scenario C is expected, open that workspace from Workspaces to restore runtime PR context.
-4. If the PR is still open on GitHub, expect PR controls to return to Push mode and the workspace record to transition back to `active`.
-5. If the PR is no longer open, expect Open PR mode to remain and status messaging to explain verification results.
+3. If the PR is still open on GitHub, expect PR controls to return to Push mode and the workspace record to transition back to `active`.
+4. If the PR is no longer open, expect Open PR mode to remain and status messaging to explain verification results.
 
 ## Console Snippets
 
@@ -99,19 +101,13 @@ indexedDB.open('knighted-develop-workspaces').onsuccess = event => {
 }
 ```
 
-## Reconnect Behavior
-
-Reconnect behavior from the Workspaces drawer is implemented.
-
-Opening a `disconnected` workspace record restores active PR runtime context for that repository and reinitializes editor state from the selected workspace record.
-
 ## End-Of-Session Behavior
 
-`Disconnect` and `Close` are treated as end-of-session actions for PR-linked workspaces.
+`Close` is the end-of-session action for PR-linked workspaces.
 
-When either action is confirmed:
+When close is confirmed:
 
-1. The current workspace is archived as historical (`disconnected` or `closed`).
+1. The current workspace is archived as historical (`closed`).
 2. The app immediately switches to a fresh local workspace (`inactive`) with a single empty entry tab.
 3. Status messaging guides the user to continue locally or reopen a stored workspace from Workspaces.
 

docs/workspaces-behavior-algorithm.md

@@ -0,0 +1,96 @@
+# Workspaces Drawer Behavior Algorithm
+
+This document locks in the intended behavior for Workspaces drawer actions.
+
+## Goals
+
+- Keep action semantics explicit and predictable.
+- Keep button visibility state-driven and mutually exclusive.
+- Preserve workspace restore behavior by persisting state in IndexedDB.
+
+## Core Terms
+
+- `Local` scope: Workspaces whose `workspaceScope` is `local`.
+- `Repository` scope: Workspaces whose `workspaceScope` is `repository` and whose `repo` matches the selected repository filter.
+- `workspaceKey`: Derived identity key from repository + head branch. Used for matching/preference logic, not for UI policy by itself.
+
+## Required Invariants
+
+1. `Initialize` and `New workspace` must never be visible at the same time.
+2. `Local` scope never shows `Initialize`.
+3. `Initialize` for non-Local empty scope updates the active workspace in place (no fork).
+4. `New workspace` always forks from current editor/runtime state into a new record id.
+5. Fork creation must generate a fresh head branch suffix so `workspaceKey` and visible labels are distinct.
+6. Any workspace created via `New workspace` must persist with `prContextState = "inactive"`.
+7. For `New workspace`, `workspaceScope` is target-dependent:
+   - `local` when repository filter is `__local__`
+   - `repository` when repository filter is a non-local `owner/repo`
+
+## UI State Machine
+
+State is derived from:
+
+- Selected repository filter (`__local__` vs non-local `owner/repo`)
+- Presence of stored workspaces in the selected scope
+
+States:
+
+1. `local-empty`
+   - Show: `New workspace`
+   - Hide: `Initialize`, workspace select, `Open`, `Remove`
+2. `local-with-workspaces`
+   - Show: `New workspace`, workspace select, `Open`, `Remove`
+   - Hide: `Initialize`
+3. `repository-empty`
+   - Show: `Initialize`
+   - Hide: `New workspace`, workspace select, `Open`, `Remove`
+4. `repository-with-workspaces`
+   - Show: `New workspace`, workspace select, `Open`, `Remove`
+   - Hide: `Initialize`
+
+## Action Semantics
+
+### A) Local + New workspace
+
+- Action: fork current workspace into a new record.
+- Persisted updates:
+  - `workspaceScope = "local"`
+  - `prContextState = "inactive"`
+  - `repo = ""`
+  - fresh `id`
+  - fresh local `head` (suffix-appended)
+  - `workspaceKey = local::<fresh-head>`
+
+### B) Non-Local + Initialize (no stored workspaces in selected repository)
+
+- Action: update active workspace in place to selected repository scope.
+- Persisted updates on current record:
+  - `workspaceScope = "repository"`
+  - `repo = <selected owner/repo>`
+  - `workspaceKey = <selected owner/repo>::<current head>`
+- Must preserve current record id.
+
+### C) Non-Local + New workspace (stored workspaces exist)
+
+- Action: fork current workspace into a new repository-scoped record.
+- Persisted updates:
+  - `workspaceScope = "repository"`
+  - `prContextState = "inactive"`
+  - `repo = <selected owner/repo>`
+  - fresh `id`
+  - fresh repository `head` (suffix-appended from current head)
+  - `workspaceKey = <selected owner/repo>::<head>`
+
+## Storage Notes
+
+- Canonical workflow state lives in IndexedDB (`prWorkspaces` records).
+- `localStorage` must not own repository/workspace workflow state.
+
+## Regression Coverage Expectations
+
+At minimum, tests should verify:
+
+1. Local `New workspace` creates a new record and distinct local label/key.
+2. Non-local empty scope shows only `Initialize` and updates active record in place.
+3. Non-local scope with records shows only `New workspace` and forks new record.
+4. `Initialize` and `New workspace` never coexist.

playwright/github-byot-ai.spec.ts

@@ -768,22 +768,16 @@ test('BYOT remembers selected repository across reloads', async ({ page }) => {
 
   await page.getByRole('button', { name: 'Workspaces' }).click()
   const workspaceRepositoryFilter = page.getByLabel('Workspace repository filter')
-  const storedContextsSelect = page.getByLabel('Stored local editor contexts')
-  const openStoredContextButton = page.getByRole('button', {
-    name: 'Open',
+  const initializeButton = page.getByRole('button', {
+    name: 'Initialize',
     exact: true,
   })
   await expect(workspaceRepositoryFilter).toBeVisible()
   await workspaceRepositoryFilter.selectOption('knightedcodemonkey/develop')
   await expect(workspaceRepositoryFilter).toHaveValue('knightedcodemonkey/develop')
 
-  await expect(storedContextsSelect).toBeVisible()
-  await storedContextsSelect.selectOption({
-    label: 'Start new context for knightedcodemonkey/develop',
-  })
-  await expect(openStoredContextButton).toBeEnabled()
-  await openStoredContextButton.click()
-  await page.getByRole('button', { name: 'Close workspaces drawer' }).click()
+  await expect(initializeButton).toBeVisible()
+  await initializeButton.click()
 
   await ensureOpenPrDrawerOpen(page)
   await expect(repoSelect).toHaveValue('knightedcodemonkey/develop')