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
24 changes: 24 additions & 0 deletions docs/next-steps.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,27 @@ Focused follow-up work for `@knighted/develop`.
- 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."

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.
- Do not introduce arbitrary/custom tab names in this pass; treat custom naming as future scope after baseline tab behavior is stable.
- 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."

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.
- Include concrete Component editor examples for each case so reviewer/user expectations are clear.
- Suggested example cases to document:
- Allowed implicit wrap (standalone top-level JSX, no imports/declarations), for example:
- `(<button type="button">Standalone</button>) as any`
- Requires explicit `App` (top-level JSX with declarations/imports), for example:
- `const label = 'Hello'`
- `const Button = () => <button>{label}</button>`
- `(<Button />) as any`
- Recommended explicit pattern, for example:
- `const Button = () => <button>Hello</button>`
- `const App = () => <Button />`
- Suggested implementation prompt:
- "Document the current implicit App behavior in @knighted/develop for auto-render mode using a compact behavior matrix and concrete component-editor snippets. Clearly distinguish supported implicit wrapping from cases that intentionally require an explicit App (such as top-level JSX mixed with imports/declarations). Keep docs concise, aligned with current runtime behavior, and include at least one positive and one explicit-error example."
18 changes: 9 additions & 9 deletions playwright/diagnostics.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ test('clear component diagnostics resets rendered lint-issue status pill', async
await expect(page.getByText('Rendered', { exact: true })).toHaveClass(/status--neutral/)
})

test('component lint ignores unused App View and render bindings', async ({ page }) => {
test('component lint ignores only unused App binding', async ({ page }) => {
await waitForInitialRender(page)

await setComponentEditorSource(
Expand All @@ -361,20 +361,20 @@ test('component lint ignores unused App View and render bindings', async ({ page
await runComponentLint(page)

await ensureDiagnosticsDrawerOpen(page)
await expect(page.getByText('No Biome issues found.')).toBeVisible()

const diagnosticsToggle = page.getByRole('button', { name: /^Diagnostics/ })
await expect(page.getByText('Rendered', { exact: true })).toHaveClass(/status--neutral/)
await expect(diagnosticsToggle).toHaveText('Diagnostics')
await expect(diagnosticsToggle).toHaveClass(/diagnostics-toggle--ok/)
await expect(page.getByText(/Rendered \(Lint issues: [1-9]\d*\)/)).toHaveClass(
/status--error/,
)
await expect(diagnosticsToggle).toHaveText(/Diagnostics \([1-9]\d*\)/)
await expect(diagnosticsToggle).toHaveClass(/diagnostics-toggle--error/)

const diagnosticsText = await page.getByRole('complementary').innerText()
expect(diagnosticsText).toContain('Biome reported issues.')
expect(diagnosticsText).not.toContain('This variable App is unused')
expect(diagnosticsText).not.toContain('This variable View is unused')
expect(diagnosticsText).not.toContain('This variable render is unused')
expect(diagnosticsText).not.toContain('This function App is unused')
expect(diagnosticsText).not.toContain('This function View is unused')
expect(diagnosticsText).not.toContain('This function render is unused')
expect(diagnosticsText).toContain('This function View is unused')
expect(diagnosticsText).toContain('This function render is unused')
})

test('component lint with unresolved issues enters pending diagnostics state while typing', async ({
Expand Down
282 changes: 282 additions & 0 deletions playwright/github-pr-drawer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,15 @@ import {
connectByotWithSingleRepo,
ensureOpenPrDrawerOpen,
mockRepositoryBranches,
setComponentEditorSource,
waitForAppReady,
} from './helpers/app-test-helpers.js'

const decodeGitHubFileBodyContent = (body: Record<string, unknown>) => {
const encoded = typeof body.content === 'string' ? body.content : ''
return Buffer.from(encoded, 'base64').toString('utf8')
}

test('Open PR drawer confirms and submits component/styles filepaths', async ({
page,
}) => {
Expand Down Expand Up @@ -451,3 +457,279 @@ test('Open PR drawer rejects trailing slash file paths', async ({ page }) => {
)
await expect(page.getByRole('dialog')).toBeHidden()
})

test('Open PR drawer include App wrapper checkbox defaults off and resets on reopen', async ({
page,
}) => {
await waitForAppReady(page, `${appEntryPath}?feature-ai=true`)
await connectByotWithSingleRepo(page)
await ensureOpenPrDrawerOpen(page)

const includeWrapperToggle = page.getByLabel(
'Include App wrapper in committed component source',
)
await expect(includeWrapperToggle).not.toBeChecked()

await includeWrapperToggle.check()
await expect(includeWrapperToggle).toBeChecked()

await page.getByRole('button', { name: 'Close open pull request drawer' }).click()
await ensureOpenPrDrawerOpen(page)

await expect(includeWrapperToggle).not.toBeChecked()
})

test('Open PR drawer strips App wrapper from committed component source by default', async ({
page,
}) => {
const upsertRequests: Array<{ path: string; body: Record<string, unknown> }> = []

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 mockRepositoryBranches(page, {
'knightedcodemonkey/develop': ['main', 'release'],
})

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 => {
await route.fulfill({
status: 201,
contentType: 'application/json',
body: JSON.stringify({ ref: 'refs/heads/develop/open-pr-app-wrapper' }),
})
},
)

await page.route(
'https://api.github.com/repos/knightedcodemonkey/develop/contents/**',
async route => {
const request = route.request()
const method = request.method()
const path =
new URL(request.url()).pathname.split('/contents/')[1] ?? 'unknown-file-path'

if (method === 'GET') {
await route.fulfill({
status: 404,
contentType: 'application/json',
body: JSON.stringify({ message: 'Not Found' }),
})
return
}

const body = request.postDataJSON() as Record<string, unknown>
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 => {
await route.fulfill({
status: 201,
contentType: 'application/json',
body: JSON.stringify({
number: 101,
html_url: 'https://github.com/knightedcodemonkey/develop/pull/101',
}),
})
},
)

await waitForAppReady(page, `${appEntryPath}?feature-ai=true`)
await connectByotWithSingleRepo(page)

const componentSource = [
'const CounterButton = () => <button type="button">Counter</button>',
'const App = () => <CounterButton />',
].join('\n')

await setComponentEditorSource(page, componentSource)
await ensureOpenPrDrawerOpen(page)

await page.getByLabel('Head').fill('develop/repo/editor-sync-without-app')
await page.getByRole('button', { name: 'Open PR' }).last().click()
await page.getByRole('dialog').getByRole('button', { name: 'Open PR' }).click()

await expect(
page.getByRole('status', { name: 'Open pull request status', includeHidden: true }),
).toContainText(
'Pull request opened: https://github.com/knightedcodemonkey/develop/pull/101',
)

const componentUpserts = upsertRequests.filter(request =>
request.path.endsWith('/App.jsx'),
)

expect(componentUpserts).toHaveLength(1)

const strippedComponentSource = decodeGitHubFileBodyContent(componentUpserts[0].body)

expect(strippedComponentSource).toContain('const CounterButton = () =>')
expect(strippedComponentSource).not.toContain('const App = () =>')
})

test('Open PR drawer includes App wrapper in committed source when toggled on', async ({
page,
}) => {
const upsertRequests: Array<{ path: string; body: Record<string, unknown> }> = []

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 mockRepositoryBranches(page, {
'knightedcodemonkey/develop': ['main', 'release'],
})

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 => {
await route.fulfill({
status: 201,
contentType: 'application/json',
body: JSON.stringify({ ref: 'refs/heads/develop/open-pr-app-wrapper' }),
})
},
)

await page.route(
'https://api.github.com/repos/knightedcodemonkey/develop/contents/**',
async route => {
const request = route.request()
const method = request.method()
const path =
new URL(request.url()).pathname.split('/contents/')[1] ?? 'unknown-file-path'

if (method === 'GET') {
await route.fulfill({
status: 404,
contentType: 'application/json',
body: JSON.stringify({ message: 'Not Found' }),
})
return
}

const body = request.postDataJSON() as Record<string, unknown>
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 => {
await route.fulfill({
status: 201,
contentType: 'application/json',
body: JSON.stringify({
number: 101,
html_url: 'https://github.com/knightedcodemonkey/develop/pull/101',
}),
})
},
)

await waitForAppReady(page, `${appEntryPath}?feature-ai=true`)
await connectByotWithSingleRepo(page)

await setComponentEditorSource(
page,
[
'const CounterButton = () => <button type="button">Counter</button>',
'const App = () => <CounterButton />',
].join('\n'),
)
await ensureOpenPrDrawerOpen(page)

const includeWrapperToggle = page.getByLabel(
'Include App wrapper in committed component source',
)
await includeWrapperToggle.check()

await page.getByLabel('Head').fill('develop/repo/editor-sync-with-app')
await page.getByRole('button', { name: 'Open PR' }).last().click()
await page.getByRole('dialog').getByRole('button', { name: 'Open PR' }).click()

await expect(
page.getByRole('status', { name: 'Open pull request status', includeHidden: true }),
).toContainText(
'Pull request opened: https://github.com/knightedcodemonkey/develop/pull/101',
)

const componentUpserts = upsertRequests.filter(request =>
request.path.endsWith('/App.jsx'),
)

expect(componentUpserts).toHaveLength(1)

const fullComponentSource = decodeGitHubFileBodyContent(componentUpserts[0].body)
expect(fullComponentSource).toContain('const CounterButton = () =>')
expect(fullComponentSource).toContain('const App = () =>')
})
Loading