Skip to content

Commit f388db4

Browse files
refactor: address item five from next steps.
1 parent a2ac7c0 commit f388db4

7 files changed

Lines changed: 139 additions & 69 deletions

File tree

docs/next-steps.md

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,7 @@ Focused follow-up work for `@knighted/develop`.
2626
- Suggested implementation prompt:
2727
- "Evaluate and optionally optimize @knighted/develop GitHub file upsert behavior. Compare metadata-first preflight GET+PUT against optimistic PUT with retry-on-missing-sha for existing files. Keep current reliability guarantees and avoid reintroducing noisy false-positive failures. If implementing a hybrid/configurable strategy, keep defaults conservative, update docs, and validate with npm run lint plus targeted Playwright PR drawer flows."
2828

29-
5. **Remove pre-multitab component/styles compatibility paths**
30-
- Delete code paths that preserve or translate legacy single-component/single-styles storage and sync behavior from before the multitab update.
31-
- Remove backward-compatibility shims, fallback field reads, and migration glue tied to old `componentFilePath`/`stylesFilePath`-style assumptions when equivalent tab-derived data exists.
32-
- Favor one canonical tab-first data contract across local storage, IndexedDB workspace records, PR sync metadata, and commit target derivation.
33-
- Accept breaking changes for old locally stored app state to simplify maintenance and reduce branching logic.
34-
- Suggested implementation prompt:
35-
- "Remove backwards-compatibility code in @knighted/develop that supports pre-multitab component/styles storage/sync behavior. Standardize on the current tab-derived schema only, delete legacy field fallbacks and migration helpers, and update tests/docs to match the simplified contract. Validate with npm run lint and targeted Playwright suites for workspace tabs + PR drawer flows."
36-
37-
6. **Promise handling conventions (consistency of intent)**
29+
5. **Promise handling conventions (consistency of intent)**
3830
- Define a project default: use `async`/`await` with `try`/`catch` for most async control flow.
3931
- Keep Promise chains where they better express intent (for example, fire-and-forget paths with explicit `.catch()` to avoid unhandled rejections, or concise pass-through composition).
4032
- Document this as an intent-first rule so mixed syntax is acceptable only when deliberate and easy to reason about.

playwright/github-pr-drawer.spec.ts

Lines changed: 44 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -968,8 +968,10 @@ test('Active PR context disconnect uses local-only confirmation flow', async ({
968968
localStorage.setItem(
969969
'knighted:develop:github-pr-config:knightedcodemonkey/develop',
970970
JSON.stringify({
971-
syncComponentFilePath: 'src/components/App.tsx',
972-
syncStylesFilePath: 'src/styles/app.css',
971+
syncTabTargets: [
972+
{ kind: 'component', path: 'src/components/App.tsx' },
973+
{ kind: 'styles', path: 'src/styles/app.css' },
974+
],
973975
renderMode: 'react',
974976
baseBranch: 'main',
975977
headBranch: 'develop/open-pr-test',
@@ -1141,8 +1143,10 @@ test('Active PR context updates controls and can be closed from AI controls', as
11411143
localStorage.setItem(
11421144
'knighted:develop:github-pr-config:knightedcodemonkey/develop',
11431145
JSON.stringify({
1144-
syncComponentFilePath: 'src/components/App.tsx',
1145-
syncStylesFilePath: 'src/styles/app.css',
1146+
syncTabTargets: [
1147+
{ kind: 'component', path: 'src/components/App.tsx' },
1148+
{ kind: 'styles', path: 'src/styles/app.css' },
1149+
],
11461150
renderMode: 'react',
11471151
baseBranch: 'main',
11481152
headBranch: 'develop/open-pr-test',
@@ -1231,8 +1235,10 @@ test('Active PR context is disabled on load when pull request is closed', async
12311235
localStorage.setItem(
12321236
'knighted:develop:github-pr-config:knightedcodemonkey/develop',
12331237
JSON.stringify({
1234-
syncComponentFilePath: 'src/components/App.tsx',
1235-
syncStylesFilePath: 'src/styles/app.css',
1238+
syncTabTargets: [
1239+
{ kind: 'component', path: 'src/components/App.tsx' },
1240+
{ kind: 'styles', path: 'src/styles/app.css' },
1241+
],
12361242
renderMode: 'react',
12371243
baseBranch: 'main',
12381244
headBranch: 'develop/open-pr-test',
@@ -1330,8 +1336,10 @@ test('Active PR context rehydrates after token remove and re-add', async ({ page
13301336
localStorage.setItem(
13311337
'knighted:develop:github-pr-config:knightedcodemonkey/css',
13321338
JSON.stringify({
1333-
syncComponentFilePath: 'src/components/App.tsx',
1334-
syncStylesFilePath: 'src/styles/app.css',
1339+
syncTabTargets: [
1340+
{ kind: 'component', path: 'src/components/App.tsx' },
1341+
{ kind: 'styles', path: 'src/styles/app.css' },
1342+
],
13351343
renderMode: 'react',
13361344
baseBranch: 'main',
13371345
headBranch: 'css/rehydrate-test',
@@ -1441,8 +1449,10 @@ test('Active PR context deactivates after token remove and re-add when PR is clo
14411449
localStorage.setItem(
14421450
'knighted:develop:github-pr-config:knightedcodemonkey/css',
14431451
JSON.stringify({
1444-
syncComponentFilePath: 'src/components/App.tsx',
1445-
syncStylesFilePath: 'src/styles/app.css',
1452+
syncTabTargets: [
1453+
{ kind: 'component', path: 'src/components/App.tsx' },
1454+
{ kind: 'styles', path: 'src/styles/app.css' },
1455+
],
14461456
renderMode: 'react',
14471457
baseBranch: 'main',
14481458
headBranch: 'css/rehydrate-test',
@@ -1555,8 +1565,10 @@ test('Active PR context recovers when saved head branch is missing but PR metada
15551565
localStorage.setItem(
15561566
'knighted:develop:github-pr-config:knightedcodemonkey/develop',
15571567
JSON.stringify({
1558-
syncComponentFilePath: 'src/components/App.tsx',
1559-
syncStylesFilePath: 'src/styles/app.css',
1568+
syncTabTargets: [
1569+
{ kind: 'component', path: 'src/components/App.tsx' },
1570+
{ kind: 'styles', path: 'src/styles/app.css' },
1571+
],
15601572
renderMode: 'react',
15611573
baseBranch: 'main',
15621574
headBranch: '',
@@ -1699,8 +1711,10 @@ test('Active PR context uses Push commit flow without creating a new pull reques
16991711
localStorage.setItem(
17001712
'knighted:develop:github-pr-config:knightedcodemonkey/develop',
17011713
JSON.stringify({
1702-
syncComponentFilePath: 'src/components/App.tsx',
1703-
syncStylesFilePath: 'src/styles/app.css',
1714+
syncTabTargets: [
1715+
{ kind: 'component', path: 'src/components/App.tsx' },
1716+
{ kind: 'styles', path: 'src/styles/app.css' },
1717+
],
17041718
renderMode: 'react',
17051719
baseBranch: 'main',
17061720
headBranch: 'develop/open-pr-test',
@@ -1936,8 +1950,10 @@ test('Active PR context push commit uses Git Database API atomic path by default
19361950
localStorage.setItem(
19371951
'knighted:develop:github-pr-config:knightedcodemonkey/develop',
19381952
JSON.stringify({
1939-
syncComponentFilePath: 'src/components/App.tsx',
1940-
syncStylesFilePath: 'src/styles/app.css',
1953+
syncTabTargets: [
1954+
{ kind: 'component', path: 'src/components/App.tsx' },
1955+
{ kind: 'styles', path: 'src/styles/app.css' },
1956+
],
19411957
renderMode: 'react',
19421958
baseBranch: 'main',
19431959
headBranch: 'develop/open-pr-test',
@@ -2098,8 +2114,10 @@ test('Reloaded active PR context from URL metadata keeps Push mode and status re
20982114
localStorage.setItem(
20992115
'knighted:develop:github-pr-config:knightedcodemonkey/develop',
21002116
JSON.stringify({
2101-
syncComponentFilePath: 'src/components/App.tsx',
2102-
syncStylesFilePath: 'src/styles/app.css',
2117+
syncTabTargets: [
2118+
{ kind: 'component', path: 'src/components/App.tsx' },
2119+
{ kind: 'styles', path: 'src/styles/app.css' },
2120+
],
21032121
renderMode: 'react',
21042122
baseBranch: 'main',
21052123
headBranch: 'develop/open-pr-test',
@@ -2241,8 +2259,10 @@ test('Reloaded active PR context syncs editor content from GitHub branch and res
22412259
localStorage.setItem(
22422260
'knighted:develop:github-pr-config:knightedcodemonkey/develop',
22432261
JSON.stringify({
2244-
syncComponentFilePath: 'src/components/App.tsx',
2245-
syncStylesFilePath: 'src/styles/app.css',
2262+
syncTabTargets: [
2263+
{ kind: 'component', path: 'src/components/App.tsx' },
2264+
{ kind: 'styles', path: 'src/styles/app.css' },
2265+
],
22462266
renderMode: 'react',
22472267
styleMode: 'sass',
22482268
baseBranch: 'main',
@@ -2326,8 +2346,10 @@ test('Reloaded active PR context falls back to css style mode for unsupported va
23262346
localStorage.setItem(
23272347
'knighted:develop:github-pr-config:knightedcodemonkey/develop',
23282348
JSON.stringify({
2329-
syncComponentFilePath: 'src/components/App.tsx',
2330-
syncStylesFilePath: 'src/styles/app.css',
2349+
syncTabTargets: [
2350+
{ kind: 'component', path: 'src/components/App.tsx' },
2351+
{ kind: 'styles', path: 'src/styles/app.css' },
2352+
],
23312353
renderMode: 'react',
23322354
styleMode: 'scss',
23332355
baseBranch: 'main',

src/app.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -755,8 +755,8 @@ const githubWorkflows = createGitHubWorkflowsSetup({
755755
void workspacesDrawerController?.setOpen(false)
756756
},
757757
getActivePrEditorSyncKey: () => githubAiContextState.activePrEditorSyncKey,
758-
syncFromActiveContext: ({ componentPath, stylesPath }) => {
759-
reconcileWorkspaceTabsWithEditorSync({ componentPath, stylesPath })
758+
syncFromActiveContext: ({ tabTargets }) => {
759+
reconcileWorkspaceTabsWithEditorSync({ tabTargets })
760760
},
761761
formatActivePrReference,
762762
githubPrContextClose,

src/modules/app-core/github-workflows.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,8 +204,7 @@ const initializeGitHubWorkflows = ({
204204
prContextUi.markActivePrEditorContentSynced()
205205

206206
syncFromActiveContext({
207-
componentPath: args?.syncTargets?.componentFilePath,
208-
stylesPath: args?.syncTargets?.stylesFilePath,
207+
tabTargets: args?.syncTargets?.tabTargets,
209208
})
210209
}
211210

src/modules/app-core/workspace-sync-controller.js

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -134,26 +134,38 @@ const createWorkspaceSyncController = ({
134134
}
135135

136136
const getEditorSyncTargets = () => {
137-
const componentTab = getWorkspaceTabByKind('component')
138-
const stylesTab = getWorkspaceTabByKind('styles')
137+
const tabTargets = []
139138

140-
return {
141-
componentFilePath:
142-
getTabTargetPrFilePath(componentTab) ||
143-
normalizeWorkspacePathValue(componentTab?.path) ||
144-
'',
145-
stylesFilePath:
146-
getTabTargetPrFilePath(stylesTab) ||
147-
normalizeWorkspacePathValue(stylesTab?.path) ||
148-
'',
139+
for (const kind of ['component', 'styles']) {
140+
const tab = getWorkspaceTabByKind(kind)
141+
const path =
142+
getTabTargetPrFilePath(tab) || normalizeWorkspacePathValue(tab?.path) || ''
143+
144+
if (!path) {
145+
continue
146+
}
147+
148+
tabTargets.push({ kind, path })
149149
}
150+
151+
return { tabTargets }
150152
}
151153

152-
const reconcileWorkspaceTabsWithEditorSync = ({ componentPath, stylesPath } = {}) => {
153-
const normalizedComponentPath = normalizeWorkspacePathValue(componentPath)
154-
const normalizedStylesPath = normalizeWorkspacePathValue(stylesPath)
154+
const reconcileWorkspaceTabsWithEditorSync = ({ tabTargets } = {}) => {
155+
const targetsByKind = new Map()
156+
const normalizedTargets = Array.isArray(tabTargets) ? tabTargets : []
157+
158+
for (const target of normalizedTargets) {
159+
const kind = toNonEmptyWorkspaceText(target?.kind)
160+
const normalizedPath = normalizeWorkspacePathValue(target?.path)
161+
if (!kind || !normalizedPath) {
162+
continue
163+
}
155164

156-
if (!normalizedComponentPath && !normalizedStylesPath) {
165+
targetsByKind.set(kind, normalizedPath)
166+
}
167+
168+
if (targetsByKind.size === 0) {
157169
return 0
158170
}
159171

@@ -165,8 +177,7 @@ const createWorkspaceSyncController = ({
165177

166178
const nextTabs = workspaceTabsState.getTabs().map(tab => {
167179
const tabKind = getTabKind(tab)
168-
const expectedPath =
169-
tabKind === 'styles' ? normalizedStylesPath : normalizedComponentPath
180+
const expectedPath = targetsByKind.get(tabKind)
170181
if (!expectedPath) {
171182
return tab
172183
}
@@ -184,6 +195,7 @@ const createWorkspaceSyncController = ({
184195
updatedTabCount += 1
185196
return {
186197
...tab,
198+
targetPrFilePath: matchedPath,
187199
content: syncedContent,
188200
syncedContent,
189201
isDirty: false,

src/modules/github/github-pr-drawer.js

Lines changed: 48 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,14 @@ const supportedStyleModes = new Set(['css', 'module', 'less', 'sass'])
2424

2525
const toSafeText = value => (typeof value === 'string' ? value.trim() : '')
2626

27+
const toPullRequestNumber = value => {
28+
if (typeof value === 'number' && Number.isFinite(value) && value > 0) {
29+
return value
30+
}
31+
32+
return null
33+
}
34+
2735
const normalizeRenderMode = value => {
2836
const mode = toSafeText(value).toLowerCase()
2937
return supportedRenderModes.has(mode) ? mode : 'dom'
@@ -102,6 +110,26 @@ const saveRepositoryPrConfig = ({ repositoryFullName, config }) => {
102110
}
103111
}
104112

113+
const sanitizeRepositoryPrConfig = config => {
114+
const source = config && typeof config === 'object' ? config : {}
115+
const pullRequestUrl = toSafeText(source.pullRequestUrl)
116+
const fallbackPullRequestNumber = parsePullRequestNumberFromUrl(pullRequestUrl)
117+
const pullRequestNumber =
118+
toPullRequestNumber(source.pullRequestNumber) ?? fallbackPullRequestNumber
119+
120+
return {
121+
baseBranch: toSafeText(source.baseBranch),
122+
headBranch: sanitizeBranchPart(source.headBranch),
123+
prTitle: toSafeText(source.prTitle),
124+
prBody: typeof source.prBody === 'string' ? source.prBody.trim() : '',
125+
renderMode: normalizeRenderMode(source.renderMode),
126+
styleMode: normalizeStyleMode(source.styleMode),
127+
isActivePr: source.isActivePr === true,
128+
pullRequestNumber,
129+
pullRequestUrl,
130+
}
131+
}
132+
105133
const removeRepositoryPrConfig = repositoryFullName => {
106134
if (typeof repositoryFullName !== 'string' || !repositoryFullName.trim()) {
107135
return
@@ -703,8 +731,15 @@ export const createGitHubPrDrawer = ({
703731

704732
const syncTargets =
705733
typeof getEditorSyncTargets === 'function' ? getEditorSyncTargets() : null
706-
const componentSyncPath = toSafeText(syncTargets?.componentFilePath)
707-
const stylesSyncPath = toSafeText(syncTargets?.stylesFilePath)
734+
const tabSyncTargets = Array.isArray(syncTargets?.tabTargets)
735+
? syncTargets.tabTargets
736+
: []
737+
const componentSyncPath = toSafeText(
738+
tabSyncTargets.find(target => toSafeText(target?.kind) === 'component')?.path,
739+
)
740+
const stylesSyncPath = toSafeText(
741+
tabSyncTargets.find(target => toSafeText(target?.kind) === 'styles')?.path,
742+
)
708743

709744
if (!componentSyncPath || !stylesSyncPath) {
710745
lastActiveContentSyncKey = ''
@@ -734,8 +769,10 @@ export const createGitHubPrDrawer = ({
734769
repository,
735770
activeContext,
736771
syncTargets: {
737-
componentFilePath: componentSyncPath,
738-
stylesFilePath: stylesSyncPath,
772+
tabTargets: [
773+
{ kind: 'component', path: componentSyncPath },
774+
{ kind: 'styles', path: stylesSyncPath },
775+
],
739776
},
740777
signal: abortController.signal,
741778
})
@@ -837,6 +874,7 @@ export const createGitHubPrDrawer = ({
837874
}
838875

839876
if (resolvedPullRequest?.isOpen) {
877+
const normalizedSavedConfig = sanitizeRepositoryPrConfig(savedConfig)
840878
const nextHeadBranch =
841879
sanitizeBranchPart(resolvedPullRequest.headRef) || headBranch
842880
const nextBaseBranch =
@@ -845,10 +883,8 @@ export const createGitHubPrDrawer = ({
845883
saveRepositoryPrConfig({
846884
repositoryFullName,
847885
config: {
848-
...savedConfig,
886+
...normalizedSavedConfig,
849887
isActivePr: true,
850-
renderMode: normalizeRenderMode(savedConfig.renderMode),
851-
styleMode: normalizeStyleMode(savedConfig.styleMode),
852888
headBranch: nextHeadBranch,
853889
baseBranch: nextBaseBranch,
854890
pullRequestNumber: resolvedPullRequest.number,
@@ -867,7 +903,7 @@ export const createGitHubPrDrawer = ({
867903
saveRepositoryPrConfig({
868904
repositoryFullName,
869905
config: {
870-
...savedConfig,
906+
...sanitizeRepositoryPrConfig(savedConfig),
871907
isActivePr: false,
872908
},
873909
})
@@ -1147,13 +1183,14 @@ export const createGitHubPrDrawer = ({
11471183
const currentRenderMode = normalizeRenderMode(getRenderMode?.())
11481184
const currentStyleMode = normalizeStyleMode(getStyleMode?.())
11491185
const existingConfig = readRepositoryPrConfig(repositoryFullName)
1186+
const normalizedExistingConfig = sanitizeRepositoryPrConfig(existingConfig)
11501187
const isActivePr = existingConfig?.isActivePr === true
11511188

11521189
if (isActivePr) {
11531190
saveRepositoryPrConfig({
11541191
repositoryFullName,
11551192
config: {
1156-
...existingConfig,
1193+
...normalizedExistingConfig,
11571194
renderMode: currentRenderMode,
11581195
styleMode: currentStyleMode,
11591196
isActivePr: true,
@@ -1558,6 +1595,7 @@ export const createGitHubPrDrawer = ({
15581595
}
15591596

15601597
const savedConfig = readRepositoryPrConfig(repositoryFullName)
1598+
const normalizedSavedConfig = sanitizeRepositoryPrConfig(savedConfig)
15611599
const previousActiveContext =
15621600
savedConfig?.isActivePr === true
15631601
? {
@@ -1574,7 +1612,7 @@ export const createGitHubPrDrawer = ({
15741612
saveRepositoryPrConfig({
15751613
repositoryFullName,
15761614
config: {
1577-
...savedConfig,
1615+
...normalizedSavedConfig,
15781616
isActivePr: false,
15791617
},
15801618
})

0 commit comments

Comments
 (0)