Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions docs/idb-workspace-state.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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`
71 changes: 71 additions & 0 deletions docs/issue-99-workspaces-drawer-plan.md
Original file line number Diff line number Diff line change
@@ -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
6 changes: 5 additions & 1 deletion docs/localstorage-state.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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`
34 changes: 15 additions & 19 deletions docs/pr-context-storage-matrix.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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

Expand Down Expand Up @@ -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

Expand All @@ -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.

Expand Down
96 changes: 96 additions & 0 deletions docs/workspaces-behavior-algorithm.md
Original file line number Diff line number Diff line change
@@ -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.
14 changes: 4 additions & 10 deletions playwright/github-byot-ai.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -768,29 +768,23 @@

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')

await page.reload()
await expect(page.getByRole('heading', { name: '@knighted/develop' })).toBeVisible()
await expect(page.getByRole('status', { name: 'App status' })).toHaveText(

Check failure on line 787 in playwright/github-byot-ai.spec.ts

View workflow job for this annotation

GitHub Actions / E2E (Playwright, chromium)

[chromium] › playwright/github-byot-ai.spec.ts:723:1 › BYOT remembers selected repository across reloads

1) [chromium] › playwright/github-byot-ai.spec.ts:723:1 › BYOT remembers selected repository across reloads Error: expect(locator).toHaveText(expected) failed Locator: getByRole('status', { name: 'App status' }) Expected: "Loaded 2 writable repositories" Received: "Rendered" Timeout: 60000ms Call log: - Expect "toHaveText" with timeout 60000ms - waiting for getByRole('status', { name: 'App status' }) 4 × locator resolved to <div id="status" role="status" class="status" aria-label="App status">Idle</div> - unexpected value "Idle" - locator resolved to <div id="status" role="status" aria-label="App status" class="status status--pending">Loading CDN assets…</div> - unexpected value "Loading CDN assets…" 58 × locator resolved to <div id="status" role="status" aria-label="App status" class="status status--neutral">Rendered</div> - unexpected value "Rendered" 785 | await page.reload() 786 | await expect(page.getByRole('heading', { name: '@knighted/develop' })).toBeVisible() > 787 | await expect(page.getByRole('status', { name: 'App status' })).toHaveText( | ^ 788 | 'Loaded 2 writable repositories', 789 | { 790 | timeout: 60_000, at /home/runner/work/develop/develop/playwright/github-byot-ai.spec.ts:787:66
'Loaded 2 writable repositories',
{
timeout: 60_000,
Expand Down
Loading
Loading