Skip to content

Commit 44229a2

Browse files
wenshaowenshao
committed
fix(managed-agent): record the stranded takeover adoption instead of letting it go silent
Review round R5 (Critical R5-1). The two post-adoption load refusals close the Session before it ever registers, so no harness route can ever hand the owed lease back — every discharger resolves Sessions through the registry. A persistent refusal (durable capture evidence, read-only or full workspace store) then re-adopts and re-refuses on every coordinator retry, straining silently until session retirement, and the same silent strand hit the continuation takeover too. The refusals now record the stranded (sessionId, runtimeSessionId) and report it once per identity on stderr, a successful later load of the Session drains the record, and the release wedge stays closed: nothing re-adds a release to either exit, so the transient case this PR's tests pin (release never runs, retried load settles) is untouched. From the same round: - The rewritten runtimeLeaseHeld contract is scoped to the cancellation path — the continuation route keeps its #13083 handback discipline, and the comment names that as the recorded follow-up instead of stating an invariant four continue-route refusals falsify (R4-4, fix-induced). - The handback-failure log names the prompt and session identity, like both sibling release-failure logs (R5-2). - The final-handback test now asserts the release count right after the replay — distinguishing the replay-path discharge from the close-path one — and rewords the comment to match reality (R5-3). - Design docs (EN+ZH) qualify the retirement sentence: a load refused before registration has no session to retire; the record-and-drain path covers it. R4-3's two remaining fixture asks are deferred per the round-5 gate (Critical-only), recorded in the thread. Mutation-checked: dropping the stranded line reddens both load tests, dropping the registration drain reddens the persistent-refusal variant, and dropping the replay-branch release reddens the handback test at count one.
1 parent b0e8b1e commit 44229a2

4 files changed

Lines changed: 153 additions & 19 deletions

File tree

‎docs/design/2026-09-30-hosted-turn-failover-e2e.md‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,11 @@ takeover is implemented for contract completeness but has no E2E mode.
7676
because a release persists RELEASED while a stranded READY identity is
7777
still usable — a redriven cancel is re-admitted against the current
7878
checkpoint (the daemon never re-loads an attached Session), and a
79-
takeover after an owner change re-acquires idempotently. Only session
80-
retirement discharges an abandonment.
79+
takeover after an owner change re-acquires idempotently. A load refused
80+
after adopting but before the Session registers records the owed adoption
81+
and reports it by name, since no registered session exists to retire it;
82+
the record drains on the next successful load of that Session, and only
83+
session retirement discharges an abandonment otherwise.
8184
Executions the Broker cannot account for report `unknown`, the coordinator
8285
blocks the Turn as `managed_runtime_recovery_blocked`, and nothing replays.
8386
- **Continue runs the model from `results_ready`; cancel settles without new

‎docs/design/2026-09-30-hosted-turn-failover-e2e.zh-CN.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,9 @@ Workspace 绑定文件工具会话的公开准入，但在打包栈上实际运
5757
路由交还租约；失败时留作欠账——因为一次 release 会持久化为 RELEASED，而搁浅的
5858
READY 身份仍然可用：重驱动的 cancel 会按当前 checkpoint 被重新接纳（daemon
5959
不会对已 attach 的会话重新 load），owner 变更后的接管则幂等重 acquire。
60-
只有会话退休才会清偿一次遗弃。Broker 无法交代的执行上报 `unknown`，协调器把该轮次阻塞为
60+
在已接管、但会话尚未注册成功就被拒绝的 load 上，欠账的接管会被按身份记录并报告——
61+
因为不存在可供退役的已注册会话；该记录在下一次成功加载同一会话时清除，其余情形仍只有
62+
会话退休才会清偿一次遗弃。Broker 无法交代的执行上报 `unknown`，协调器把该轮次阻塞为
6163
`managed_runtime_recovery_blocked`，什么都不重放。
6264
- **continue 从 `results_ready` 起跑模型；cancel 不做新工作直接结算。**
6365
`managed-runtime/continue` 校验 prompt、checkpoint 与 activation 身份，以 200 回执准入

‎packages/cli/src/serve/hosted-harness-session.test.ts‎

Lines changed: 103 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7062,26 +7062,121 @@ describe('Hosted Harness Runtime turn takeover', () => {
70627062
vi.spyOn(HostedWorkspaceBroker.prototype, 'status').mockResolvedValue({
70637063
state: 'prepared',
70647064
});
7065+
const stderr = vi
7066+
.spyOn(stdio, 'writeStderrLineSafe')
7067+
.mockImplementation(() => undefined);
70657068
// The restore-stage write probe passes, so the passive takeover adopts
70667069
// first; the post-recovery probe then rejects once.
70677070
state.assertWritable
70687071
.mockResolvedValueOnce(undefined)
70697072
.mockRejectedValueOnce(new Error('writer lost'));
7070-
const { loaded } = await loadReplacement(true);
7073+
const { server, loaded } = await loadReplacement(true);
70717074
expect(loaded.status).toBe(409);
70727075
expect(loaded.body.code).toBe('hosted_turn_recovery_required');
70737076
expect(acquireSpy).toHaveBeenCalled();
70747077
// The refusal invited a retried takeover load: the adopted lease must
7075-
// stay owed, or that retry re-acquires into a RELEASED record forever.
7078+
// stay owed — but never silently, since a Session closed before
7079+
// registration leaves no route to hand it back.
70767080
expect(release).not.toHaveBeenCalled();
7077-
const { server: server2, loaded: reloaded } = await loadReplacement(true);
7081+
const owedLines = () =>
7082+
stderr.mock.calls.filter(
7083+
([line]) =>
7084+
typeof line === 'string' &&
7085+
line.includes('stays owed') &&
7086+
line.includes(PROMPT_ID),
7087+
);
7088+
expect(owedLines()).toHaveLength(1);
7089+
// The retried takeover on the same daemon re-acquires the READY
7090+
// identity idempotently and reports.
7091+
const reloaded = await replacementHeaders(
7092+
supertest(server).post(`/session/${SESSION_ID}/load`),
7093+
).send({
7094+
managedSessionStore: storeFor(BOOT_ID_2),
7095+
toolProfile: FILE_PROFILE,
7096+
passiveManagedRuntimeRecovery: true,
7097+
});
70787098
expect(reloaded.status).toBe(200);
70797099
expect(
70807100
reloaded.body._meta?.['qwen.daemon.managedRuntimeRecovery'],
70817101
).toBeDefined();
7102+
expect(owedLines()).toHaveLength(1);
7103+
await replacementHeaders(
7104+
supertest(server).delete(`/session/${SESSION_ID}`),
7105+
);
7106+
});
7107+
7108+
it('records the owed adoption when every takeover load refuses the workspace writes', async () => {
7109+
await parkToolTurn();
7110+
const release = vi
7111+
.spyOn(HostedWorkspaceBroker.prototype, 'release')
7112+
.mockResolvedValue();
7113+
vi.spyOn(HostedWorkspaceBroker.prototype, 'status').mockResolvedValue({
7114+
state: 'prepared',
7115+
});
7116+
const stderr = vi
7117+
.spyOn(stdio, 'writeStderrLineSafe')
7118+
.mockImplementation(() => undefined);
7119+
const owedLines = () =>
7120+
stderr.mock.calls.filter(
7121+
([line]) =>
7122+
typeof line === 'string' &&
7123+
line.includes('stays owed') &&
7124+
line.includes(PROMPT_ID),
7125+
);
7126+
// The probe rejects after every adoption, on every attempt: the refusal
7127+
// is persistent, so the same daemon keeps refusing.
7128+
let probeCalls = 0;
7129+
state.assertWritable.mockImplementation(async () => {
7130+
probeCalls += 1;
7131+
if (probeCalls % 2 === 0) throw new Error('writer lost');
7132+
});
7133+
const { server, loaded } = await loadReplacement(true);
7134+
expect(loaded.status).toBe(409);
7135+
const second = await replacementHeaders(
7136+
supertest(server).post(`/session/${SESSION_ID}/load`),
7137+
).send({
7138+
managedSessionStore: storeFor(BOOT_ID_2),
7139+
toolProfile: FILE_PROFILE,
7140+
passiveManagedRuntimeRecovery: true,
7141+
});
7142+
expect(second.status).toBe(409);
7143+
// Every refused attempt re-adopts the same READY identity — never
7144+
// releasing it, or the wedge this PR removes would reopen — while the
7145+
// stranded adoption is reported exactly once per identity, by name.
7146+
expect(acquireSpy).toHaveBeenCalledTimes(2);
7147+
expect(release).not.toHaveBeenCalled();
7148+
expect(owedLines()).toHaveLength(1);
7149+
// The next successful load drains the record, so a later refusal must
7150+
// report again rather than stay silent on a stale one.
7151+
state.assertWritable.mockImplementation(async () => undefined);
7152+
probeCalls = 0;
7153+
const third = await replacementHeaders(
7154+
supertest(server).post(`/session/${SESSION_ID}/load`),
7155+
).send({
7156+
managedSessionStore: storeFor(BOOT_ID_2),
7157+
toolProfile: FILE_PROFILE,
7158+
passiveManagedRuntimeRecovery: true,
7159+
});
7160+
expect(third.status).toBe(200);
70827161
await replacementHeaders(
7083-
supertest(server2).delete(`/session/${SESSION_ID}`),
7162+
supertest(server).delete(`/session/${SESSION_ID}`),
70847163
);
7164+
state.assertWritable.mockImplementation(async () => {
7165+
probeCalls += 1;
7166+
if (probeCalls % 2 === 0) throw new Error('writer lost');
7167+
});
7168+
probeCalls = 0;
7169+
const fourth = await replacementHeaders(
7170+
supertest(server).post(`/session/${SESSION_ID}/load`),
7171+
).send({
7172+
managedSessionStore: storeFor(BOOT_ID_2),
7173+
toolProfile: FILE_PROFILE,
7174+
passiveManagedRuntimeRecovery: true,
7175+
});
7176+
expect(fourth.status).toBe(409);
7177+
expect(owedLines()).toHaveLength(2);
7178+
// Only the successful Session's own teardown released anything.
7179+
expect(release).toHaveBeenCalledTimes(1);
70857180
});
70867181

70877182
it('keeps the lease owed when the cancel meets a blocked session', async () => {
@@ -7196,13 +7291,16 @@ describe('Hosted Harness Runtime turn takeover', () => {
71967291
]),
71977292
);
71987293
// The admission survives the failed handback: a replay still replays at
7199-
// its watermark, and the owed lease is retried at session close.
7294+
// its watermark, and the replay discharges the owed lease — the failed
7295+
// handback (call 1) plus the replay's own (call 2).
72007296
const replayed = await cancelTurn();
72017297
expect(replayed.status).toBe(200);
72027298
expect(replayed.body.lastEventId).toBe(cancelled.body.lastEventId);
7299+
expect(release).toHaveBeenCalledTimes(2);
72037300
await replacementHeaders(
72047301
supertest(server).delete(`/session/${SESSION_ID}`),
72057302
);
7303+
// The teardown must not re-release an already-discharged lease.
72067304
expect(release).toHaveBeenCalledTimes(2);
72077305
});
72087306

‎packages/cli/src/serve/hosted-harness-session.ts‎

Lines changed: 42 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -147,11 +147,13 @@ interface HostedSession {
147147
mcpRecovering?: boolean;
148148
approval?: HostedApprovalSettings;
149149
waiters: HostedApprovalWaiters;
150-
/** A recovery load acquired the Runtime Session for this promptId. Only
151-
* the terminal success route and session teardown hand it back;
152-
* retry-inviting refusals deliberately leave it owed, because a release
153-
* persists RELEASED forever while a stranded READY lease is re-admitted
154-
* against the current checkpoint or re-acquired idempotently. */
150+
/** A recovery load acquired the Runtime Session for this promptId. On
151+
* the cancellation path, only the terminal success route and session
152+
* teardown hand it back; retry-inviting refusals deliberately leave it
153+
* owed, because a release persists RELEASED forever while a stranded
154+
* READY lease is re-admitted against the current checkpoint or
155+
* re-acquired idempotently. The continuation route keeps its #13083
156+
* handback discipline (a recorded follow-up). */
155157
runtimeLeaseHeld?: string;
156158
}
157159

@@ -1719,6 +1721,10 @@ export function registerHostedHarnessSessionRoutes(
17191721
// the Broker side: the coordinator's retried load re-acquires the
17201722
// READY identity idempotently, while a release would persist
17211723
// RELEASED and wedge every retry with runtime_session_not_acquirable.
1724+
// This Session, though, is closed before registration, so no route
1725+
// can ever see the owed lease again — record it and say so, or the
1726+
// strand is silent until retirement.
1727+
noteOwedAdoption(session, sessionId);
17221728
await managed.close();
17231729
error(res, 409, 'hosted_turn_recovery_required');
17241730
return;
@@ -1728,6 +1734,7 @@ export function registerHostedHarnessSessionRoutes(
17281734
await stores.assertWritable();
17291735
} catch {
17301736
// Same owed-lease discipline as the refusal above.
1737+
noteOwedAdoption(session, sessionId);
17311738
await managed.close();
17321739
error(res, 409, 'hosted_turn_recovery_required');
17331740
return;
@@ -1783,6 +1790,9 @@ export function registerHostedHarnessSessionRoutes(
17831790
});
17841791
}
17851792
sessions.set(sessionId, session);
1793+
// The registered Session now carries the owed lease itself; the
1794+
// refusal-time record is discharged.
1795+
refusedAdoptions.delete(sessionId);
17861796
res.status(200).json({
17871797
sessionId,
17881798
clientId: session.clientId,
@@ -2436,11 +2446,32 @@ export function registerHostedHarnessSessionRoutes(
24362446
session.managed.activation.activationId === activationId &&
24372447
unsettledPromptId(session) === promptId;
24382448

2439-
// A recovery load may hold the Runtime Session. Terminal routes hand it
2440-
// back — or the workspace lease stays pinned forever — but retry-inviting
2441-
// refusals must not (see the field doc): a release persists RELEASED.
2442-
// The flag clears only once the release is confirmed, so a failed
2443-
// handback stays owed and the next terminal route retries it.
2449+
// A takeover adoption owed on a Session closed before ever registering
2450+
// can never be handed back by a route — every discharger resolves the
2451+
// Session through this map. Record the stranded identity so it is neither
2452+
// silent nor wedged by a release; the next successful load of the id
2453+
// drains the record.
2454+
const refusedAdoptions = new Map<string, string>();
2455+
const noteOwedAdoption = (
2456+
session: HostedSession,
2457+
sessionId: string,
2458+
): void => {
2459+
const runtimeSessionId = session.runtimeLeaseHeld;
2460+
if (runtimeSessionId === undefined || refusedAdoptions.has(sessionId))
2461+
return;
2462+
refusedAdoptions.set(sessionId, runtimeSessionId);
2463+
writeStderrLineSafe(
2464+
`qwen serve: Hosted Harness takeover of session ${sessionId} adopted Runtime Session ${runtimeSessionId} but refuses the load: the lease stays owed until this session loads successfully or retires.`,
2465+
);
2466+
};
2467+
2468+
// A recovery load may hold the Runtime Session. On the cancellation
2469+
// path, terminal routes hand it back — or the workspace lease stays
2470+
// pinned forever — but retry-inviting refusals must not (see the field
2471+
// doc): a release persists RELEASED. The continuation route keeps its
2472+
// #13083 handback discipline (recorded follow-up). The flag clears only
2473+
// once the release is confirmed, so a failed handback stays owed and the
2474+
// next terminal route retries it.
24442475
const releaseRecoveredRuntime = (session: HostedSession): void => {
24452476
const promptId = session.runtimeLeaseHeld;
24462477
if (promptId === undefined || !brokerOptions) return;
@@ -2873,7 +2904,7 @@ export function registerHostedHarnessSessionRoutes(
28732904
// refuse an answered cancellation. Leave the lease owed; later
28742905
// replays and the session close retry it.
28752906
writeStderrLineSafe(
2876-
`qwen serve: Hosted Harness could not hand back the recovered Runtime Session: ${String(cause)}`,
2907+
`qwen serve: Hosted Harness could not hand back the recovered Runtime ${promptId} for session ${sessionId}: ${String(cause)}`,
28772908
);
28782909
return false;
28792910
},

0 commit comments

Comments
 (0)