From bb6cd5a06ac0601d6f3a8ea6a8f7474d6623715f Mon Sep 17 00:00:00 2001 From: KCM Date: Sun, 22 Mar 2026 16:27:22 -0500 Subject: [PATCH 1/5] feat: open pr styles and ux. --- playwright/app.spec.ts | 192 +++++++++- src/app.js | 102 ++++- src/index.html | 146 +++++++- src/modules/github-api.js | 329 +++++++++++++++++ src/modules/github-byot-controls.js | 111 ++++-- src/modules/github-pr-drawer.js | 552 ++++++++++++++++++++++++++++ src/styles/ai-controls.css | 232 ++++++++++++ src/styles/dialogs-overlays.css | 11 +- 8 files changed, 1628 insertions(+), 47 deletions(-) create mode 100644 src/modules/github-pr-drawer.js diff --git a/playwright/app.spec.ts b/playwright/app.spec.ts index d6a4b58..8fe8e00 100644 --- a/playwright/app.spec.ts +++ b/playwright/app.spec.ts @@ -17,10 +17,19 @@ type ChatRequestBody = { stream?: boolean } +type CreateRefRequestBody = { + ref?: string + sha?: string +} + +type PullRequestCreateBody = { + head?: string + base?: string +} + const waitForAppReady = async (page: Page, path = appEntryPath) => { await page.goto(path) await expect(page.getByRole('heading', { name: '@knighted/develop' })).toBeVisible() - await expect(page.locator('#cdn-loading')).toHaveAttribute('hidden', '') } const waitForInitialRender = async (page: Page) => { @@ -124,6 +133,18 @@ const ensureAiChatDrawerOpen = async (page: Page) => { await expect(page.locator('#ai-chat-drawer')).toBeVisible() } +const ensureOpenPrDrawerOpen = async (page: Page) => { + const toggle = page.locator('#github-pr-toggle') + await expect(toggle).toBeEnabled({ timeout: 60_000 }) + const isExpanded = await toggle.getAttribute('aria-expanded') + + if (isExpanded !== 'true') { + await toggle.click() + } + + await expect(page.locator('#github-pr-drawer')).toBeVisible() +} + const connectByotWithSingleRepo = async (page: Page) => { await page.route('https://api.github.com/user/repos**', async route => { await route.fulfill({ @@ -144,9 +165,8 @@ const connectByotWithSingleRepo = async (page: Page) => { await page.locator('#github-token-input').fill('github_pat_fake_chat_1234567890') await page.locator('#github-token-add').click() - await expect(page.locator('#github-repo-select')).toHaveValue( - 'knightedcodemonkey/develop', - ) + await expect(page.locator('#status')).toHaveText('Loaded 1 writable repositories') + await expect(page.locator('#github-pr-toggle')).toBeVisible() } const expectCollapseButtonState = async ( @@ -187,6 +207,8 @@ test('BYOT controls stay hidden when feature flag is disabled', async ({ page }) await expect(byotControls).toBeHidden() await expect(page.locator('#ai-chat-toggle')).toBeHidden() await expect(page.locator('#ai-chat-drawer')).toBeHidden() + await expect(page.locator('#github-pr-toggle')).toBeHidden() + await expect(page.locator('#github-pr-drawer')).toBeHidden() }) test('BYOT controls render when feature flag is enabled by query param', async ({ @@ -199,6 +221,7 @@ test('BYOT controls render when feature flag is enabled by query param', async ( await expect(page.locator('#github-token-input')).toBeVisible() await expect(page.locator('#github-token-add')).toBeVisible() await expect(page.locator('#github-ai-controls #ai-chat-toggle')).toBeHidden() + await expect(page.locator('#github-ai-controls #github-pr-toggle')).toBeHidden() }) test('GitHub token info panel reflects missing and present token states', async ({ @@ -476,6 +499,8 @@ test('AI chat falls back to non-streaming response when streaming fails', async }) test('BYOT remembers selected repository across reloads', async ({ page }) => { + test.setTimeout(90_000) + await page.route('https://api.github.com/user/repos**', async route => { await route.fulfill({ status: 200, @@ -506,7 +531,9 @@ test('BYOT remembers selected repository across reloads', async ({ page }) => { await page.locator('#github-token-input').fill('github_pat_fake_1234567890') await page.locator('#github-token-add').click() - const repoSelect = page.locator('#github-repo-select') + await ensureOpenPrDrawerOpen(page) + + const repoSelect = page.locator('#github-pr-repo-select') await expect(repoSelect).toBeEnabled() await expect(page.locator('#status')).toHaveText('Loaded 2 writable repositories') @@ -515,11 +542,166 @@ test('BYOT remembers selected repository across reloads', async ({ page }) => { await page.reload() await expect(page.getByRole('heading', { name: '@knighted/develop' })).toBeVisible() + await expect(page.locator('#status')).toHaveText('Loaded 2 writable repositories', { + timeout: 60_000, + }) await expect(page.locator('#github-token-add')).toBeHidden() await expect(page.locator('#github-token-delete')).toBeVisible() + await ensureOpenPrDrawerOpen(page) await expect(repoSelect).toHaveValue('knightedcodemonkey/develop') }) +test('Open PR drawer confirms and submits component/styles filepaths', async ({ + page, +}) => { + let createdRefBody: CreateRefRequestBody | null = null + const upsertRequests: Array<{ path: string; body: Record }> = [] + let pullRequestBody: PullRequestCreateBody | null = null + + await page.route('https://api.github.com/user/repos**', async route => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify([ + { + id: 11, + owner: { login: 'knightedcodemonkey' }, + name: 'develop', + full_name: 'knightedcodemonkey/develop', + default_branch: 'main', + permissions: { push: true }, + }, + ]), + }) + }) + + await page.route( + 'https://api.github.com/repos/knightedcodemonkey/develop/git/ref/**', + async route => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + ref: 'refs/heads/main', + object: { type: 'commit', sha: 'abc123mainsha' }, + }), + }) + }, + ) + + await page.route( + 'https://api.github.com/repos/knightedcodemonkey/develop/git/refs', + async route => { + createdRefBody = route.request().postDataJSON() as CreateRefRequestBody + await route.fulfill({ + status: 201, + contentType: 'application/json', + body: JSON.stringify({ ref: 'refs/heads/develop/open-pr-test' }), + }) + }, + ) + + await page.route( + 'https://api.github.com/repos/knightedcodemonkey/develop/contents/**', + async route => { + const request = route.request() + const method = request.method() + const url = request.url() + const path = new URL(url).pathname.split('/contents/')[1] ?? '' + + if (method === 'GET') { + await route.fulfill({ + status: 404, + contentType: 'application/json', + body: JSON.stringify({ message: 'Not Found' }), + }) + return + } + + const body = request.postDataJSON() as Record + upsertRequests.push({ path: decodeURIComponent(path), body }) + await route.fulfill({ + status: 201, + contentType: 'application/json', + body: JSON.stringify({ commit: { sha: 'commit-sha' } }), + }) + }, + ) + + await page.route( + 'https://api.github.com/repos/knightedcodemonkey/develop/pulls', + async route => { + pullRequestBody = route.request().postDataJSON() as PullRequestCreateBody + await route.fulfill({ + status: 201, + contentType: 'application/json', + body: JSON.stringify({ + number: 42, + html_url: 'https://github.com/knightedcodemonkey/develop/pull/42', + }), + }) + }, + ) + + await waitForAppReady(page, `${appEntryPath}?feature-ai=true`) + await connectByotWithSingleRepo(page) + await ensureOpenPrDrawerOpen(page) + + await page.locator('#github-pr-head-branch').fill('develop/open-pr-test') + await page.locator('#github-pr-component-path').fill('examples/component/App.tsx') + await page.locator('#github-pr-styles-path').fill('examples/styles/app.css') + await page.locator('#github-pr-title').fill('Apply editor updates from develop') + await page + .locator('#github-pr-body') + .fill('Generated from editor content in @knighted/develop.') + + await page.locator('#github-pr-submit').click() + + const dialog = page.locator('#clear-confirm-dialog') + await expect(dialog).toHaveAttribute('open', '') + await expect(page.locator('#clear-confirm-title')).toHaveText( + 'Open pull request with editor content?', + ) + await expect(page.locator('#clear-confirm-copy')).toContainText( + 'Component file path: examples/component/App.tsx', + ) + await expect(page.locator('#clear-confirm-copy')).toContainText( + 'Styles file path: examples/styles/app.css', + ) + + await dialog.getByRole('button', { name: 'Open PR' }).click() + + await expect(page.locator('#github-pr-status')).toContainText( + 'Pull request opened: https://github.com/knightedcodemonkey/develop/pull/42', + ) + + const createdRefPayload = createdRefBody as CreateRefRequestBody | null + const pullRequestPayload = pullRequestBody as PullRequestCreateBody | null + + expect(createdRefPayload?.ref).toBe('refs/heads/develop/open-pr-test') + expect(createdRefPayload?.sha).toBe('abc123mainsha') + + expect(upsertRequests).toHaveLength(2) + expect(upsertRequests[0]?.path).toBe('examples/component/App.tsx') + expect(upsertRequests[1]?.path).toBe('examples/styles/app.css') + expect(pullRequestPayload?.head).toBe('develop/open-pr-test') + expect(pullRequestPayload?.base).toBe('main') +}) + +test('Open PR drawer validates unsafe filepaths', async ({ page }) => { + await waitForAppReady(page, `${appEntryPath}?feature-ai=true`) + await connectByotWithSingleRepo(page) + await ensureOpenPrDrawerOpen(page) + + await page.locator('#github-pr-component-path').fill('../outside/App.tsx') + await page.locator('#github-pr-submit').click() + + await expect(page.locator('#github-pr-status')).toContainText( + 'Component path: File path cannot include parent directory traversal.', + ) + await expect(page.locator('#clear-confirm-dialog')).not.toHaveAttribute('open', '') +}) + test('renders default playground preview', async ({ page }) => { await waitForInitialRender(page) diff --git a/src/app.js b/src/app.js index d1b4d70..ea47975 100644 --- a/src/app.js +++ b/src/app.js @@ -10,6 +10,7 @@ import { createDiagnosticsUiController } from './modules/diagnostics-ui.js' import { isAiAssistantFeatureEnabled } from './modules/feature-flags.js' import { createGitHubChatDrawer } from './modules/github-chat-drawer.js' import { createGitHubByotControls } from './modules/github-byot-controls.js' +import { createGitHubPrDrawer } from './modules/github-pr-drawer.js' import { createLayoutThemeController } from './modules/layout-theme.js' import { createLintDiagnosticsController } from './modules/lint-diagnostics.js' import { createPreviewBackgroundController } from './modules/preview-background.js' @@ -24,8 +25,6 @@ const githubTokenInfo = document.getElementById('github-token-info') const githubTokenInfoPanel = document.getElementById('github-token-info-panel') const githubTokenAdd = document.getElementById('github-token-add') const githubTokenDelete = document.getElementById('github-token-delete') -const githubRepoWrap = document.getElementById('github-repo-wrap') -const githubRepoSelect = document.getElementById('github-repo-select') const aiChatToggle = document.getElementById('ai-chat-toggle') const aiChatDrawer = document.getElementById('ai-chat-drawer') const aiChatClose = document.getElementById('ai-chat-close') @@ -38,6 +37,18 @@ const aiChatStatus = document.getElementById('ai-chat-status') const aiChatRate = document.getElementById('ai-chat-rate') const aiChatRepository = document.getElementById('ai-chat-repository') const aiChatMessages = document.getElementById('ai-chat-messages') +const githubPrToggle = document.getElementById('github-pr-toggle') +const githubPrDrawer = document.getElementById('github-pr-drawer') +const githubPrClose = document.getElementById('github-pr-close') +const githubPrStatus = document.getElementById('github-pr-status') +const githubPrRepoSelect = document.getElementById('github-pr-repo-select') +const githubPrBaseBranch = document.getElementById('github-pr-base-branch') +const githubPrHeadBranch = document.getElementById('github-pr-head-branch') +const githubPrComponentPath = document.getElementById('github-pr-component-path') +const githubPrStylesPath = document.getElementById('github-pr-styles-path') +const githubPrTitle = document.getElementById('github-pr-title') +const githubPrBody = document.getElementById('github-pr-body') +const githubPrSubmit = document.getElementById('github-pr-submit') const viewControlsToggle = document.getElementById('view-controls-toggle') const viewControlsDrawer = document.getElementById('view-controls-drawer') const aiControlsToggle = document.getElementById('ai-controls-toggle') @@ -498,6 +509,7 @@ const { const githubAiContextState = { token: null, selectedRepository: null, + writableRepositories: [], } let chatDrawerController = { @@ -507,17 +519,29 @@ let chatDrawerController = { dispose: () => {}, } +let prDrawerController = { + setOpen: () => {}, + setSelectedRepository: () => {}, + setToken: () => {}, + syncRepositories: () => {}, + dispose: () => {}, +} + const syncAiChatTokenVisibility = token => { const hasToken = typeof token === 'string' && token.trim().length > 0 if (hasToken) { aiChatToggle?.removeAttribute('hidden') + githubPrToggle?.removeAttribute('hidden') return } aiChatToggle?.setAttribute('hidden', '') aiChatToggle?.setAttribute('aria-expanded', 'false') + githubPrToggle?.setAttribute('hidden', '') + githubPrToggle?.setAttribute('aria-expanded', 'false') chatDrawerController.setOpen(false) + prDrawerController.setOpen(false) } const byotControls = createGitHubByotControls({ @@ -527,11 +551,16 @@ const byotControls = createGitHubByotControls({ tokenInfoButton: githubTokenInfo, tokenAddButton: githubTokenAdd, tokenDeleteButton: githubTokenDelete, - repoSelect: githubRepoSelect, - repoWrap: githubRepoWrap, onRepositoryChange: repository => { githubAiContextState.selectedRepository = repository chatDrawerController.setSelectedRepository(repository) + prDrawerController.setSelectedRepository(repository) + }, + onWritableRepositoriesChange: ({ repositories }) => { + githubAiContextState.writableRepositories = Array.isArray(repositories) + ? [...repositories] + : [] + prDrawerController.syncRepositories() }, onTokenDeleteRequest: onConfirm => { confirmAction({ @@ -547,18 +576,28 @@ const byotControls = createGitHubByotControls({ githubAiContextState.token = token syncAiChatTokenVisibility(token) chatDrawerController.setToken(token) + prDrawerController.setToken(token) }, setStatus, }) githubAiContextState.selectedRepository = byotControls.getSelectedRepository() githubAiContextState.token = byotControls.getToken() +githubAiContextState.writableRepositories = byotControls.getWritableRepositories() const getCurrentGitHubToken = () => githubAiContextState.token ?? byotControls.getToken() const getCurrentSelectedRepository = () => githubAiContextState.selectedRepository ?? byotControls.getSelectedRepository() +const getCurrentWritableRepositories = () => + githubAiContextState.writableRepositories.length > 0 + ? [...githubAiContextState.writableRepositories] + : byotControls.getWritableRepositories() + +const setCurrentSelectedRepository = fullName => + byotControls.setSelectedRepository(fullName) + chatDrawerController = createGitHubChatDrawer({ featureEnabled: aiAssistantFeatureEnabled, toggleButton: aiChatToggle, @@ -585,6 +624,39 @@ chatDrawerController = createGitHubChatDrawer({ }, }) +prDrawerController = createGitHubPrDrawer({ + featureEnabled: aiAssistantFeatureEnabled, + toggleButton: githubPrToggle, + drawer: githubPrDrawer, + closeButton: githubPrClose, + repositorySelect: githubPrRepoSelect, + baseBranchInput: githubPrBaseBranch, + headBranchInput: githubPrHeadBranch, + componentPathInput: githubPrComponentPath, + stylesPathInput: githubPrStylesPath, + prTitleInput: githubPrTitle, + prBodyInput: githubPrBody, + submitButton: githubPrSubmit, + statusNode: githubPrStatus, + getToken: getCurrentGitHubToken, + getSelectedRepository: getCurrentSelectedRepository, + getWritableRepositories: getCurrentWritableRepositories, + setSelectedRepository: setCurrentSelectedRepository, + getComponentSource: () => getJsxSource(), + getStylesSource: () => getCssSource(), + getDrawerSide: () => { + const layout = getCurrentLayout() + return layout === 'preview-left' ? 'left' : 'right' + }, + confirmBeforeSubmit: options => { + confirmAction(options) + }, +}) + +prDrawerController.setToken(githubAiContextState.token) +prDrawerController.setSelectedRepository(githubAiContextState.selectedRepository) +prDrawerController.syncRepositories() + const getStyleEditorLanguage = mode => { if (mode === 'less') return 'less' if (mode === 'sass') return 'sass' @@ -985,6 +1057,7 @@ const confirmAction = ({ fallbackConfirmText, onConfirm, }) => { + const toConfirmText = value => (typeof value === 'string' ? value.trim() : '') const supportsModalDialog = clearConfirmDialog instanceof HTMLDialogElement && typeof clearConfirmDialog.showModal === 'function' @@ -1004,7 +1077,25 @@ const confirmAction = ({ clearConfirmTitle.textContent = title } - if (clearConfirmCopy) { + if (clearConfirmCopy instanceof HTMLUListElement) { + const lines = toConfirmText(copy) + .split('\n') + .map(line => line.replace(/^\s*[-*]\s*/, '').trim()) + .filter(Boolean) + + clearConfirmCopy.replaceChildren() + const items = lines.length > 0 ? lines : [toConfirmText(copy)] + + for (const line of items) { + if (!line) { + continue + } + + const listItem = document.createElement('li') + listItem.textContent = line + clearConfirmCopy.append(listItem) + } + } else if (clearConfirmCopy) { clearConfirmCopy.textContent = copy } @@ -1336,6 +1427,7 @@ window.addEventListener('beforeunload', () => { clearStylesLintRecheckTimer() lintDiagnostics.dispose() chatDrawerController.dispose() + prDrawerController.dispose() }) applyAppGridLayout(getInitialAppGridLayout(), { persist: false }) diff --git a/src/index.html b/src/index.html index 9020995..ba23187 100644 --- a/src/index.html +++ b/src/index.html @@ -115,12 +115,22 @@

- +

+ +
@@ -636,7 +766,9 @@

AI Chat

>

Clear source?

-

This action will remove all text from the editor.

+
    +
  • This action will remove all text from the editor.
    • +
+ +
diff --git a/src/modules/github-api.js b/src/modules/github-api.js index 2f51a8b..84664ea 100644 --- a/src/modules/github-api.js +++ b/src/modules/github-api.js @@ -645,6 +645,18 @@ const toUtf8Base64 = value => { return btoa(binary) } +const isMissingShaForExistingFileError = error => { + if (!(error instanceof Error)) { + return false + } + + const message = error.message.toLowerCase() + return ( + message.includes('sha') && + (message.includes('already exists') || message.includes('must be supplied')) + ) +} + export const upsertRepositoryFile = async ({ token, owner, @@ -655,34 +667,62 @@ export const upsertRepositoryFile = async ({ message, signal, }) => { - const existingFile = await getRepositoryFileMetadata({ - token, - owner, - repo, - path, - ref: branch, - signal, - }) - const encodedPath = encodePathForApi(path) - const response = await requestGitHubJson({ - token, - url: `${githubApiBaseUrl}/repos/${owner}/${repo}/contents/${encodedPath}`, - method: 'PUT', - body: { - message, - content: toUtf8Base64(content), - branch, - ...(existingFile?.sha ? { sha: existingFile.sha } : {}), - }, - signal, - }) + const baseBody = { + message, + content: toUtf8Base64(content), + branch, + } - return { - path, - commitSha: typeof response?.commit?.sha === 'string' ? response.commit.sha : null, - created: !existingFile?.sha, + try { + const response = await requestGitHubJson({ + token, + url: `${githubApiBaseUrl}/repos/${owner}/${repo}/contents/${encodedPath}`, + method: 'PUT', + body: baseBody, + signal, + }) + + return { + path, + commitSha: typeof response?.commit?.sha === 'string' ? response.commit.sha : null, + created: true, + } + } catch (error) { + if (!isMissingShaForExistingFileError(error)) { + throw error + } + + const existingFile = await getRepositoryFileMetadata({ + token, + owner, + repo, + path, + ref: branch, + signal, + }) + + if (!existingFile?.sha) { + throw error + } + + const response = await requestGitHubJson({ + token, + url: `${githubApiBaseUrl}/repos/${owner}/${repo}/contents/${encodedPath}`, + method: 'PUT', + body: { + ...baseBody, + sha: existingFile.sha, + }, + signal, + }) + + return { + path, + commitSha: typeof response?.commit?.sha === 'string' ? response.commit.sha : null, + created: false, + } } } diff --git a/src/modules/github-pr-drawer.js b/src/modules/github-pr-drawer.js index 9745a39..777ee05 100644 --- a/src/modules/github-pr-drawer.js +++ b/src/modules/github-pr-drawer.js @@ -40,6 +40,18 @@ const saveRepositoryPrConfig = ({ repositoryFullName, config }) => { } } +const clearRepositoryPrConfig = repositoryFullName => { + if (typeof repositoryFullName !== 'string' || !repositoryFullName.trim()) { + return + } + + try { + localStorage.removeItem(`${prConfigStoragePrefix}${repositoryFullName}`) + } catch { + /* noop */ + } +} + const toSafeText = value => (typeof value === 'string' ? value.trim() : '') const normalizeFilePath = value => @@ -92,10 +104,38 @@ const sanitizeBranchPart = value => { .replace(/^[-/.]+|[-/.]+$/g, '') } +const toUtcBranchStamp = () => { + const now = new Date() + const parts = [ + String(now.getUTCFullYear()), + String(now.getUTCMonth() + 1).padStart(2, '0'), + String(now.getUTCDate()).padStart(2, '0'), + String(now.getUTCHours()).padStart(2, '0'), + String(now.getUTCMinutes()).padStart(2, '0'), + String(now.getUTCSeconds()).padStart(2, '0'), + ] + + return `${parts[0]}${parts[1]}${parts[2]}-${parts[3]}${parts[4]}${parts[5]}` +} + +const createBranchEntropySuffix = () => Math.random().toString(36).slice(2, 6) + +const isAutoGeneratedHeadBranch = value => { + const branch = sanitizeBranchPart(value) + if (!branch) { + return false + } + + return /^develop\/[^/]+\/editor-sync-\d{8}(?:-\d{6})?(?:-[a-z0-9]{4})?(?:-\d+)?$/.test( + branch, + ) +} + const createDefaultBranchName = repository => { const repoName = sanitizeBranchPart(repository?.name ?? '') || 'repo' - const stamp = new Date().toISOString().slice(0, 10).replace(/-/g, '') - return `develop/${repoName}/editor-sync-${stamp}` + const stamp = toUtcBranchStamp() + const entropy = createBranchEntropySuffix() + return `develop/${repoName}/editor-sync-${stamp}-${entropy}` } const toDefaultPrTitle = repository => { @@ -156,6 +196,7 @@ export const createGitHubPrDrawer = ({ getStylesSource, getDrawerSide, confirmBeforeSubmit, + onPullRequestOpened, }) => { if (!featureEnabled) { toggleButton?.setAttribute('hidden', '') @@ -174,6 +215,11 @@ export const createGitHubPrDrawer = ({ let open = false let submitting = false let pendingAbortController = null + let resetOnNextOpen = false + const submitButtonDefaultLabel = + submitButton instanceof HTMLButtonElement && toSafeText(submitButton.textContent) + ? toSafeText(submitButton.textContent) + : 'Open PR' const setStatus = (text, level = 'neutral') => { if (!statusNode) { @@ -190,6 +236,8 @@ export const createGitHubPrDrawer = ({ if (submitButton instanceof HTMLButtonElement) { submitButton.disabled = isPending submitButton.setAttribute('aria-busy', isPending ? 'true' : 'false') + submitButton.classList.toggle('render-button--loading', isPending) + submitButton.textContent = isPending ? 'Opening PR...' : submitButtonDefaultLabel } for (const input of [ @@ -266,10 +314,10 @@ export const createGitHubPrDrawer = ({ repositorySelect.value = selectedFullName } - const syncFormForRepository = ({ resetBranch = false } = {}) => { + const syncFormForRepository = ({ resetBranch = false, resetAll = false } = {}) => { const repository = getSelectedRepositoryObject() const repositoryFullName = getRepositoryFullName(repository) - const savedConfig = readRepositoryPrConfig(repositoryFullName) + const savedConfig = resetAll ? {} : readRepositoryPrConfig(repositoryFullName) const componentFilePath = typeof savedConfig.componentFilePath === 'string' && savedConfig.componentFilePath @@ -298,21 +346,24 @@ export const createGitHubPrDrawer = ({ } if (headBranchInput instanceof HTMLInputElement) { - if (resetBranch || !toSafeText(headBranchInput.value)) { + if (resetAll || resetBranch || !toSafeText(headBranchInput.value)) { + const savedHeadBranch = sanitizeBranchPart(savedConfig.headBranch) headBranchInput.value = - toSafeText(savedConfig.headBranch) || createDefaultBranchName(repository) + savedHeadBranch && !isAutoGeneratedHeadBranch(savedHeadBranch) + ? savedHeadBranch + : createDefaultBranchName(repository) } } if (prTitleInput instanceof HTMLInputElement) { - if (!toSafeText(prTitleInput.value)) { + if (resetAll || !toSafeText(prTitleInput.value)) { prTitleInput.value = toSafeText(savedConfig.prTitle) || toDefaultPrTitle(repository) } } if (prBodyInput instanceof HTMLTextAreaElement) { - if (!toSafeText(prBodyInput.value)) { + if (resetAll || !toSafeText(prBodyInput.value)) { prBodyInput.value = typeof savedConfig.prBody === 'string' && savedConfig.prBody ? savedConfig.prBody @@ -335,7 +386,7 @@ export const createGitHubPrDrawer = ({ componentFilePath: values.componentFilePath, stylesFilePath: values.stylesFilePath, baseBranch: values.baseBranch, - headBranch: values.headBranch, + headBranch: isAutoGeneratedHeadBranch(values.headBranch) ? '' : values.headBranch, prTitle: values.prTitle, prBody: values.prBody, }, @@ -364,7 +415,14 @@ export const createGitHubPrDrawer = ({ drawer.toggleAttribute('hidden', !open) if (open) { - syncRepositories() + const repositories = getWritableRepositories?.() ?? [] + const selectedRepository = getSelectedRepositoryObject() + syncRepositorySelect({ repositories, selectedRepository }) + syncFormForRepository({ + resetAll: resetOnNextOpen, + resetBranch: resetOnNextOpen, + }) + resetOnNextOpen = false repositorySelect?.focus() } } @@ -455,6 +513,12 @@ export const createGitHubPrDrawer = ({ url ? `Pull request opened: ${url}` : 'Pull request opened successfully.', 'ok', ) + onPullRequestOpened?.({ + url, + pullRequestNumber: result.pullRequest.number, + }) + clearRepositoryPrConfig(repositoryLabel) + resetOnNextOpen = true setOpen(false) }) .catch(error => { @@ -465,6 +529,8 @@ export const createGitHubPrDrawer = ({ const message = error instanceof Error ? error.message : 'Failed to open pull request.' setStatus(`Open PR failed: ${message}`, 'error') + clearRepositoryPrConfig(repositoryLabel) + resetOnNextOpen = true }) .finally(() => { if (pendingAbortController === abortController) { diff --git a/src/styles/dialogs-overlays.css b/src/styles/dialogs-overlays.css index f694975..1ca96c2 100644 --- a/src/styles/dialogs-overlays.css +++ b/src/styles/dialogs-overlays.css @@ -100,6 +100,42 @@ font-size: 0.85rem; } +.app-toast { + position: fixed; + top: 76px; + right: 24px; + max-width: min(560px, calc(100vw - 48px)); + border-radius: 12px; + border: 1px solid color-mix(in srgb, #22c55e 45%, var(--border-control)); + background: color-mix(in srgb, #22c55e 14%, var(--surface-dialog)); + color: color-mix(in srgb, #22c55e 88%, var(--panel-text)); + box-shadow: 0 14px 28px var(--shadow-elev-1); + padding: 10px 12px; + font-size: 0.84rem; + line-height: 1.35; + z-index: 130; + opacity: 0; + transform: translateY(-6px); + transition: + opacity 170ms ease, + transform 170ms ease; + pointer-events: none; +} + +.app-toast[data-open='true'] { + opacity: 1; + transform: translateY(0); +} + +@media (max-width: 900px) { + .app-toast { + top: 66px; + left: 12px; + right: 12px; + max-width: none; + } +} + .cdn-loading { position: fixed; inset: 0; From b4835cf3ea9181527836d6c8b0f5d1cde2f86fd7 Mon Sep 17 00:00:00 2001 From: KCM Date: Sun, 22 Mar 2026 17:18:26 -0500 Subject: [PATCH 3/5] refactor: address comments. --- playwright/app.spec.ts | 33 +++++++++++++++++++++++++++++ src/modules/github-api.js | 17 ++++++++++----- src/modules/github-byot-controls.js | 10 +++++++++ src/modules/github-pr-drawer.js | 11 +++++++--- 4 files changed, 63 insertions(+), 8 deletions(-) diff --git a/playwright/app.spec.ts b/playwright/app.spec.ts index 8fe8e00..40800d2 100644 --- a/playwright/app.spec.ts +++ b/playwright/app.spec.ts @@ -30,6 +30,8 @@ type PullRequestCreateBody = { const waitForAppReady = async (page: Page, path = appEntryPath) => { await page.goto(path) await expect(page.getByRole('heading', { name: '@knighted/develop' })).toBeVisible() + await expect(page.locator('#cdn-loading')).toHaveAttribute('hidden', '') + await expect.poll(() => page.locator('#status').textContent()).not.toBe('Idle') } const waitForInitialRender = async (page: Page) => { @@ -702,6 +704,37 @@ test('Open PR drawer validates unsafe filepaths', async ({ page }) => { await expect(page.locator('#clear-confirm-dialog')).not.toHaveAttribute('open', '') }) +test('Open PR drawer allows dotted file segments that are not traversal', async ({ + page, +}) => { + await waitForAppReady(page, `${appEntryPath}?feature-ai=true`) + await connectByotWithSingleRepo(page) + await ensureOpenPrDrawerOpen(page) + + await page.locator('#github-pr-component-path').fill('docs/v1.0..v1.1/App.tsx') + await page.locator('#github-pr-styles-path').fill('styles/foo..bar.css') + await page.locator('#github-pr-submit').click() + + await expect(page.locator('#clear-confirm-dialog')).toHaveAttribute('open', '') + await expect(page.locator('#github-pr-status')).not.toContainText( + 'File path cannot include parent directory traversal.', + ) +}) + +test('Open PR drawer rejects trailing slash file paths', async ({ page }) => { + await waitForAppReady(page, `${appEntryPath}?feature-ai=true`) + await connectByotWithSingleRepo(page) + await ensureOpenPrDrawerOpen(page) + + await page.locator('#github-pr-component-path').fill('src/components/') + await page.locator('#github-pr-submit').click() + + await expect(page.locator('#github-pr-status')).toContainText( + 'Component path: File path must include a filename (no trailing slash).', + ) + await expect(page.locator('#clear-confirm-dialog')).not.toHaveAttribute('open', '') +}) + test('renders default playground preview', async ({ page }) => { await waitForInitialRender(page) diff --git a/src/modules/github-api.js b/src/modules/github-api.js index 84664ea..2d73a32 100644 --- a/src/modules/github-api.js +++ b/src/modules/github-api.js @@ -545,9 +545,14 @@ const requestGitHubJson = async ({ signal, allowNotFound = false, }) => { + const headers = { + ...buildRequestHeaders(token), + ...(body ? { 'Content-Type': 'application/json' } : {}), + } + const response = await fetch(url, { method, - headers: buildRequestHeaders(token), + headers, body: body ? JSON.stringify(body) : undefined, signal, }) @@ -636,13 +641,15 @@ export const getRepositoryFileMetadata = async ({ const toUtf8Base64 = value => { const encoder = new TextEncoder() const bytes = encoder.encode(value) - let binary = '' + const chunkSize = 0x8000 + const chunks = [] - for (const byte of bytes) { - binary += String.fromCharCode(byte) + for (let offset = 0; offset < bytes.length; offset += chunkSize) { + const chunk = bytes.subarray(offset, offset + chunkSize) + chunks.push(String.fromCharCode(...chunk)) } - return btoa(binary) + return btoa(chunks.join('')) } const isMissingShaForExistingFileError = error => { diff --git a/src/modules/github-byot-controls.js b/src/modules/github-byot-controls.js index 2b84acd..3142992 100644 --- a/src/modules/github-byot-controls.js +++ b/src/modules/github-byot-controls.js @@ -193,6 +193,16 @@ export const createGitHubByotControls = ({ } const clearRepoOptions = placeholderLabel => { + if (repoSelect instanceof HTMLSelectElement) { + repoSelect.replaceChildren( + createDefaultRepoOption({ + label: placeholderLabel, + disabled: true, + }), + ) + repoSelect.disabled = true + } + if (typeof onWritableRepositoriesChange === 'function') { onWritableRepositoriesChange({ repositories: [], diff --git a/src/modules/github-pr-drawer.js b/src/modules/github-pr-drawer.js index 777ee05..8de9825 100644 --- a/src/modules/github-pr-drawer.js +++ b/src/modules/github-pr-drawer.js @@ -71,7 +71,13 @@ const validateFilePath = value => { } } - if (path.includes('..')) { + if (path.endsWith('/')) { + return { ok: false, reason: 'File path must include a filename (no trailing slash).' } + } + + const segments = path.split('/').filter(Boolean) + + if (segments.some(segment => segment === '..')) { return { ok: false, reason: 'File path cannot include parent directory traversal.' } } @@ -83,7 +89,6 @@ const validateFilePath = value => { } } - const segments = path.split('/').filter(Boolean) if (segments.length === 0 || segments.some(segment => segment === '.' || !segment)) { return { ok: false, reason: 'File path is invalid.' } } @@ -99,7 +104,7 @@ const sanitizeBranchPart = value => { return trimmed .replace(/[^a-z0-9._/-]/g, '-') - .replace(/\/+/, '/') + .replace(/\/+/g, '/') .replace(/-{2,}/g, '-') .replace(/^[-/.]+|[-/.]+$/g, '') } From f53e515a1ee9d6edc50f463dc3ce45d772500178 Mon Sep 17 00:00:00 2001 From: KCM Date: Sun, 22 Mar 2026 17:20:58 -0500 Subject: [PATCH 4/5] docs: update next steps. --- docs/next-steps.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/docs/next-steps.md b/docs/next-steps.md index de4148e..19d5106 100644 --- a/docs/next-steps.md +++ b/docs/next-steps.md @@ -19,19 +19,21 @@ Focused follow-up work for `@knighted/develop`. - Suggested implementation prompt: - "Add a deterministic E2E execution mode for `@knighted/develop` that serves pinned runtime artifacts locally (instead of live CDN fetches) and wire it into CI as a required check on every PR. Keep a separate lightweight CDN-smoke E2E check for real-network coverage. Validate with `npm run lint`, deterministic Playwright PR checks, and one CDN-smoke Playwright run." -4. **Issue #18 continuation (resume from Phase 3)** +4. **Issue #18 continuation (finish remaining Phase 3 scope)** - Current rollout status: - Phase 0 complete: feature flag + scaffolding. - Phase 1 complete: BYOT token flow, localStorage persistence, writable repo discovery/filtering. - Phase 2 complete: separate AI chat drawer UX, streaming-first responses with non-stream fallback, selected repository context plumbing, and README fine-grained PAT setup links. - - Implement the next slice first (Phase 3): + - Phase 3 partially complete: PR-prep filename/path groundwork landed via the Open PR drawer with repository-scoped persistence and stricter path validation. + - Phase 4 complete: open PR flow from editor content (branch creation, file upserts, PR creation), confirmation UX, loading/success states, and toast feedback. + - Post-implementation hardening complete: traversal/path validation edge cases, trailing-slash rejection, writable-repo select reset behavior during loading/error states, and a JS-driven Playwright readiness check. + - Implement the next slice (remaining Phase 3 assistant features): - 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. - - Add filename groundwork for upcoming PR flows by allowing user-defined Component and Styles file names, persisted per selected repository. - Keep behavior and constraints aligned with current implementation: - Keep everything behind the existing browser-only AI feature flag. - 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. - - Phase 3 mini-spec (agent implementation prompt): - - "Continue Issue #18 in @knighted/develop from the current Phase 2 baseline. Implement Phase 3 with three 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. (3) Add PR-prep filename metadata: introduce user-editable fields for Component filename and Styles filename in AI controls, validate simple safe filename format, and persist/reload values scoped to selected repository so Phase 4 PR write flow can reuse them. 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, apply/undo editor actions, and repository-scoped filename persistence." + - 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." From 87f942b4d5070c407394050a001296589b849b1f Mon Sep 17 00:00:00 2001 From: KCM Date: Sun, 22 Mar 2026 17:41:00 -0500 Subject: [PATCH 5/5] refactor: address comments. --- playwright/app.spec.ts | 33 +++++++++++++++-- src/modules/github-byot-controls.js | 2 +- src/modules/github-pr-drawer.js | 57 ++++++++++++++++++++--------- 3 files changed, 71 insertions(+), 21 deletions(-) diff --git a/playwright/app.spec.ts b/playwright/app.spec.ts index 40800d2..14a2a6f 100644 --- a/playwright/app.spec.ts +++ b/playwright/app.spec.ts @@ -649,7 +649,7 @@ test('Open PR drawer confirms and submits component/styles filepaths', async ({ await connectByotWithSingleRepo(page) await ensureOpenPrDrawerOpen(page) - await page.locator('#github-pr-head-branch').fill('develop/open-pr-test') + await page.locator('#github-pr-head-branch').fill('Develop/Open-Pr-Test') await page.locator('#github-pr-component-path').fill('examples/component/App.tsx') await page.locator('#github-pr-styles-path').fill('examples/styles/app.css') await page.locator('#github-pr-title').fill('Apply editor updates from develop') @@ -680,14 +680,41 @@ test('Open PR drawer confirms and submits component/styles filepaths', async ({ const createdRefPayload = createdRefBody as CreateRefRequestBody | null const pullRequestPayload = pullRequestBody as PullRequestCreateBody | null - expect(createdRefPayload?.ref).toBe('refs/heads/develop/open-pr-test') + expect(createdRefPayload?.ref).toBe('refs/heads/Develop/Open-Pr-Test') expect(createdRefPayload?.sha).toBe('abc123mainsha') expect(upsertRequests).toHaveLength(2) expect(upsertRequests[0]?.path).toBe('examples/component/App.tsx') expect(upsertRequests[1]?.path).toBe('examples/styles/app.css') - expect(pullRequestPayload?.head).toBe('develop/open-pr-test') + expect(pullRequestPayload?.head).toBe('Develop/Open-Pr-Test') expect(pullRequestPayload?.base).toBe('main') + + await ensureOpenPrDrawerOpen(page) + await expect(page.locator('#github-pr-component-path')).toHaveValue( + 'examples/component/App.tsx', + ) + await expect(page.locator('#github-pr-styles-path')).toHaveValue( + 'examples/styles/app.css', + ) + await expect(page.locator('#github-pr-base-branch')).toHaveValue('main') + + await expect(page.locator('#github-pr-head-branch')).toHaveValue( + /^develop\/develop\/editor-sync-/, + ) + await expect(page.locator('#github-pr-head-branch')).not.toHaveValue( + 'Develop/Open-Pr-Test', + ) + await expect(page.locator('#github-pr-title')).toHaveValue( + 'Apply component and styles edits to knightedcodemonkey/develop', + ) + await expect(page.locator('#github-pr-body')).toHaveValue( + [ + 'This PR was created from @knighted/develop editor content.', + '', + '- Component source -> examples/component/App.tsx', + '- Styles source -> examples/styles/app.css', + ].join('\n'), + ) }) test('Open PR drawer validates unsafe filepaths', async ({ page }) => { diff --git a/src/modules/github-byot-controls.js b/src/modules/github-byot-controls.js index 3142992..a2f097b 100644 --- a/src/modules/github-byot-controls.js +++ b/src/modules/github-byot-controls.js @@ -214,7 +214,7 @@ export const createGitHubByotControls = ({ const getSelectedRepositoryObject = () => { if (!lastSelectedRepository) { - return + return null } return writableRepos.find(repo => repo.fullName === lastSelectedRepository) ?? null diff --git a/src/modules/github-pr-drawer.js b/src/modules/github-pr-drawer.js index 8de9825..cbee5ee 100644 --- a/src/modules/github-pr-drawer.js +++ b/src/modules/github-pr-drawer.js @@ -40,16 +40,27 @@ const saveRepositoryPrConfig = ({ repositoryFullName, config }) => { } } -const clearRepositoryPrConfig = repositoryFullName => { +const clearRepositoryPrDraftFields = ({ + repositoryFullName, + componentFilePath, + stylesFilePath, + baseBranch, +}) => { if (typeof repositoryFullName !== 'string' || !repositoryFullName.trim()) { return } - try { - localStorage.removeItem(`${prConfigStoragePrefix}${repositoryFullName}`) - } catch { - /* noop */ - } + saveRepositoryPrConfig({ + repositoryFullName, + config: { + componentFilePath, + stylesFilePath, + baseBranch, + headBranch: '', + prTitle: '', + prBody: '', + }, + }) } const toSafeText = value => (typeof value === 'string' ? value.trim() : '') @@ -97,18 +108,20 @@ const validateFilePath = value => { } const sanitizeBranchPart = value => { - const trimmed = toSafeText(value).toLowerCase() + const trimmed = toSafeText(value) if (!trimmed) { return '' } return trimmed - .replace(/[^a-z0-9._/-]/g, '-') + .replace(/[^A-Za-z0-9._/-]/g, '-') .replace(/\/+/g, '/') .replace(/-{2,}/g, '-') .replace(/^[-/.]+|[-/.]+$/g, '') } +const sanitizeAutoBranchPart = value => sanitizeBranchPart(value).toLowerCase() + const toUtcBranchStamp = () => { const now = new Date() const parts = [ @@ -137,7 +150,7 @@ const isAutoGeneratedHeadBranch = value => { } const createDefaultBranchName = repository => { - const repoName = sanitizeBranchPart(repository?.name ?? '') || 'repo' + const repoName = sanitizeAutoBranchPart(repository?.name ?? '') || 'repo' const stamp = toUtcBranchStamp() const entropy = createBranchEntropySuffix() return `develop/${repoName}/editor-sync-${stamp}-${entropy}` @@ -322,7 +335,8 @@ export const createGitHubPrDrawer = ({ const syncFormForRepository = ({ resetBranch = false, resetAll = false } = {}) => { const repository = getSelectedRepositoryObject() const repositoryFullName = getRepositoryFullName(repository) - const savedConfig = resetAll ? {} : readRepositoryPrConfig(repositoryFullName) + const savedConfig = readRepositoryPrConfig(repositoryFullName) + const savedDraftConfig = resetAll ? {} : savedConfig const componentFilePath = typeof savedConfig.componentFilePath === 'string' && savedConfig.componentFilePath @@ -352,7 +366,7 @@ export const createGitHubPrDrawer = ({ if (headBranchInput instanceof HTMLInputElement) { if (resetAll || resetBranch || !toSafeText(headBranchInput.value)) { - const savedHeadBranch = sanitizeBranchPart(savedConfig.headBranch) + const savedHeadBranch = sanitizeBranchPart(savedDraftConfig.headBranch) headBranchInput.value = savedHeadBranch && !isAutoGeneratedHeadBranch(savedHeadBranch) ? savedHeadBranch @@ -363,15 +377,15 @@ export const createGitHubPrDrawer = ({ if (prTitleInput instanceof HTMLInputElement) { if (resetAll || !toSafeText(prTitleInput.value)) { prTitleInput.value = - toSafeText(savedConfig.prTitle) || toDefaultPrTitle(repository) + toSafeText(savedDraftConfig.prTitle) || toDefaultPrTitle(repository) } } if (prBodyInput instanceof HTMLTextAreaElement) { if (resetAll || !toSafeText(prBodyInput.value)) { prBodyInput.value = - typeof savedConfig.prBody === 'string' && savedConfig.prBody - ? savedConfig.prBody + typeof savedDraftConfig.prBody === 'string' && savedDraftConfig.prBody + ? savedDraftConfig.prBody : toDefaultPrBody({ componentFilePath, stylesFilePath }) } } @@ -512,7 +526,12 @@ export const createGitHubPrDrawer = ({ signal: abortController.signal, }) .then(result => { - persistCurrentPaths() + clearRepositoryPrDraftFields({ + repositoryFullName: repositoryLabel, + componentFilePath: componentPathValidation.value, + stylesFilePath: stylesPathValidation.value, + baseBranch: values.baseBranch, + }) const url = result.pullRequest.htmlUrl setStatus( url ? `Pull request opened: ${url}` : 'Pull request opened successfully.', @@ -522,7 +541,6 @@ export const createGitHubPrDrawer = ({ url, pullRequestNumber: result.pullRequest.number, }) - clearRepositoryPrConfig(repositoryLabel) resetOnNextOpen = true setOpen(false) }) @@ -534,7 +552,12 @@ export const createGitHubPrDrawer = ({ const message = error instanceof Error ? error.message : 'Failed to open pull request.' setStatus(`Open PR failed: ${message}`, 'error') - clearRepositoryPrConfig(repositoryLabel) + clearRepositoryPrDraftFields({ + repositoryFullName: repositoryLabel, + componentFilePath: componentPathValidation.value, + stylesFilePath: stylesPathValidation.value, + baseBranch: values.baseBranch, + }) resetOnNextOpen = true }) .finally(() => {