mirror of
https://github.com/screenci/screenci.git
synced 2026-09-19 08:57:46 +08:00
feat(codeSync): soft-skip stale override keys instead of hard-failing
An override whose key is absent from the current recording snapshot (its
action was removed, or an ordinal-keyed target such as a waitForTimeout
delay drifted since the edit was authored) was reported as a hard
'unstamped-action' failure. That surfaced a permanent editor error the
user had to clear by hand, even though there was no call site to write
and nothing wrong.
Introduce a typed 'orphaned-override' reason for the absent-key case and
distinguish it from a genuine unstamped action (present in the snapshot
but with no editId) and a slug#N loop repeat. applyCodegenRequest now
returns { outcome: 'orphaned' } instead of throwing when every refusal is
that soft kind, and the dev listener reports it as 'orphaned' so the
request is auto-discarded rather than failed.
This commit is contained in:
@@ -220,6 +220,34 @@ describe('runDevListenLoop', () => {
|
||||
])
|
||||
})
|
||||
|
||||
it('reports a stale-key edit as orphaned (soft skip), not failed', async () => {
|
||||
const controller = { stopped: false }
|
||||
const applyCodegen = vi.fn(async () => ({ outcome: 'orphaned' as const }))
|
||||
const deps = makeDeps({ applyCodegen })
|
||||
deps.fetchMock.mockImplementation(async (url: string) => {
|
||||
if (url.endsWith('/cli/dev/poll')) {
|
||||
if (deps.fetchMock.mock.calls.length === 1) {
|
||||
return jsonResponse({
|
||||
trigger: null,
|
||||
codegenRequests: [codegenRequest],
|
||||
})
|
||||
}
|
||||
controller.stopped = true
|
||||
return jsonResponse({ trigger: null })
|
||||
}
|
||||
return jsonResponse({ ok: true })
|
||||
})
|
||||
|
||||
await runDevListenLoop(config, deps, 'lst_1', controller)
|
||||
|
||||
const reports = deps.fetchMock.mock.calls
|
||||
.filter(([url]) => (url as string).endsWith('/cli/dev/report-codegen'))
|
||||
.map(([, init]) => JSON.parse((init as RequestInit).body as string))
|
||||
expect(reports).toEqual([
|
||||
expect.objectContaining({ requestId: 'cgr_1', state: 'orphaned' }),
|
||||
])
|
||||
})
|
||||
|
||||
it('logs attribution when a deferred edit carries queuedBy', async () => {
|
||||
const controller = { stopped: false }
|
||||
const applyCodegen = vi.fn(async () => {})
|
||||
|
||||
@@ -3397,7 +3397,7 @@ export async function runDevCommand(
|
||||
loadTypescript,
|
||||
dirname(screenciDir)
|
||||
)
|
||||
await applyCodegenRequest(request, {
|
||||
return await applyCodegenRequest(request, {
|
||||
ts,
|
||||
formatFile: createProjectFormatter(dirname(resolvedConfigPath), {
|
||||
warn: (message) => logger.warn(message),
|
||||
|
||||
+10
-2
@@ -504,12 +504,20 @@ Each edit is applied to code the moment it is saved: the dev session locates
|
||||
the call site by editId and writes the call-position statement into the
|
||||
source. An edit that cannot be applied fails the codegen request and the
|
||||
editor reverts the optimistic value instead of dropping it silently. The
|
||||
failure carries a typed reason plus a message, so the editor toast says what
|
||||
to fix: `unknown-edit-id`, `ambiguous-edit-id`, `inside-control-flow`,
|
||||
failure carries a typed reason plus a message, surfaced in the editor's
|
||||
pending-sync queue (with per-row retry/discard and a clear-all), so the user
|
||||
sees what to fix: `unknown-edit-id`, `ambiguous-edit-id`, `inside-control-flow`,
|
||||
`unstamped-action`, `loop-repeat`, `unsupported-field`, `invalid-edit`,
|
||||
`unresolved-import` (the effect function needs a named import from
|
||||
'screenci'), `unknown-video`, `app-managed`, or `unsupported-shape`.
|
||||
|
||||
One reason is not a failure: `orphaned-override` means the override's key is
|
||||
absent from the current recording snapshot (its action was removed, or an
|
||||
ordinal-keyed target such as a `waitForTimeout` delay drifted since the edit
|
||||
was authored). There is no call site to write, so the request is reported as a
|
||||
soft skip and auto-discarded (resolved as superseded, no re-record) rather than
|
||||
surfaced as something the user must clear by hand.
|
||||
|
||||
Aliased imports are supported throughout: a file that does
|
||||
`import { autoZoom as az } from 'screenci'` has its `az(...)` wraps
|
||||
recognised, updated, and unwrapped like the canonical name, and codegen reuses
|
||||
|
||||
@@ -317,6 +317,71 @@ describe('applyCodegenRequest: typed refusal reasons in errors', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('applyCodegenRequest: orphaned (stale key) soft skip', () => {
|
||||
async function applyReturning(editJson: string) {
|
||||
const writes: Record<string, string> = {}
|
||||
const result = await applyCodegenRequest(
|
||||
{
|
||||
requestId: 'req1',
|
||||
videoName: 'Demo',
|
||||
editId: 'param|delay|||0',
|
||||
editJson,
|
||||
requiresRecord: true,
|
||||
},
|
||||
{
|
||||
ts,
|
||||
readFile: (path) => (path === FILE ? SOURCE : null),
|
||||
writeFile: (path, content) => {
|
||||
writes[path] = content
|
||||
},
|
||||
editableSnapshot: SNAPSHOT,
|
||||
}
|
||||
)
|
||||
return { result, writes }
|
||||
}
|
||||
|
||||
// A paramEdit whose target key is absent from the current recording snapshot
|
||||
// (SNAPSHOT only knows 'fill1'). The action drifted or was removed; there is
|
||||
// no call site to touch.
|
||||
const staleDelayEdit = JSON.stringify({
|
||||
type: 'paramEdit',
|
||||
id: 'param|delay|||0',
|
||||
target: { key: 'delay|||0' },
|
||||
fields: { durationMs: 500 },
|
||||
})
|
||||
|
||||
it('returns { outcome: orphaned } instead of throwing', async () => {
|
||||
const { result, writes } = await applyReturning(staleDelayEdit)
|
||||
expect(result).toEqual({ outcome: 'orphaned' })
|
||||
expect(writes).toEqual({})
|
||||
})
|
||||
|
||||
it('returns { outcome: applied } for a real edit that writes', async () => {
|
||||
const record = JSON.stringify({
|
||||
type: 'paramEdit',
|
||||
id: 'p1',
|
||||
target: { key: 'fill1' },
|
||||
fields: { moveDuration: 400 },
|
||||
})
|
||||
const result = await applyCodegenRequest(
|
||||
{
|
||||
requestId: 'req1',
|
||||
videoName: 'Demo',
|
||||
editId: 'param|fill1',
|
||||
editJson: record,
|
||||
requiresRecord: true,
|
||||
},
|
||||
{
|
||||
ts,
|
||||
readFile: (path) => (path === FILE ? SOURCE : null),
|
||||
writeFile: () => {},
|
||||
editableSnapshot: SNAPSHOT,
|
||||
}
|
||||
)
|
||||
expect(result).toEqual({ outcome: 'applied' })
|
||||
})
|
||||
})
|
||||
|
||||
describe('applyCodegenRequest: duplicate editId self-heal', () => {
|
||||
const DUP_SOURCE = [
|
||||
"import { video } from 'screenci'",
|
||||
|
||||
+20
-2
@@ -9,6 +9,11 @@
|
||||
* uses; the call site is located via the editable entries of the video's kept
|
||||
* recording data. Throws when the edit cannot be applied, so the listener
|
||||
* reports the request failed and the editor reverts the optimistic value.
|
||||
*
|
||||
* Returns `{ outcome: 'orphaned' }` (instead of throwing) when the only reason
|
||||
* the edit could not be applied is that its target is absent from the current
|
||||
* recording snapshot: a stale override key, which the listener reports as a
|
||||
* soft skip so the request is auto-discarded rather than surfaced as a failure.
|
||||
*/
|
||||
import { planCodeSync } from './codeSync.js'
|
||||
import type { TsModule } from './codemod.js'
|
||||
@@ -60,10 +65,12 @@ export function requireTypescriptForCodegen(
|
||||
return ts
|
||||
}
|
||||
|
||||
export type ApplyCodegenOutcome = { outcome: 'applied' | 'orphaned' }
|
||||
|
||||
export async function applyCodegenRequest(
|
||||
request: DevCodegenRequest,
|
||||
deps: ApplyCodegenDeps
|
||||
): Promise<void> {
|
||||
): Promise<ApplyCodegenOutcome> {
|
||||
let record: unknown
|
||||
try {
|
||||
record = JSON.parse(request.editJson)
|
||||
@@ -138,7 +145,17 @@ export async function applyCodegenRequest(
|
||||
}
|
||||
|
||||
if (plan.unappliable.length > 0) {
|
||||
const reasons = plan.unappliable
|
||||
// A stale override key (its action left the recording) is not a failure:
|
||||
// there is nothing to write and nothing the user did wrong. When every
|
||||
// refusal is that soft kind, report it as orphaned so the request is
|
||||
// auto-discarded. Any other refusal in the mix is still a hard failure.
|
||||
const hard = plan.unappliable.filter(
|
||||
(item) => item.reason !== 'orphaned-override'
|
||||
)
|
||||
if (hard.length === 0) {
|
||||
return { outcome: 'orphaned' }
|
||||
}
|
||||
const reasons = hard
|
||||
.map((item) => `[${item.reason}] ${item.message}`)
|
||||
.join('; ')
|
||||
throw new Error(
|
||||
@@ -154,4 +171,5 @@ export async function applyCodegenRequest(
|
||||
: file.after
|
||||
deps.writeFile(file.path, content)
|
||||
}
|
||||
return { outcome: 'applied' }
|
||||
}
|
||||
|
||||
@@ -1983,8 +1983,27 @@ describe('planCodeSync: typed refusal reasons', () => {
|
||||
})
|
||||
|
||||
it("reports 'loop-repeat' and 'unstamped-action' for locked keys", () => {
|
||||
// `unstamped-one` is present in the snapshot but carries no editId (a
|
||||
// genuine unstamped action, stamping pending), which is distinct from a
|
||||
// key the snapshot has never heard of (that is 'orphaned-override').
|
||||
const snapshotWithUnstamped: EditableSnapshot = {
|
||||
version: 1,
|
||||
videos: {
|
||||
...EDITABLE_SNAPSHOT.videos,
|
||||
Demo: [
|
||||
...EDITABLE_SNAPSHOT.videos.Demo!,
|
||||
{
|
||||
key: 'unstamped-one',
|
||||
locked: false,
|
||||
defaults: { sleepBefore: 0 },
|
||||
source: { file: FILE, line: 4 },
|
||||
},
|
||||
],
|
||||
},
|
||||
}
|
||||
const result = plan(
|
||||
inputWith({
|
||||
editableSnapshot: snapshotWithUnstamped,
|
||||
editableOverrides: {
|
||||
Demo: [
|
||||
{ key: 'click1#1', values: { sleepBefore: 500 } },
|
||||
@@ -1997,6 +2016,21 @@ describe('planCodeSync: typed refusal reasons', () => {
|
||||
expect(reasons).toEqual(['loop-repeat', 'unstamped-action'])
|
||||
})
|
||||
|
||||
it("reports 'orphaned-override' for a key absent from the snapshot", () => {
|
||||
// A stale override (its action was removed, or an ordinal-keyed delay
|
||||
// drifted): a soft skip the caller auto-discards, not a hard failure.
|
||||
const result = plan(
|
||||
inputWith({
|
||||
editableOverrides: {
|
||||
Demo: [{ key: 'delay|||0', values: { durationMs: 500 } }],
|
||||
},
|
||||
})
|
||||
)
|
||||
expect(result.unappliable.map((item) => item.reason)).toEqual([
|
||||
'orphaned-override',
|
||||
])
|
||||
})
|
||||
|
||||
it("reports 'unsupported-field' for a field with no code form", () => {
|
||||
const result = plan(
|
||||
inputWith({
|
||||
|
||||
+55
-22
@@ -200,6 +200,9 @@ export type AppliedItem = {
|
||||
* - `ambiguous-edit-id`: the editId appears more than once in the file.
|
||||
* - `inside-control-flow`: the call sits inside a loop, branch, or ternary.
|
||||
* - `unstamped-action`: the action carries no editId yet (stamping pending).
|
||||
* - `orphaned-override`: the override's key is absent from the current
|
||||
* recording snapshot (the action was removed, or its ordinal drifted since
|
||||
* the edit was authored). Nothing to write; a soft skip, not a failure.
|
||||
* - `loop-repeat`: the target is a `slug#N` repeat execution of a loop.
|
||||
* - `unsupported-field`: the edited param field has no code representation.
|
||||
* - `invalid-edit`: the edit record itself is incomplete or invalid.
|
||||
@@ -218,6 +221,7 @@ export type UnappliableReason =
|
||||
| 'ambiguous-edit-id'
|
||||
| 'inside-control-flow'
|
||||
| 'unstamped-action'
|
||||
| 'orphaned-override'
|
||||
| 'loop-repeat'
|
||||
| 'unsupported-field'
|
||||
| 'invalid-edit'
|
||||
@@ -1554,14 +1558,53 @@ export function planCodeSync(
|
||||
// ── Timeline param edits (sleepBefore, autoZoom offsets) ───────────────
|
||||
for (const override of input.editableOverrides[videoName] ?? []) {
|
||||
const entry = byKey.get(override.key)
|
||||
const editId = entry?.editId
|
||||
|
||||
// A `slug#N` loop repeat execution has no call site of its own: locked,
|
||||
// regardless of whether the snapshot carries the repeat. Checked before
|
||||
// the orphaned test below so a repeat key is never mistaken for a stale
|
||||
// one.
|
||||
if (override.key.includes('#')) {
|
||||
for (const [field, value] of Object.entries(override.values)) {
|
||||
if (value === undefined) continue
|
||||
if (entry !== undefined && jsonEqual(entry.defaults[field], value)) {
|
||||
continue
|
||||
}
|
||||
markUnappliable(
|
||||
'loop-repeat',
|
||||
`locked param edit '${override.key}' ${field}: the action is ` +
|
||||
`a loop repeat execution and has no call site of its own`
|
||||
)
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
// The override's key is not in the current recording snapshot: the action
|
||||
// it targeted was removed, or its ordinal drifted since the edit was
|
||||
// authored (this is the classic failure mode of ordinal-keyed delays).
|
||||
// There is no call site to write, but it is not a real failure either, so
|
||||
// it is reported as a soft skip the caller auto-discards rather than a
|
||||
// hard refusal the editor surfaces and the user must clear by hand.
|
||||
if (entry === undefined) {
|
||||
const changed = Object.entries(override.values).some(
|
||||
([, value]) => value !== undefined
|
||||
)
|
||||
if (changed) {
|
||||
markUnappliable(
|
||||
'orphaned-override',
|
||||
`override '${override.key}': no matching action in the current ` +
|
||||
`recording (the action was removed or its position drifted)`
|
||||
)
|
||||
}
|
||||
continue
|
||||
}
|
||||
const editId = entry.editId
|
||||
|
||||
// Recorded delay (waitForTimeout): durationMs rewrites the numeric arg in
|
||||
// place (or removes the whole call at 0). The wait has no editId of its
|
||||
// own, so it is located via a stamped neighbor action. Dragging an
|
||||
// interaction along the timeline commits durationMs on the adjacent delay,
|
||||
// which is how a move re-expresses as resizing the neighboring sleep.
|
||||
if (entry?.schemaKind === 'delay') {
|
||||
if (entry.schemaKind === 'delay') {
|
||||
const value = override.values.durationMs
|
||||
if (
|
||||
typeof value === 'number' &&
|
||||
@@ -1622,33 +1665,23 @@ export function planCodeSync(
|
||||
continue
|
||||
}
|
||||
|
||||
// No editId (unstamped action or a `slug#N` loop repeat execution):
|
||||
// never guess at a call site; the section is locked.
|
||||
if (editId === undefined || override.key.includes('#')) {
|
||||
// Present in the snapshot but with no editId: a genuine unstamped action
|
||||
// (stamping pending). Never guess at a call site; the section is locked.
|
||||
if (editId === undefined) {
|
||||
for (const [field, value] of Object.entries(override.values)) {
|
||||
if (value === undefined) continue
|
||||
if (entry !== undefined && jsonEqual(entry.defaults[field], value)) {
|
||||
continue
|
||||
}
|
||||
if (override.key.includes('#')) {
|
||||
markUnappliable(
|
||||
'loop-repeat',
|
||||
`locked param edit '${override.key}' ${field}: the action is ` +
|
||||
`a loop repeat execution and has no call site of its own`
|
||||
)
|
||||
} else {
|
||||
markUnappliable(
|
||||
'unstamped-action',
|
||||
`locked param edit '${override.key}' ${field}: the action ` +
|
||||
`carries no editId yet`
|
||||
)
|
||||
}
|
||||
if (jsonEqual(entry.defaults[field], value)) continue
|
||||
markUnappliable(
|
||||
'unstamped-action',
|
||||
`locked param edit '${override.key}' ${field}: the action ` +
|
||||
`carries no editId yet`
|
||||
)
|
||||
}
|
||||
continue
|
||||
}
|
||||
for (const [field, value] of Object.entries(override.values)) {
|
||||
if (value === undefined) continue
|
||||
if (jsonEqual(entry!.defaults[field], value)) {
|
||||
if (jsonEqual(entry.defaults[field], value)) {
|
||||
continue // in sync, nothing to do
|
||||
}
|
||||
if (field === 'sleepBefore' && typeof value === 'number' && value > 0) {
|
||||
|
||||
+27
-4
@@ -96,8 +96,14 @@ export type DevListenDeps = {
|
||||
* supersedes it with a newer trigger.
|
||||
*/
|
||||
runRecord: (trigger: DevTrigger, signal?: AbortSignal) => Promise<void>
|
||||
/** Applies one codegen request to the test source; throws on failure. */
|
||||
applyCodegen?: (request: DevCodegenRequest) => Promise<void>
|
||||
/**
|
||||
* Applies one codegen request to the test source; throws on failure.
|
||||
* Resolves with `{ outcome: 'orphaned' }` when the edit targets an action no
|
||||
* longer in the recording (a stale key), so the listener soft-skips it.
|
||||
*/
|
||||
applyCodegen?: (
|
||||
request: DevCodegenRequest
|
||||
) => Promise<{ outcome: 'applied' | 'orphaned' } | void>
|
||||
/**
|
||||
* Drains one pending machine-local record request (from the source-file
|
||||
* watcher). Checked every poll iteration; local requests share the record
|
||||
@@ -222,7 +228,7 @@ export async function reportDevCodegen(
|
||||
deps: DevListenDeps,
|
||||
listenerId: string,
|
||||
requestId: string,
|
||||
state: 'applied' | 'failed',
|
||||
state: 'applied' | 'failed' | 'orphaned',
|
||||
errorMessage?: string
|
||||
): Promise<void> {
|
||||
await postDev(config, deps, '/cli/dev/report-codegen', {
|
||||
@@ -297,7 +303,24 @@ async function handleCodegenRequest(
|
||||
return
|
||||
}
|
||||
try {
|
||||
await deps.applyCodegen(request)
|
||||
const result = await deps.applyCodegen(request)
|
||||
if (result != null && result.outcome === 'orphaned') {
|
||||
// The edit targeted an action no longer in the recording (a stale key).
|
||||
// Nothing to write; report a soft skip so the request is auto-discarded
|
||||
// instead of surfacing as a failure the user has to clear.
|
||||
await reportDevCodegen(
|
||||
config,
|
||||
deps,
|
||||
listenerId,
|
||||
request.requestId,
|
||||
'orphaned'
|
||||
)
|
||||
deps.logger.info(
|
||||
`Skipped ${describeEditId(request.editId)} on "${request.videoName}": ` +
|
||||
`its action is no longer in the recording.`
|
||||
)
|
||||
return
|
||||
}
|
||||
await reportDevCodegen(
|
||||
config,
|
||||
deps,
|
||||
|
||||
Reference in New Issue
Block a user