Skip to content

Commit 056532b

Browse files
Lykhoydaclaude
andauthored
fix(audit): resolve HIGH + MEDIUM + perf/correctness findings from multi-agent repo audit (#185)
* fix(audit): resolve HIGH + MEDIUM + perf/correctness findings from multi-agent repo audit A 13-subsystem multi-agent audit (adversarially verified, 18 findings refuted) surfaced 86 confirmed issues. This resolves the 1 HIGH, all 16 MEDIUM, and the high-value perf/low/doc items — ~42 findings — each with regression tests. Full suite: 1558 pass. Swift runner build-for-testing SUCCEEDED. The multi-renderer getTree fix was live-verified on the booted Bridgeless+Reanimated test app over CDP. HIGH - collect-feedback.sh redact() failed OPEN on macOS (invalid sed bracket range) leaking ghp_/AKIA/JWT/Bearer/sk_ secrets into public feedback issues — now one sed program that fails CLOSED ([REDACTION_FAILED]); + bash regression test. MEDIUM (correctness/inconsistency) - device_batch swipe was a scroll; now a real swipe gesture honoring duration - runIOS/postCommand + android client: AbortController per-command HTTP timeout - ref-map @ref coords gated on snapshot age (isRefMapFresh) -> STALE_REF when stale - iOS raw screenshot honors png format via extension; NaN tap coords rejected - repair-engine extract/replaceIdSelector match the maestro-error-parser quoted-id grammar (quoted/commented testIDs no longer skip auto-repair) - experience: candidate-id regex matches real filenames; imported heuristic ids deduped; ghost recovery gated to idempotent tools; redaction catches quoted-JSON /object-auth secrets; PII regex no longer over-redacts bare numbers - cross_platform_verify FAIL returns ok:false (was warnResult/ok:true) - runMaestroInline routes through chooseMaestroDispatch (B59 iOS-only fallback) - injected getTree() unfiltered branch walks ALL renderers (B143/B145) - nav-graph strikes namespaced per project (no cross-project cooldown poisoning) - swift textInputAt restricts query to input types; eas_resolve_artifact.sh portable stat PERF - meta.timings_ms helper + scrollintoview instrumentation (CLAUDE.md mandate) - object_inspect concurrent child fetch; nav_graph go single graph read (was 4x) - connect candidate loop O(n^2) indexOf -> indexed; auto-login polls instead of 3s wait LOW / correctness + docs - guarded JSON.parse (navigation-state, android client); navigateTo loop depth cap - sidecar missing lastSeenMtimeMs re-seeded (re-enables human-edit guard) - proof_step no-session verify -> verified:false; device_record path-traversal guard - device_find surfaces recoveredTier on NOT_FOUND/AMBIGUOUS; detectPrerequisites whole-word permission match; learned-actions --max NaN fallback; collect-feedback grep double-zero fix; iOS /health probe 127.0.0.1 (was [::1]) - tool-count drift (banner 51, CLAUDE.md 74 -> 75); REACT_READY renderer range 1-5 -> 1-20 - repair budget refusal reports 24h-windowed count, not lifetime total Swift (xcodebuild build-for-testing verified) - rn-fast-runner answers GET /health directly (was unparseable -> liveness always stale) - snapshotHasFocus uses `as?` not `as!` (force-cast trap is uncatchable) Deferred (~20 moderate-risk perf + cosmetic items) tracked for a focused follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(audit): clear the deferred perf + cosmetic findings (B12) Follow-up to the audit fixes — addresses the remaining lower-severity items. Full suite still 1558 pass; tsc clean. Perf/lifecycle: logger buffered WriteStream (no event-loop block); connectWs backstop terminates a wedged socket (no leak); cdp_network_log truncated/total accurate on the unfiltered path via buffer size. Correctness/clarity: maestro_run pass/fail no longer false-flags benign 'Error:' (exit code authoritative); fingerprint engine = Hermes only with an RN signal else null; groupFailures unreachable passed++ removed; multiplexer debug-logs unrouted-id drops; RunIOSArgs dead x1/y1 removed; path-safety sidecar-path comment fixed; ensure-agent-device uses npm prefix -g (npm 9+); expo_ensure_running preserves temp logs on failure. Not-actioned (correct as-is): lockfile release (signals route to exit->release; SIGKILL covered by mtime), dumpsys (action=all runs once), persistRun CAS re-read (required after conflict), test-recorder dedup edge / buildNavigationPlan scan / loadExperience forceReload (dev-only/negligible). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 68393fc commit 056532b

85 files changed

Lines changed: 1500 additions & 329 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ Fallback: `xcrun simctl` (iOS) + `adb` (Android) for device lifecycle (boot / in
123123

124124
### MCP Server (cdp-bridge)
125125

126-
**74 tools** exposed via MCP (re-audited 2026-05-18; counted from `trackedTool()` calls in `scripts/cdp-bridge/src/index.ts`). Five conceptual families:
126+
**75 tools** exposed via MCP (re-audited 2026-05-29; counted from `trackedTool()` calls in `scripts/cdp-bridge/src/index.ts`). Five conceptual families:
127127

128128
**CDP tools** — React internals via Chrome DevTools Protocol over WebSocket:
129129
- `cdp_status` — health check with domain capabilities + reconnect state

hooks/detect-rn-project.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ if [ "$has_rn_config" = true ]; then
9191
fi
9292

9393
cat <<'EOF'
94-
React Native project detected. The rn-dev-agent plugin is active with 51 MCP tools.
94+
React Native project detected. The rn-dev-agent plugin is active with 75 MCP tools.
9595
9696
## How to interact with the running app
9797

scripts/cdp-bridge/dist/agent-device-wrapper.js

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { homedir } from 'node:os';
77
import { createHash } from 'node:crypto';
88
import { okResult, failResult } from './utils.js';
99
import { isFastRunnerAvailable, startFastRunner, } from './runners/rn-fast-runner-client.js';
10-
import { refCenter, getScreenRect, clearRefMap } from './fast-runner-ref-map.js';
10+
import { refCenter, getScreenRect, clearRefMap, isRefMapFresh } from './fast-runner-ref-map.js';
1111
import { resolveBundleId } from './project-config.js';
1212
const execFile = promisify(execFileCb);
1313
/**
@@ -284,22 +284,26 @@ const RN_FAST_RUNNER_COMMANDS = new Set([
284284
export function getCachedScreenRect() {
285285
return getScreenRect();
286286
}
287-
function buildRunIOSArgs(cliArgs, bundleId) {
287+
export function buildRunIOSArgs(cliArgs, bundleId) {
288288
const cmd = cliArgs[0];
289289
const positionals = cliArgs.slice(1).filter((a) => !a.startsWith('--'));
290290
switch (cmd) {
291291
case 'press':
292292
case 'tap': {
293293
const ref = positionals[0];
294294
if (ref && ref.startsWith('@')) {
295-
const center = refCenter(ref);
295+
const center = isRefMapFresh() ? refCenter(ref) : null;
296296
if (!center) {
297297
return { command: 'tap', _staleRef: ref, ...(bundleId ? { bundleId } : {}) };
298298
}
299299
return { command: 'tap', x: center.x, y: center.y, ...(bundleId ? { bundleId } : {}) };
300300
}
301301
const [xS, yS] = positionals;
302-
return { command: 'tap', x: Number(xS), y: Number(yS), ...(bundleId ? { bundleId } : {}) };
302+
const x = Number(xS), y = Number(yS);
303+
if (Number.isNaN(x) || Number.isNaN(y)) {
304+
throw new Error(`buildRunIOSArgs: tap requires a @ref or numeric x, y`);
305+
}
306+
return { command: 'tap', x, y, ...(bundleId ? { bundleId } : {}) };
303307
}
304308
case 'fill':
305309
case 'type': {
@@ -310,7 +314,7 @@ function buildRunIOSArgs(cliArgs, bundleId) {
310314
const ref = positionals[0];
311315
const text = positionals.slice(1).join(' ');
312316
if (ref && ref.startsWith('@')) {
313-
const center = refCenter(ref);
317+
const center = isRefMapFresh() ? refCenter(ref) : null;
314318
if (!center) {
315319
return { command: 'type', _staleRef: ref, text, ...(bundleId ? { bundleId } : {}) };
316320
}
@@ -414,7 +418,7 @@ function androidPositionals(cliArgs) {
414418
}
415419
return out;
416420
}
417-
function buildRunAndroidArgs(cliArgs, bundleId) {
421+
export function buildRunAndroidArgs(cliArgs, bundleId) {
418422
const cmd = cliArgs[0];
419423
const positionals = androidPositionals(cliArgs);
420424
const withBundle = bundleId ? { bundleId } : {};
@@ -423,20 +427,24 @@ function buildRunAndroidArgs(cliArgs, bundleId) {
423427
case 'tap': {
424428
const ref = positionals[0];
425429
if (ref && ref.startsWith('@')) {
426-
const center = refCenter(ref);
430+
const center = isRefMapFresh() ? refCenter(ref) : null;
427431
if (!center)
428432
return { command: 'tap', _staleRef: ref, ...withBundle };
429433
return { command: 'tap', x: center.x, y: center.y, ...withBundle };
430434
}
431435
const [xS, yS] = positionals;
432-
return { command: 'tap', x: Number(xS), y: Number(yS), ...withBundle };
436+
const x = Number(xS), y = Number(yS);
437+
if (Number.isNaN(x) || Number.isNaN(y)) {
438+
throw new Error(`buildRunAndroidArgs: tap requires a @ref or numeric x, y`);
439+
}
440+
return { command: 'tap', x, y, ...withBundle };
433441
}
434442
case 'fill':
435443
case 'type': {
436444
const ref = positionals[0];
437445
const text = positionals.slice(1).join(' ');
438446
if (ref && ref.startsWith('@')) {
439-
const center = refCenter(ref);
447+
const center = isRefMapFresh() ? refCenter(ref) : null;
440448
if (!center)
441449
return { command: 'type', _staleRef: ref, text, ...withBundle };
442450
return { command: 'type', x: center.x, y: center.y, text, ...withBundle };

scripts/cdp-bridge/dist/cdp/connect.js

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,9 @@ discoverFn = discover) {
6767
throw new Error(selectionWarning ?? 'No matching CDP targets found.');
6868
}
6969
let connectedTarget = null;
70-
for (const candidate of sorted) {
70+
for (let idx = 0; idx < sorted.length; idx++) {
71+
const candidate = sorted[idx];
72+
const isLast = idx === sorted.length - 1;
7173
try {
7274
await connectToTarget(ctx, candidate);
7375
const devCheck = await ctx.evaluate('typeof __DEV__ !== "undefined" && __DEV__ === true');
@@ -76,7 +78,7 @@ discoverFn = discover) {
7678
break;
7779
}
7880
console.error(`CDP: target ${candidate.id} (${candidate.title}) has __DEV__=${devCheck.value}, skipping`);
79-
if (sorted.indexOf(candidate) < sorted.length - 1) {
81+
if (!isLast) {
8082
closeAndResetWs(ctx);
8183
ctx.setState('disconnected');
8284
ctx.setHelpersInjected(false);
@@ -87,7 +89,7 @@ discoverFn = discover) {
8789
connectedTarget = candidate;
8890
}
8991
catch (err) {
90-
if (sorted.indexOf(candidate) < sorted.length - 1)
92+
if (!isLast)
9193
continue;
9294
throw err;
9395
}
@@ -210,15 +212,34 @@ function connectWs(ctx, url) {
210212
maxPayload: 100 * 1024 * 1024,
211213
});
212214
let settled = false;
215+
// Backstop: handshakeTimeout should emit 'error', but if the socket ever
216+
// wedges without firing open/error/close it would leak with its listeners.
217+
// Terminate it after a grace window so it can't linger.
218+
const guard = setTimeout(() => {
219+
if (settled)
220+
return;
221+
settled = true;
222+
try {
223+
ws.terminate();
224+
}
225+
catch { /* already gone */ }
226+
reject(new Error('WebSocket connect timed out'));
227+
}, 7000);
213228
ws.on('open', () => {
214229
settled = true;
230+
clearTimeout(guard);
215231
ctx.setWs(ws);
216232
ctx.setState('connected');
217233
resolve();
218234
});
219235
ws.on('error', (err) => {
220236
if (!settled) {
221237
settled = true;
238+
clearTimeout(guard);
239+
try {
240+
ws.terminate();
241+
}
242+
catch { /* already closing */ }
222243
reject(err);
223244
}
224245
else {
@@ -231,6 +252,7 @@ function connectWs(ctx, url) {
231252
ws.on('close', (code) => {
232253
if (!settled) {
233254
settled = true;
255+
clearTimeout(guard);
234256
reject(new Error(`WebSocket closed before connecting: ${code}`));
235257
return;
236258
}

scripts/cdp-bridge/dist/cdp/multiplexer.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,8 +236,12 @@ export class CDPMultiplexer {
236236
return;
237237
}
238238
const route = this.routingTable.get(upstreamId);
239-
if (!route)
239+
if (!route) {
240+
// No routing entry for this upstream id — a late reply after the entry was
241+
// swept, or an id-space collision. Log it (debug) so it isn't silent.
242+
logger.debug(this.opts.logTag, `dropping Hermes reply with unrouted id=${upstreamId}`);
240243
return;
244+
}
241245
this.routingTable.delete(upstreamId);
242246
m.id = route.consumerOriginalId;
243247
const rewritten = JSON.stringify(m);

scripts/cdp-bridge/dist/domain/path-safety.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export class PathTraversalError extends Error {
2727
}
2828
// ── Action ID validation ────────────────────────────────────────────
2929
// Action IDs flow into `.rn-agent/actions/<id>.yaml` and the matching
30-
// sidecar `<id>.action.json`. They must start with an alphanumeric
30+
// sidecar `.rn-agent/state/<id>.state.json`. They must start with an alphanumeric
3131
// character, then accept hyphen/underscore/dot/alphanumeric. We
3232
// deliberately exclude `..`, `/`, `\`, and control characters so the
3333
// ID can never escape its parent directory.

scripts/cdp-bridge/dist/domain/repair-engine.js

Lines changed: 42 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -136,24 +136,35 @@ export function replaceIdSelector(body, oldId, newId) {
136136
const lines = body.split('\n');
137137
const out = [];
138138
let replacements = 0;
139-
// Match the line-trimmed `id: <quoted-or-bare><oldId><quoted-or-bare>`.
140-
// Capture leading whitespace + quote style so we can preserve them.
141-
// Allowed quotes: `"X"`, `'X'`, or bare `X` if it has no spaces.
139+
// Match the line-trimmed `id: <quoted-or-bare><oldId>`, preserving leading
140+
// whitespace, quote style, and any trailing `# comment`. Three explicit
141+
// shapes (double / single / bare) keep this in lockstep with
142+
// extractIdSelectors and the maestro-error-parser matched-quote grammar:
143+
// a double-quoted value may contain `'` and vice-versa.
142144
const escapedOld = oldId.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
143-
const re = new RegExp(`^(\\s*)id:\\s*("?)${escapedOld}("?)\\s*$`);
144-
const reSingle = new RegExp(`^(\\s*)id:\\s*(')${escapedOld}(')\\s*$`);
145+
const dq = new RegExp(`^(\\s*)id:\\s*"${escapedOld}"(\\s*(?:#.*)?)$`);
146+
const sq = new RegExp(`^(\\s*)id:\\s*'${escapedOld}'(\\s*(?:#.*)?)$`);
147+
const bare = new RegExp(`^(\\s*)id:\\s*${escapedOld}(\\s*(?:#.*)?)$`);
145148
for (const line of lines) {
146-
const m = line.match(re) ?? line.match(reSingle);
149+
let m = line.match(dq);
147150
if (m) {
148-
const indent = m[1];
149-
const quoteOpen = m[2] ?? '';
150-
const quoteClose = m[3] ?? '';
151-
out.push(`${indent}id: ${quoteOpen}${newId}${quoteClose}`);
151+
out.push(`${m[1]}id: "${newId}"${m[2]}`);
152152
replacements++;
153+
continue;
153154
}
154-
else {
155-
out.push(line);
155+
m = line.match(sq);
156+
if (m) {
157+
out.push(`${m[1]}id: '${newId}'${m[2]}`);
158+
replacements++;
159+
continue;
160+
}
161+
m = line.match(bare);
162+
if (m) {
163+
out.push(`${m[1]}id: ${newId}${m[2]}`);
164+
replacements++;
165+
continue;
156166
}
167+
out.push(line);
157168
}
158169
return { body: out.join('\n'), replacements };
159170
}
@@ -168,22 +179,27 @@ export function replaceIdSelector(body, oldId, newId) {
168179
export function extractIdSelectors(body) {
169180
const out = [];
170181
const lines = body.split('\n');
171-
// Issue #102 A2: previous regex `[^"'\\n]*?` greedily included
172-
// trailing inline comments — `id: foo-bar # human note` captured
173-
// `foo-bar # human note` as the testID. Hand-authored YAML
174-
// sometimes has these comments; the bug fails safely (no fuzzy
175-
// match) but produces a UX issue. Fix: strip trailing
176-
// `\s+#.*` from the captured group before pushing. Also tighten
177-
// the bare-form regex to reject `#` directly so quoted forms are
178-
// unaffected.
179-
const re = /^\s*id:\s*"?'?([^"'\s][^"'\n]*?)"?'?\s*$/;
182+
// Mirror the maestro-error-parser matched-quote grammar (PR #115) so the
183+
// failure parser and this extractor agree on what a testID is. Previously
184+
// the char class `[^"'\s]` rejected any testID containing a quote (e.g.
185+
// `user's-task`), so attemptRepair's gate short-circuited to
186+
// 'no-stale-selector' and auto-repair silently no-op'd for ids the parser
187+
// had correctly extracted. Three explicit shapes:
188+
// id: "value" — value may contain '
189+
// id: 'value' — value may contain "
190+
// id: value — bare; strip a trailing ` # comment` (Issue #102 A2)
191+
const dq = /^\s*id:\s*"([^"\n]*)"\s*(?:#.*)?$/;
192+
const sq = /^\s*id:\s*'([^'\n]*)'\s*(?:#.*)?$/;
193+
const bare = /^\s*id:\s*([^"'#\s][^#\n]*?)\s*(?:#.*)?$/;
180194
for (const line of lines) {
181-
const m = line.match(re);
195+
const m = line.match(dq) ?? line.match(sq);
182196
if (m) {
183-
// Trim any trailing `<space>#<rest-of-line>` that the lazy
184-
// capture could not exclude.
185-
const cleaned = m[1].replace(/\s+#.*$/, '');
186-
out.push(cleaned);
197+
out.push(m[1]);
198+
continue;
199+
}
200+
const b = line.match(bare);
201+
if (b) {
202+
out.push(b[1].trimEnd());
187203
}
188204
}
189205
return out;

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,12 @@ export function appendRepairRecord(state, record) {
102102
* Check whether a self-repair attempt is within the rolling-24h budget.
103103
* Pure function — `now` is injectable for tests.
104104
*/
105-
export function repairBudgetAvailable(state, now = () => new Date()) {
105+
export function recentRepairCount(state, now = () => new Date()) {
106106
const cutoff = now().getTime() - 24 * 60 * 60 * 1000;
107-
const recent = state.repairHistory.filter((r) => new Date(r.timestamp).getTime() >= cutoff);
108-
return recent.length < REPAIR_BUDGET.ATTEMPTS_PER_24H;
107+
return state.repairHistory.filter((r) => new Date(r.timestamp).getTime() >= cutoff).length;
108+
}
109+
export function repairBudgetAvailable(state, now = () => new Date()) {
110+
return recentRepairCount(state, now) < REPAIR_BUDGET.ATTEMPTS_PER_24H;
109111
}
110112
/**
111113
* Promote `experimental → active` after a clean replay.

scripts/cdp-bridge/dist/domain/sidecar-io.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,18 @@ export function loadOrInitSidecar(yamlFilePath, now = () => new Date()) {
4646
Array.isArray(parsed.runHistory) &&
4747
Array.isArray(parsed.repairHistory) &&
4848
typeof parsed.stats === 'object') {
49+
// A sidecar missing lastSeenMtimeMs (e.g. written before the field
50+
// existed) would silently disable the human-edit guard, since
51+
// yamlEditedSinceLastSeen compares against undefined. Re-seed just that
52+
// field from the YAML's mtime — preserves run/repair history.
53+
if (typeof parsed.lastSeenMtimeMs !== 'number') {
54+
try {
55+
parsed.lastSeenMtimeMs = statSync(yamlFilePath).mtimeMs;
56+
}
57+
catch {
58+
parsed.lastSeenMtimeMs = 0;
59+
}
60+
}
4961
return parsed;
5062
}
5163
// Fall through — corrupted; return fresh.

scripts/cdp-bridge/dist/experience/compact.js

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,11 @@ export function groupFailures(events) {
8080
if (isGhostRecovery) {
8181
stats.ghost_recovered++;
8282
}
83-
else if (event.result === 'FAIL' || event.result === 'ERROR') {
84-
stats.failed++;
85-
}
8683
else {
87-
stats.passed++;
84+
// The filter above only admits fails/ghost-recoveries, so a non-ghost
85+
// event here is always FAIL/ERROR — the prior `else { passed++ }` branch
86+
// was unreachable (stats.passed stayed 0 forever).
87+
stats.failed++;
8888
}
8989
if (event.family_id && !stats.family_id) {
9090
stats.family_id = event.family_id;
@@ -96,19 +96,27 @@ export function groupFailures(events) {
9696
* Generate candidate heuristics from failure stats.
9797
* Only generates for patterns with >= MIN_OCCURRENCES and >= MIN_SUCCESS_RATE.
9898
*/
99+
// Files are written as `candidate-<id>.md` where id is RS-C<n>/FP-C<n>
100+
// (writeCandidateFile lowercases it → `candidate-rs-c1.md`). The prior
101+
// `/candidate-(\d+)/` never matched (a letter always follows the dash), so the
102+
// counter stayed 1 and each compaction cycle clobbered earlier candidate files.
103+
export function nextCandidateId(existingFilenames) {
104+
let nextId = 1;
105+
for (const f of existingFilenames) {
106+
const match = f.match(/candidate-(?:rs|fp)-c(\d+)/i);
107+
if (match)
108+
nextId = Math.max(nextId, parseInt(match[1], 10) + 1);
109+
}
110+
return nextId;
111+
}
99112
export function generateCandidates(stats, events) {
100113
const candidates = [];
101114
let nextId = 1;
102115
// Find the highest existing candidate ID
103116
const candidatesDir = join(AGENT_DIR, 'candidates');
104117
if (existsSync(candidatesDir)) {
105118
try {
106-
const existing = readdirSync(candidatesDir).filter(f => f.startsWith('candidate-'));
107-
for (const f of existing) {
108-
const match = f.match(/candidate-(\d+)/);
109-
if (match)
110-
nextId = Math.max(nextId, parseInt(match[1], 10) + 1);
111-
}
119+
nextId = nextCandidateId(readdirSync(candidatesDir).filter(f => f.startsWith('candidate-')));
112120
}
113121
catch { /* best-effort */ }
114122
}

0 commit comments

Comments
 (0)