Skip to content

Commit a18669f

Browse files
Lykhoydaclaude
andauthored
feat(run-action): forceReload flag acknowledges human edits as new baseline (#176)
Closes #173 (sub-issue 3). Symptom: cdp_run_action refuses to complete with "yaml at ... has been edited externally since the in-memory action was loaded" every time the human touches the YAML during composition. The Phase 129 guardrail in cdp_repair_action — added to protect offline human edits from auto-repair clobber — can't distinguish "human edited offline, protect" from "human is actively composing, use latest". Forces users to fall back to raw maestro_run, losing auto-repair telemetry. Fix: new `forceReload: boolean` arg on cdp_run_action (default true). When true, the orchestrator stats the YAML on load and persists its current mtime to the sidecar's lastSeenMtimeMs BEFORE running. Any downstream actionWasEditedExternally() check returns false, so repair proceeds normally on top of the human's latest edit. Pass `forceReload: false` to opt back into the strict Phase 129 behavior (refuse to persist over an externally-modified YAML). The right choice for CI replays of fixed baselines and any flow where offline-edit protection matters more than active-composition ergonomics. Why default true: the high-frequency friction is the active-composition case. The protection case is the minority. Defaulting true fixes the reported pain without requiring callers to remember a flag every call. codex-pair flagged this design call as a HIGH during review; it's a deliberate user-approved tradeoff (the strict behavior is opt-in for callers who specifically need it). Implementation: - new acknowledgeExternalEdit(action) helper in action-store.ts — stats YAML, persists markSeen(state, currentMtime) to sidecar, returns refreshed ReusableAction. No-op when mtime is already current. - cdp_run_action handler invokes it iff forceReload !== false, between loadAction() and the maestroRun call. - new Zod field + extended tool description so MCP clients see the flag and understand the tradeoff. Tests (1471/1471 passing, +4 net new): - acknowledgeExternalEdit: refreshes sidecar lastSeenMtimeMs to YAML's current stat-mtime - acknowledgeExternalEdit: no-op when sidecar already current (returns same object reference, no disk write) - cdp_run_action forceReload=true (default): the sidecar advance happens BEFORE the run starts (snapshot captured inside fake maestroRun) - cdp_run_action forceReload=false: the sidecar is still at baseline at the moment maestro runs (snapshot proves no pre-run acknowledgment), and the strict mode run fails with isError=true on stale YAML The post-run mtime check was strengthened to a mid-run snapshot per codex-pair's MED review — the prior assertion could pass even if acknowledgment never happened (the RunRecord save would advance mtime anyway). The new snapshot capture proves the pre-run acknowledgment specifically. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 3af569f commit a18669f

7 files changed

Lines changed: 260 additions & 11 deletions

File tree

scripts/cdp-bridge/dist/domain/action-store.js

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@
55
// composite. Underpins /run-action, self-repair, and auto-emission —
66
// they all read/write through this single chokepoint so schema
77
// invariants stay enforced.
8-
import { existsSync, readFileSync } from 'node:fs';
8+
import { existsSync, readFileSync, statSync } from 'node:fs';
99
import { join } from 'node:path';
1010
import { parseM7Header, serializeM7Header, } from './reusable-action.js';
11-
import { loadOrInitSidecar, sidecarPathFor, yamlEditedSinceLastSeen, } from './sidecar-io.js';
11+
import { loadOrInitSidecar, markSeen, saveSidecar, sidecarPathFor, yamlEditedSinceLastSeen, } from './sidecar-io.js';
1212
import { atomicWriter } from './atomic-writer.js';
1313
import { assertValidActionId, assertWithinDir } from './path-safety.js';
1414
/**
@@ -229,6 +229,37 @@ export function saveAction(action) {
229229
export function actionWasEditedExternally(action) {
230230
return yamlEditedSinceLastSeen(action.filePath, action.state);
231231
}
232+
/**
233+
* GH #173 (sub-issue 3): treat the YAML's current on-disk mtime as the
234+
* new baseline. Stats the YAML, persists `markSeen(state, currentMtime)`
235+
* to the sidecar, and returns a new ReusableAction with the refreshed
236+
* lastSeenMtimeMs. Subsequent `actionWasEditedExternally()` checks
237+
* return false until something edits the YAML again.
238+
*
239+
* Use case: `cdp_run_action` is called while the human is actively
240+
* composing the YAML. The human's edit IS the intent; the Phase 129
241+
* guardrail (which exists to protect offline human edits from
242+
* auto-repair clobber) is over-protective in this loop. The orchestrator
243+
* acknowledges the edit before running so any downstream repair
244+
* proceeds without `STALE_TARGET`.
245+
*
246+
* No-op when the YAML mtime equals the sidecar's lastSeenMtimeMs (the
247+
* common case where no external write happened).
248+
*/
249+
export function acknowledgeExternalEdit(action) {
250+
let currentMtimeMs;
251+
try {
252+
currentMtimeMs = statSync(action.filePath).mtimeMs;
253+
}
254+
catch {
255+
return action;
256+
}
257+
if (currentMtimeMs <= action.state.lastSeenMtimeMs)
258+
return action;
259+
const nextState = markSeen(action.state, currentMtimeMs);
260+
saveSidecar(action.filePath, nextState);
261+
return { ...action, state: nextState };
262+
}
232263
/**
233264
* Issue #117: CAS variant of `saveAction`. Compares the on-disk
234265
* sidecar's `lastSeenMtimeMs` to the in-memory `action.state.

scripts/cdp-bridge/dist/index.js

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

scripts/cdp-bridge/dist/tools/run-action.js

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
// 30s+ device snapshot; cascading retries would be slow and could
2929
// mask underlying screen churn).
3030
import { okResult, failResult } from '../utils.js';
31-
import { loadAction, saveActionWithCAS } from '../domain/action-store.js';
31+
import { acknowledgeExternalEdit, loadAction, saveActionWithCAS } from '../domain/action-store.js';
3232
import { appendRunRecord, } from '../domain/reusable-action.js';
3333
import { parseMaestroFailure, isAutoRepairable, } from '../domain/maestro-error-parser.js';
3434
import { createMaestroRunHandler } from './maestro-run.js';
@@ -121,10 +121,16 @@ export function createRunActionHandler(deps = {}) {
121121
return failResult(`Invalid actionId "${String(args.actionId).slice(0, 80)}" — must match /^[A-Za-z0-9][A-Za-z0-9_-]*$/ and be <= 64 chars`, 'BAD_FILENAME');
122122
}
123123
const projectRoot = args.projectRoot ?? process.cwd();
124-
const action = loadAction(projectRoot, args.actionId);
125-
if (!action) {
124+
const loaded = loadAction(projectRoot, args.actionId);
125+
if (!loaded) {
126126
return failResult(`cdp_run_action: action "${args.actionId}" not found at ${projectRoot}/.rn-agent/actions/${args.actionId}.yaml`, 'NO_PROJECT_ROOT', { hint: 'Verify with /list-learned-actions, or pass projectRoot if cdp-bridge is invoked outside the project dir.' });
127127
}
128+
// GH #173 (sub-issue 3): default-true forceReload acknowledges any
129+
// human edit to the YAML as the new baseline so downstream auto-repair
130+
// doesn't abort with STALE_TARGET. Opt out with forceReload: false to
131+
// get the strict Phase 129 "respect external edits" behavior back.
132+
const forceReload = args.forceReload !== false;
133+
const action = forceReload ? acknowledgeExternalEdit(loaded) : loaded;
128134
const autoRepairEnabled = args.autoRepair !== false;
129135
const trigger = args.trigger ?? 'agent';
130136
const timeoutMs = args.timeoutMs ?? 120_000;

scripts/cdp-bridge/src/domain/action-store.ts

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
// they all read/write through this single chokepoint so schema
77
// invariants stay enforced.
88

9-
import { existsSync, readFileSync } from 'node:fs';
9+
import { existsSync, readFileSync, statSync } from 'node:fs';
1010
import { join } from 'node:path';
1111
import {
1212
type ReusableAction,
@@ -16,6 +16,8 @@ import {
1616
} from './reusable-action.js';
1717
import {
1818
loadOrInitSidecar,
19+
markSeen,
20+
saveSidecar,
1921
sidecarPathFor,
2022
yamlEditedSinceLastSeen,
2123
} from './sidecar-io.js';
@@ -265,6 +267,36 @@ export function actionWasEditedExternally(action: ReusableAction): boolean {
265267
return yamlEditedSinceLastSeen(action.filePath, action.state);
266268
}
267269

270+
/**
271+
* GH #173 (sub-issue 3): treat the YAML's current on-disk mtime as the
272+
* new baseline. Stats the YAML, persists `markSeen(state, currentMtime)`
273+
* to the sidecar, and returns a new ReusableAction with the refreshed
274+
* lastSeenMtimeMs. Subsequent `actionWasEditedExternally()` checks
275+
* return false until something edits the YAML again.
276+
*
277+
* Use case: `cdp_run_action` is called while the human is actively
278+
* composing the YAML. The human's edit IS the intent; the Phase 129
279+
* guardrail (which exists to protect offline human edits from
280+
* auto-repair clobber) is over-protective in this loop. The orchestrator
281+
* acknowledges the edit before running so any downstream repair
282+
* proceeds without `STALE_TARGET`.
283+
*
284+
* No-op when the YAML mtime equals the sidecar's lastSeenMtimeMs (the
285+
* common case where no external write happened).
286+
*/
287+
export function acknowledgeExternalEdit(action: ReusableAction): ReusableAction {
288+
let currentMtimeMs: number;
289+
try {
290+
currentMtimeMs = statSync(action.filePath).mtimeMs;
291+
} catch {
292+
return action;
293+
}
294+
if (currentMtimeMs <= action.state.lastSeenMtimeMs) return action;
295+
const nextState = markSeen(action.state, currentMtimeMs);
296+
saveSidecar(action.filePath, nextState);
297+
return { ...action, state: nextState };
298+
}
299+
268300
/**
269301
* Issue #117: CAS variant of `saveAction`. Compares the on-disk
270302
* sidecar's `lastSeenMtimeMs` to the in-memory `action.state.

scripts/cdp-bridge/src/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1098,14 +1098,15 @@ trackedTool(
10981098
// stderr classification + cdp_repair_action retry on SELECTOR_NOT_FOUND.
10991099
trackedTool(
11001100
'cdp_run_action',
1101-
'Replay a learned action by id with end-to-end auto-repair. Loads the action from .rn-agent/actions/<actionId>.yaml, runs the Maestro flow, and on a SELECTOR_NOT_FOUND failure automatically invokes cdp_repair_action and retries once. Appends a RunRecord to the sidecar with full auto-repair telemetry (passed/failed/refused/skipped + diff). The repair attempt counts toward cdp_repair_action\'s 24h budget. Pass autoRepair=false to opt out of auto-repair (returns the raw maestro_run failure verbatim). The orchestrated home for the L3 self-healing loop — prefer this over invoking maestro_run + cdp_repair_action manually for any flow you intend to re-run on schedule.',
1101+
'Replay a learned action by id with end-to-end auto-repair. Loads the action from .rn-agent/actions/<actionId>.yaml, runs the Maestro flow, and on a SELECTOR_NOT_FOUND failure automatically invokes cdp_repair_action and retries once. Appends a RunRecord to the sidecar with full auto-repair telemetry (passed/failed/refused/skipped + diff). The repair attempt counts toward cdp_repair_action\'s 24h budget. Pass autoRepair=false to opt out of auto-repair (returns the raw maestro_run failure verbatim). forceReload defaults true: any human edit to the YAML since the agent\'s last write is acknowledged as the new baseline so downstream repair does not abort with STALE_TARGET (the right default for active composition). Pass forceReload=false for the strict "respect offline human edits" behavior. The orchestrated home for the L3 self-healing loop — prefer this over invoking maestro_run + cdp_repair_action manually for any flow you intend to re-run on schedule.',
11021102
{
11031103
actionId: z.string().describe('Action id matching <projectRoot>/.rn-agent/actions/<actionId>.yaml.'),
11041104
projectRoot: z.string().optional().describe('Override project root (default: process.cwd()).'),
11051105
platform: z.enum(['ios', 'android']).optional().describe('Force a specific platform; otherwise auto-detected from the active device session.'),
11061106
autoRepair: z.boolean().optional().describe('Auto-repair on SELECTOR_NOT_FOUND failures. Default true. Pass false to disable (e.g. when investigating a failure manually).'),
11071107
timeoutMs: z.number().optional().describe('Maestro execution timeout per attempt (ms). Default 120_000.'),
11081108
trigger: z.enum(['agent', 'ci', 'human']).optional().describe('RunRecord trigger annotation. Default "agent". CI calls should pass "ci".'),
1109+
forceReload: z.boolean().optional().describe('GH #173: when true (default), acknowledge any human edit to the YAML as the new baseline before running so downstream repair does not abort with STALE_TARGET. Pass false for the strict Phase 129 "respect external edits" behavior (useful for CI replays of fixed baselines).'),
11091110
},
11101111
createRunActionHandler(),
11111112
);

scripts/cdp-bridge/src/tools/run-action.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
import { okResult, failResult } from '../utils.js';
3232
import type { ToolResult } from '../utils.js';
3333
import type { ToolErrorCode } from '../types.js';
34-
import { loadAction, saveActionWithCAS } from '../domain/action-store.js';
34+
import { acknowledgeExternalEdit, loadAction, saveActionWithCAS } from '../domain/action-store.js';
3535
import {
3636
type RunRecord,
3737
type AutoRepairOutcome,
@@ -103,6 +103,18 @@ export interface RunActionArgs {
103103
* so a parameterised flow can be replayed identically after repair.
104104
*/
105105
params?: Record<string, string>;
106+
/**
107+
* GH #173 (sub-issue 3): when true (default), treat the YAML's current
108+
* on-disk state as the new baseline before running. Bumps the sidecar's
109+
* lastSeenMtimeMs so a downstream cdp_repair_action call doesn't abort
110+
* with STALE_TARGET during active human composition.
111+
*
112+
* Pass `false` to opt back into the Phase 129 "respect external edits"
113+
* behavior: any human edit since the agent's last write makes repair
114+
* refuse to run. Use this when you don't want auto-repair to clobber
115+
* offline human edits (e.g. CI replays of fixed baselines).
116+
*/
117+
forceReload?: boolean;
106118
}
107119

108120
interface MaestroEnvelope {
@@ -191,14 +203,20 @@ export function createRunActionHandler(deps: RunActionDeps = {}) {
191203
}
192204

193205
const projectRoot = args.projectRoot ?? process.cwd();
194-
const action = loadAction(projectRoot, args.actionId);
195-
if (!action) {
206+
const loaded = loadAction(projectRoot, args.actionId);
207+
if (!loaded) {
196208
return failResult(
197209
`cdp_run_action: action "${args.actionId}" not found at ${projectRoot}/.rn-agent/actions/${args.actionId}.yaml`,
198210
'NO_PROJECT_ROOT',
199211
{ hint: 'Verify with /list-learned-actions, or pass projectRoot if cdp-bridge is invoked outside the project dir.' },
200212
);
201213
}
214+
// GH #173 (sub-issue 3): default-true forceReload acknowledges any
215+
// human edit to the YAML as the new baseline so downstream auto-repair
216+
// doesn't abort with STALE_TARGET. Opt out with forceReload: false to
217+
// get the strict Phase 129 "respect external edits" behavior back.
218+
const forceReload = args.forceReload !== false;
219+
const action = forceReload ? acknowledgeExternalEdit(loaded) : loaded;
202220

203221
const autoRepairEnabled = args.autoRepair !== false;
204222
const trigger: 'agent' | 'ci' | 'human' = args.trigger ?? 'agent';

0 commit comments

Comments
 (0)