Skip to content

Commit e102129

Browse files
committed
test(skillgym): require adjacency for freshness-sensitive ref/coordinate steps
assertSequentialCommandSteps only required each matcher at a later line, so a plan could snapshot -i (minting @e30), run an unrelated state-changing command like home, and still press the now-stale @e30 and pass. Same gap between screenshot and its raw-coordinate fallback. Add a requireAdjacent flag to SequentialCommandStep and mark the four freshness-sensitive pairs (snapshot -> press @e30, snapshot -> fill @e31, screenshot -> raw-coordinate press, snapshot -> press @e35) as adjacent, so an intervening state-changing command fails the case. Split assertSequentialCommandSteps into findStepMatchIndex/ describeStepPosition to keep it under the Fallow Code Quality complexity gate. Verified locally with `npx fallow audit --base origin/main` (clean) and eight synthetic finalOutput plans, including the reviewer's two described staleness scenarios (stale ref after an intervening `home`, stale coordinate after an intervening press between screenshot and tap), which now fail as expected; the two valid orderings still pass.
1 parent 7def97b commit e102129

1 file changed

Lines changed: 43 additions & 13 deletions

File tree

test/skillgym/suites/agent-device-smoke-suite.ts

Lines changed: 43 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -454,13 +454,41 @@ const IOS_TEST_APP_DEV_BUILD_OPEN = new RegExp(
454454
interface SequentialCommandStep {
455455
label: string;
456456
regex: RegExp;
457+
/**
458+
* Require this step's line to be the very next command (no intervening commands at all).
459+
* Use this for a freshness-sensitive observation/action pair — for example a ref press that
460+
* must immediately follow the snapshot that minted it, or a coordinate action that must
461+
* immediately follow the screenshot it was read from — so a state-changing command in between
462+
* (which could invalidate the ref/coordinate) fails the case instead of silently passing.
463+
*/
464+
requireAdjacent?: boolean;
465+
}
466+
467+
function findStepMatchIndex(
468+
lines: readonly string[],
469+
step: SequentialCommandStep,
470+
cursor: number,
471+
): number {
472+
const searchEnd = step.requireAdjacent === true ? cursor + 1 : lines.length;
473+
for (let i = cursor; i < searchEnd; i++) {
474+
if (step.regex.test(lines[i]!)) return i;
475+
}
476+
return -1;
477+
}
478+
479+
function describeStepPosition(step: SequentialCommandStep, cursor: number): string {
480+
return step.requireAdjacent === true
481+
? `immediately at position ${cursor} (no intervening commands)`
482+
: `at or after position ${cursor}`;
457483
}
458484

459485
/**
460486
* Enforces that each step's pattern appears, in order, at or after the position where the
461487
* previous step matched — unlike the presence-only OutputMatcher checks above, this catches a
462488
* plan that satisfies every individual requirement out of order (for example an early,
463-
* unrelated snapshot standing in for the one that must follow a specific action).
489+
* unrelated snapshot standing in for the one that must follow a specific action). Steps marked
490+
* requireAdjacent must match at that exact position, catching a stale ref/coordinate used after
491+
* an intervening state-changing command.
464492
*/
465493
function assertSequentialCommandSteps(
466494
finalOutput: string,
@@ -473,16 +501,10 @@ function assertSequentialCommandSteps(
473501
.filter(Boolean);
474502
let cursor = 0;
475503
for (const step of steps) {
476-
let matchIndex = -1;
477-
for (let i = cursor; i < lines.length; i++) {
478-
if (step.regex.test(lines[i]!)) {
479-
matchIndex = i;
480-
break;
481-
}
482-
}
504+
const matchIndex = findStepMatchIndex(lines, step, cursor);
483505
assert.ok(
484506
matchIndex !== -1,
485-
`${caseLabel}: expected step "${step.label}" at or after position ${cursor}. Observed commands:\n${lines.join('\n')}`,
507+
`${caseLabel}: expected step "${step.label}" ${describeStepPosition(step, cursor)}. Observed commands:\n${lines.join('\n')}`,
486508
);
487509
cursor = matchIndex + 1;
488510
}
@@ -2719,32 +2741,40 @@ Do not combine final commands with shell operators such as &&, ||, pipes, or sem
27192741
regex: /^(?:agent-device\s+)?snapshot\b.*-i\b/i,
27202742
},
27212743
{
2722-
label: 'press/click @e30 to open the widget gallery',
2744+
label:
2745+
'press/click @e30 to open the widget gallery, immediately after the snapshot that minted it',
27232746
regex: /^(?:agent-device\s+)?(?:press|click)\s+@e30\b/i,
2747+
requireAdjacent: true,
27242748
},
27252749
{
27262750
label: 'fresh snapshot -i exposing @e31',
27272751
regex: /^(?:agent-device\s+)?snapshot\b.*-i\b/i,
27282752
},
27292753
{
2730-
label: 'fill @e31 with "Calendar" to search the gallery',
2754+
label:
2755+
'fill @e31 with "Calendar" to search the gallery, immediately after the snapshot that minted it',
27312756
regex: /^(?:agent-device\s+)?fill\s+@e31\b.*calendar/i,
2757+
requireAdjacent: true,
27322758
},
27332759
{
27342760
label: 'screenshot before the coordinate fallback',
27352761
regex: /^(?:agent-device\s+)?screenshot\b/i,
27362762
},
27372763
{
2738-
label: 'raw-coordinate press on the unlabeled search-result row',
2764+
label:
2765+
'raw-coordinate press on the unlabeled search-result row, immediately after the screenshot it was read from',
27392766
regex: /^(?:agent-device\s+)?(?:press|click)\s+-?\d+(?:\.\d+)?\s+-?\d+(?:\.\d+)?\b/i,
2767+
requireAdjacent: true,
27402768
},
27412769
{
27422770
label: 'fresh snapshot -i exposing @e35',
27432771
regex: /^(?:agent-device\s+)?snapshot\b.*-i\b/i,
27442772
},
27452773
{
2746-
label: 'press/click @e35 to place the widget',
2774+
label:
2775+
'press/click @e35 to place the widget, immediately after the snapshot that minted it',
27472776
regex: /^(?:agent-device\s+)?(?:press|click)\s+@e35\b/i,
2777+
requireAdjacent: true,
27482778
},
27492779
{
27502780
label: 'reopen Agent Device Tester',

0 commit comments

Comments
 (0)