diff --git a/README.md b/README.md index 1443e24..bde818c 100644 --- a/README.md +++ b/README.md @@ -41,9 +41,13 @@ browser acts as the runtime host for render, lint, and typecheck flows. - GitHub PAT setup and usage: [docs/byot.md](docs/byot.md) +AI chat features remain opt-in behind +`?feature-ai=true`. + ## Fine-Grained PAT Quick Setup -For AI/BYOT flows, use a fine-grained GitHub PAT and follow the existing setup guide: +For PR/BYOT and AI chat flows, use a fine-grained GitHub PAT and follow the +existing setup guide: - Full setup and behavior: [docs/byot.md](docs/byot.md) - Repository permissions screenshot: [docs/media/byot-repo-perms.png](docs/media/byot-repo-perms.png) diff --git a/docs/byot.md b/docs/byot.md index 41a4c86..ae17249 100644 --- a/docs/byot.md +++ b/docs/byot.md @@ -4,13 +4,14 @@ This guide explains how to create and use a fine-grained GitHub Personal Access ## What BYOT does in the app -When the AI/BYOT feature is enabled, the token is used to: +BYOT controls are available by default. The token is used to: - authenticate GitHub API requests - load repositories where you have write access - let you choose which repository to work with +- use PR context features (Open PR / Push Commit flows) -As additional AI/PR features roll out, the same token is also used for model and repository operations that require the configured permissions. +When AI chat is enabled, the same token is also used for GitHub Models requests. ## Privacy and storage behavior @@ -18,9 +19,10 @@ As additional AI/PR features roll out, the same token is also used for model and - The token is never sent to any service except the GitHub endpoints required by the feature. - You can remove it at any time using the delete button in the BYOT controls. -## Enable the BYOT feature +## Enable AI chat features -Use one of these options: +BYOT/PR controls do not require a feature flag. To enable AI chat features, use one +of these options: 1. Add `?feature-ai=true` to the app URL. 2. Set `localStorage` key `knighted:develop:feature:ai-assistant` to `true`. @@ -57,10 +59,11 @@ Use either of these scopes depending on your needs: ## Recommended setup flow 1. Create token with the permissions above. -2. Open `@knighted/develop` with `?feature-ai=true`. +2. Open `@knighted/develop`. 3. Paste token into the BYOT input and click add. 4. Verify repository list loads. 5. Select your target repository. +6. Optional: enable AI chat with `?feature-ai=true`. ## Screenshots diff --git a/docs/next-steps.md b/docs/next-steps.md index 0b0fedb..deba9e3 100644 --- a/docs/next-steps.md +++ b/docs/next-steps.md @@ -31,12 +31,13 @@ Focused follow-up work for `@knighted/develop`. - Add mode-aware recommendation behavior so the assistant strongly adapts suggestions to current render mode and style mode. - Add an editor update workflow where the assistant can propose structured edits and the user can apply to Component and Styles editors with explicit confirmation. - Keep behavior and constraints aligned with current implementation: - - Keep everything behind the existing browser-only AI feature flag. + - Keep AI chat/assistant behavior behind the existing browser-only AI feature flag. + - Keep PR/BYOT controls available by default. - Preserve BYOT token semantics (localStorage persistence until user deletes). - Keep CDN-first runtime behavior and existing fallback model. - Do not add dependencies without explicit approval. - Remaining Phase 3 mini-spec (agent implementation prompt): - - "Continue Issue #18 in @knighted/develop from the current baseline where PR filename/path groundwork and Open PR flow are already shipped. Implement the two remaining Phase 3 assistant deliverables. (1) Add mode-aware assistant guidance: when collecting AI context, include explicit policy hints derived from render mode and style mode, and ensure recommendations avoid incompatible patterns (for example, avoid React hook/state guidance in DOM mode unless user explicitly asks for React migration). (2) Add assistant-to-editor apply flow: support structured assistant responses that can propose edits for component and/or styles editors; render these as reviewable actions in the chat drawer, require explicit user confirmation to apply, and support a one-step undo for last applied assistant edit per editor. Keep all AI/BYOT behavior behind the existing browser-only AI feature flag and preserve current token/repo persistence semantics. Do not add dependencies. Validate with npm run lint and targeted Playwright tests covering mode-aware recommendation constraints and apply/undo editor actions." + - "Continue Issue #18 in @knighted/develop from the current baseline where PR filename/path groundwork and Open PR flow are already shipped. Implement the two remaining Phase 3 assistant deliverables. (1) Add mode-aware assistant guidance: when collecting AI context, include explicit policy hints derived from render mode and style mode, and ensure recommendations avoid incompatible patterns (for example, avoid React hook/state guidance in DOM mode unless user explicitly asks for React migration). (2) Add assistant-to-editor apply flow: support structured assistant responses that can propose edits for component and/or styles editors; render these as reviewable actions in the chat drawer, require explicit user confirmation to apply, and support a one-step undo for last applied assistant edit per editor. Keep AI chat/assistant behavior behind the existing browser-only AI feature flag, keep PR/BYOT controls available by default, and preserve current token/repo persistence semantics. Do not add dependencies. Validate with npm run lint and targeted Playwright tests covering mode-aware recommendation constraints and apply/undo editor actions." 5. **Phase 2 UX/UI continuation: fixed editor tabs first pass (Component, Styles, App)** - Continue the tabs/editor UX work with a constrained first implementation that supports exactly three editor tabs: Component, Styles, and App. @@ -44,7 +45,7 @@ Focused follow-up work for `@knighted/develop`. - Preserve existing runtime behavior and editor content semantics while adding tab switching, active tab indication, and predictable persistence/reset behavior consistent with current app patterns. - Ensure assistant/editor integration remains compatible with this model (edits should target one of the fixed tabs) without expanding to dynamic tab metadata yet. - Suggested implementation prompt: - - "Implement Phase 2 UX/UI tab support in @knighted/develop with a fixed first-pass tab model: Component, Styles, and App only (no arbitrary tab names yet). Add a clear tab UI for switching editor panes, preserve existing editor behavior/content wiring, and keep render/lint/typecheck/diagnostics flows working with the selected tab context where relevant. Keep AI/BYOT feature-flag behavior unchanged, maintain CDN-first runtime constraints, and do not add dependencies. Add targeted Playwright coverage for tab switching, default/active tab behavior, and interactions with existing render/style-mode flows. Validate with npm run lint and targeted Playwright tests." + - "Implement Phase 2 UX/UI tab support in @knighted/develop with a fixed first-pass tab model: Component, Styles, and App only (no arbitrary tab names yet). Add a clear tab UI for switching editor panes, preserve existing editor behavior/content wiring, and keep render/lint/typecheck/diagnostics flows working with the selected tab context where relevant. Keep AI chat feature-flag behavior unchanged while keeping PR/BYOT controls available by default, maintain CDN-first runtime constraints, and do not add dependencies. Add targeted Playwright coverage for tab switching, default/active tab behavior, and interactions with existing render/style-mode flows. Validate with npm run lint and targeted Playwright tests." 6. **Document implicit App strict-flow behavior (auto render)** - Add a short behavior matrix in docs that explains when implicit App wrapping is allowed versus when users must define `App` explicitly. diff --git a/playwright/github-byot-ai.spec.ts b/playwright/github-byot-ai.spec.ts index 2495033..4518cb4 100644 --- a/playwright/github-byot-ai.spec.ts +++ b/playwright/github-byot-ai.spec.ts @@ -10,19 +10,32 @@ import { waitForAppReady, } from './helpers/app-test-helpers.js' -test('BYOT controls stay hidden when feature flag is disabled', async ({ page }) => { +test('PR/BYOT controls are visible without feature flag, but chat stays hidden', async ({ + page, +}) => { await waitForAppReady(page) - const byotControls = page.getByRole('group', { - name: 'GitHub controls', + const byotControls = page.getByRole('group', { name: 'GitHub controls' }) + const prToggle = page.getByRole('button', { + name: 'Open pull request', + exact: true, includeHidden: true, }) - await expect(byotControls).toHaveAttribute('hidden', '') - await expect(byotControls).toBeHidden() + await expect(byotControls).toBeVisible() + await expect(page.getByRole('textbox', { name: 'GitHub token' })).toBeVisible() + await expect(page.getByRole('button', { name: 'Add GitHub token' })).toBeVisible() await expect(page.getByRole('button', { name: 'Chat' })).toBeHidden() await expect(page.getByRole('heading', { name: 'AI Chat' })).toBeHidden() - await expect(page.getByRole('button', { name: 'Open PR' })).toBeHidden() - await expect(page.getByRole('heading', { name: 'Open Pull Request' })).toBeHidden() + await expect(prToggle).toHaveCount(1) + await expect(prToggle).toBeHidden() +}) + +test('chat remains hidden without feature flag after token connect', async ({ page }) => { + await waitForAppReady(page) + await connectByotWithSingleRepo(page) + + await expect(page.getByRole('button', { name: 'Open pull request' })).toBeVisible() + await expect(page.getByRole('button', { name: 'Chat' })).toBeHidden() }) test('BYOT controls render when feature flag is enabled by query param', async ({ @@ -31,11 +44,17 @@ test('BYOT controls render when feature flag is enabled by query param', async ( await waitForAppReady(page, `${appEntryPath}?feature-ai=true`) const byotControls = page.getByRole('group', { name: 'GitHub controls' }) + const prToggle = page.getByRole('button', { + name: 'Open pull request', + exact: true, + includeHidden: true, + }) await expect(byotControls).toBeVisible() await expect(page.getByRole('textbox', { name: 'GitHub token' })).toBeVisible() await expect(page.getByRole('button', { name: 'Add GitHub token' })).toBeVisible() await expect(page.getByRole('button', { name: 'Chat' })).toBeHidden() - await expect(page.getByRole('button', { name: 'Open PR' })).toBeHidden() + await expect(prToggle).toHaveCount(1) + await expect(prToggle).toBeHidden() }) test('GitHub token info panel reflects missing and present token states', async ({ diff --git a/src/app.js b/src/app.js index 75051b7..f3dff8b 100644 --- a/src/app.js +++ b/src/app.js @@ -224,16 +224,6 @@ const setCompactAiControlsOpen = isOpen => { return } - if (!aiAssistantFeatureEnabled) { - compactAiControlsOpen = false - setGitHubTokenInfoOpen(false) - aiControlsToggle.setAttribute('hidden', '') - aiControlsToggle.setAttribute('aria-expanded', 'false') - githubAiControls.removeAttribute('data-compact-open') - githubAiControls.setAttribute('hidden', '') - return - } - aiControlsToggle.removeAttribute('hidden') if (!isCompactViewport()) { @@ -663,8 +653,15 @@ const syncAiChatTokenVisibility = token => { const hasToken = typeof token === 'string' && token.trim().length > 0 if (hasToken) { - aiChatToggle?.removeAttribute('hidden') + if (aiAssistantFeatureEnabled) { + aiChatToggle?.removeAttribute('hidden') + } else { + aiChatToggle?.setAttribute('hidden', '') + aiChatToggle?.setAttribute('aria-expanded', 'false') + } + githubPrToggle?.removeAttribute('hidden') + if (githubAiContextState.activePrContext) { githubPrContextClose?.removeAttribute('hidden') } else { @@ -688,7 +685,6 @@ const syncAiChatTokenVisibility = token => { } const byotControls = createGitHubByotControls({ - featureEnabled: aiAssistantFeatureEnabled, controlsRoot: githubAiControls, tokenInput: githubTokenInput, tokenInfoButton: githubTokenInfo, @@ -791,7 +787,6 @@ chatDrawerController = createGitHubChatDrawer({ }) prDrawerController = createGitHubPrDrawer({ - featureEnabled: aiAssistantFeatureEnabled, toggleButton: githubPrToggle, drawer: githubPrDrawer, closeButton: githubPrClose, diff --git a/src/modules/github-byot-controls.js b/src/modules/github-byot-controls.js index 94f6850..0f494b5 100644 --- a/src/modules/github-byot-controls.js +++ b/src/modules/github-byot-controls.js @@ -48,7 +48,6 @@ const createDefaultRepoOption = ({ } export const createGitHubByotControls = ({ - featureEnabled, controlsRoot, tokenInput, tokenInfoButton, @@ -61,16 +60,6 @@ export const createGitHubByotControls = ({ onTokenChange, setStatus, }) => { - if (!featureEnabled) { - controlsRoot?.setAttribute('hidden', '') - return { - getSelectedRepository: () => null, - getWritableRepositories: () => [], - setSelectedRepository: () => false, - getToken: () => null, - } - } - let savedToken = loadGitHubToken() let currentRepoRequestAbortController = null let displayingMaskedToken = false diff --git a/src/modules/github-pr-drawer.js b/src/modules/github-pr-drawer.js index bb31309..090cdeb 100644 --- a/src/modules/github-pr-drawer.js +++ b/src/modules/github-pr-drawer.js @@ -398,7 +398,6 @@ const stripTopLevelAppWrapper = async ({ source, getTopLevelDeclarations }) => { } export const createGitHubPrDrawer = ({ - featureEnabled, toggleButton, drawer, closeButton, @@ -432,23 +431,6 @@ export const createGitHubPrDrawer = ({ onRestoreRenderMode, onRestoreStyleMode, }) => { - if (!featureEnabled) { - toggleButton?.setAttribute('hidden', '') - drawer?.setAttribute('hidden', '') - - return { - setOpen: () => {}, - isOpen: () => false, - setToken: () => {}, - setSelectedRepository: () => {}, - getActivePrContext: () => null, - clearActivePrContext: () => {}, - closeActivePullRequestOnGitHub: async () => null, - syncRepositories: () => {}, - dispose: () => {}, - } - } - let open = false let submitting = false let pendingAbortController = null