mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-16 08:06:24 +03:00
Address review: re-check a failed resolution before declining an agent target
A page's element can be momentarily absent (a route still rendering, an HMR commit mid-swap), so a failed resolution is not that page's final word. The overlay now re-checks at 300, 700, and 1500 ms, claims the moment the element mounts (the server already drops the stale report on an eligible claim), and reports only the last miss. A genuine no_match now takes about two seconds instead of tens of milliseconds, well inside the server's hold. Also normalizes a path separator in the new hook unit test, which failed on rust-windows because the audit's file path carries backslashes there. AI-assisted: implemented and tested with Claude Code under maintainer direction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
committed by
Abdul Wahab
co-authored by
Claude Fable 5
parent
25263adc51
commit
46f1dd6b36
@@ -2279,5 +2279,6 @@ fn run_hook_stands_down_for_the_whole_edit_when_the_primary_carries_live_markers
|
||||
let skipped = hook::run_hook(&r, &edit_event(&cwd, &wrapped, "s2"));
|
||||
assert_eq!(skipped.stdout, "", "nothing is emitted while the edited file is in a live session");
|
||||
assert_eq!(skipped.audit["skipped"], json!("live-preview"));
|
||||
assert!(audit_str(&skipped.audit, "file").unwrap_or("").ends_with("src/App.jsx"), "the audit names the edited file, not the companion");
|
||||
let audited = audit_str(&skipped.audit, "file").unwrap_or("").replace('\\', "/");
|
||||
assert!(audited.ends_with("src/App.jsx"), "the audit names the edited file, not the companion: {audited}");
|
||||
}
|
||||
|
||||
@@ -7291,13 +7291,36 @@
|
||||
// another page has. The server prefers a busy report (a tab that could
|
||||
// serve later) over these, and returns the resolution verdict only when
|
||||
// no connected page can serve.
|
||||
//
|
||||
// An element can be momentarily absent (a route still rendering, an HMR
|
||||
// commit mid-swap), so a failed resolution is not this page's final word:
|
||||
// it is re-checked a few times over about two seconds, claiming the
|
||||
// moment the element mounts, and only the last miss is reported. The
|
||||
// server's timeout still bounds the whole exchange.
|
||||
const AGENT_TARGET_RESOLVE_RETRY_MS = [300, 700, 1500];
|
||||
|
||||
function declineAgentTargetUnresolvable(msg) {
|
||||
const probe = resolveAgentTargetElement(msg);
|
||||
if (!probe.error) return false;
|
||||
claimAgentTarget(msg.targetId, { eligible: false, state, reason: 'no_match', result: probe.error });
|
||||
retryAgentTargetResolution(msg, 0, probe.error);
|
||||
return true;
|
||||
}
|
||||
|
||||
function retryAgentTargetResolution(msg, attempt, lastError) {
|
||||
if (attempt >= AGENT_TARGET_RESOLVE_RETRY_MS.length) {
|
||||
claimAgentTarget(msg.targetId, { eligible: false, state, reason: 'no_match', result: lastError });
|
||||
return;
|
||||
}
|
||||
setTimeout(() => {
|
||||
if (agentTargetOverlayGone()) return;
|
||||
const busy = agentTargetBusyReason();
|
||||
if (busy) { declineAgentTargetBusy(msg, busy); return; }
|
||||
const probe = resolveAgentTargetElement(msg);
|
||||
if (!probe.error) { claimAndActOnAgentTarget(msg); return; }
|
||||
retryAgentTargetResolution(msg, attempt + 1, probe.error);
|
||||
}, AGENT_TARGET_RESOLVE_RETRY_MS[attempt]);
|
||||
}
|
||||
|
||||
function handleAgentTarget(msg) {
|
||||
if (!msg || typeof msg.targetId !== 'string') return;
|
||||
if (agentTargetsSeen.includes(msg.targetId)) return;
|
||||
|
||||
@@ -866,8 +866,13 @@ describe('live-browser source contracts', () => {
|
||||
);
|
||||
assert.match(
|
||||
SOURCE,
|
||||
/function declineAgentTargetUnresolvable\(msg\) \{[\s\S]{0,200}?resolveAgentTargetElement\(msg\)[\s\S]{0,200}?reason: 'no_match', result: probe\.error/,
|
||||
'the decline carries the resolution verdict for the server to return when no page can serve',
|
||||
/function declineAgentTargetUnresolvable\(msg\) \{[\s\S]{0,200}?resolveAgentTargetElement\(msg\)[\s\S]{0,120}?retryAgentTargetResolution\(msg, 0, probe\.error\)/,
|
||||
'a failed resolution is re-checked before it becomes this page\'s word',
|
||||
);
|
||||
assert.match(
|
||||
SOURCE,
|
||||
/function retryAgentTargetResolution\(msg, attempt, lastError\) \{[\s\S]{0,200}?reason: 'no_match', result: lastError[\s\S]{0,600}?if \(!probe\.error\) \{ claimAndActOnAgentTarget\(msg\); return; \}/,
|
||||
'the page claims the moment the element mounts, and reports only the last miss',
|
||||
);
|
||||
// The per-origin session cache must not let a tab on another page of
|
||||
// the app resume this page's session (it would sit in GENERATING for a
|
||||
@@ -885,8 +890,8 @@ describe('live-browser source contracts', () => {
|
||||
);
|
||||
assert.equal(
|
||||
(SOURCE.match(/claimAndActOnAgentTarget\(msg\)/g) || []).length,
|
||||
4,
|
||||
'the first claim and the busy-to-idle re-claim must share the rescue path (definition, two call sites, the retry)',
|
||||
5,
|
||||
'the first claim, the busy-to-idle re-claim, and the resolution re-check must share the rescue path (definition, three call sites, the retry)',
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user