Skip to content

Commit 93e9e10

Browse files
deepcoldyclaude
andcommitted
fix(trigger): 收 codex #776 四轮 2 P1(requestHash 全量绑定 + 布尔 scope-gate 严格化)
前 6 crash-atomicity blocker 已 held;四轮发现 2 个新 surface P1: A. requestHash 只 hash model/reasoningEffort/suppressFinalOutput,但 prompt 由整个 req.options+source+envelope+instruction+presentation 渲染。options.status firing→resolved(及 dedupKey 等)改 prompt 不改 hash → 同键静默复用而非文档承诺的 409 (codex 实测 prompt differs=true hash same=true)。修:requestHash 覆盖 instruction/ envelope/source/presentation + **整个 options 去掉 idempotencyKey**(key 是查找键非 payload;无 daemon 生成 id 混入,跨重试稳定)。 B. boolean scope-gate 与运行时不一致:validator 用 ===true 派生 async/wait, triggerSessionTurn 用 truthiness。asyncReturnSessionId:true + waitForFinalOutput:"false" 过 validator 却进 wait 分支 → fork 但不过 reserved→attempting barrier → lease 留 reserved → boot reconcile 当「从未派发」删 → 同键重试真跑第二遍(at-most-once 洞)。修:validator 严格校验 dryRun/waitForFinalOutput/asyncReturnSessionId 必须 boolean 类型,非布尔 400。 测试:trigger-api 新增非布尔 flag 拒绝("false"/1/0/"yes" 六格);e2e 新增「同 key 同 instruction 异 options.status → 409」(证 hash 覆盖全 options)。build 绿;affected+shared 8 套件 190/190 绿。普通 trigger/webhook 零行为变化。 Co-Authored-By: Claude <noreply@anthropic.com>
1 parent d7ebdb3 commit 93e9e10

4 files changed

Lines changed: 53 additions & 8 deletions

File tree

src/core/trigger-session.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -512,20 +512,23 @@ export async function triggerSessionTurn(
512512
// "session exists → reuse" map: a turn that crashed before/mid dispatch must
513513
// resolve to a terminal state, never silently re-run or hang `running`.
514514
const idempotencyKey = req.options?.idempotencyKey?.trim();
515-
// requestHash binds the key to its business payload — a same-key retry with a
516-
// different payload is a caller bug (409), not a silent join. Exclude the key
517-
// itself and daemon-generated ids (session/chat); include what drives execution.
515+
// requestHash binds the key to its full business payload — a same-key retry
516+
// with a DIFFERENT payload is a caller bug (409), not a silent join. It must
517+
// cover everything that renders into the prompt / drives execution:
518+
// instruction, envelope, source, presentation, and the WHOLE options object
519+
// EXCEPT the idempotencyKey itself (that's the lookup key, not payload). Hashing
520+
// only a hand-picked subset (model/effort/suppress) silently reused a turn when
521+
// e.g. options.status firing→resolved changed the prompt but not the hash
522+
// (codex #776 round-4). No daemon-generated ids (session/chat/triggerId) are in
523+
// these inputs, so the hash is stable across retries.
524+
const { idempotencyKey: _omitKey, ...optionsForHash } = (req.options ?? {}) as Record<string, unknown>;
518525
const requestHash = idempotencyKey
519526
? computeInputHash({
520527
instruction: req.instruction ?? null,
521528
envelope: req.envelope,
522529
source: req.source,
523530
presentation: req.presentation ?? null,
524-
options: {
525-
model: req.options?.model ?? null,
526-
reasoningEffort: req.options?.reasoningEffort ?? null,
527-
suppressFinalOutput: req.options?.suppressFinalOutput ?? null,
528-
},
531+
options: optionsForHash,
529532
})
530533
: '';
531534
const ownerBootId = getDaemonBootId();

src/services/trigger-types.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,17 @@ export function validateTriggerRequest(raw: unknown): { ok: true; request: Trigg
174174
return { ok: false, status: 400, body: { ok: false, errorCode: 'target_required', error: 'target.kind must be turn or workflow' } };
175175
}
176176
const options = isRecord(raw.options) ? raw.options : {};
177+
// Strict boolean typing for the mode/gate flags. The validator derives these
178+
// with `=== true` but triggerSessionTurn consumes some with truthiness; a
179+
// non-boolean (e.g. "false" / 1) would pass a scope gate here yet take a
180+
// different runtime branch — which, for an idempotency turn, could skip the
181+
// reserved→attempting barrier and break at-most-once. Reject non-booleans so
182+
// the two layers can never diverge (codex #776 round-4).
183+
for (const flag of ['waitForFinalOutput', 'asyncReturnSessionId', 'dryRun'] as const) {
184+
if (options[flag] !== undefined && typeof options[flag] !== 'boolean') {
185+
return { ok: false, status: 400, body: { ok: false, errorCode: 'bad_request', error: `options.${flag} must be a boolean` } };
186+
}
187+
}
177188
const waitForFinalOutput = options.waitForFinalOutput === true;
178189
const asyncReturnSessionId = options.asyncReturnSessionId === true;
179190
const hasChatId = typeof target.chatId === 'string' && target.chatId.trim().length > 0;

test/trigger-api.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,25 @@ describe('trigger request contract', () => {
201201
expect(validateTriggerRequest(req).ok).toBe(true);
202202
});
203203

204+
it('rejects non-boolean mode/gate flags (prevents validator/runtime scope divergence)', () => {
205+
// codex #776 round-4: a non-boolean waitForFinalOutput/asyncReturnSessionId/
206+
// dryRun would pass the `=== true` scope gate yet flip a truthiness branch at
207+
// runtime — for an idempotency turn that skips the reserved→attempting barrier
208+
// and breaks at-most-once. Must 400.
209+
for (const [flag, val] of [
210+
['waitForFinalOutput', 'false'], ['waitForFinalOutput', 1],
211+
['asyncReturnSessionId', 'true'], ['asyncReturnSessionId', 0],
212+
['dryRun', 'false'], ['dryRun', 'yes'],
213+
] as Array<[string, unknown]>) {
214+
const req = request();
215+
req.target = { kind: 'turn', botId: 'app1' };
216+
(req.options as any) = { [flag]: val };
217+
const v = validateTriggerRequest(req);
218+
expect(v.ok).toBe(false);
219+
if (!v.ok) expect(v.body.errorCode).toBe('bad_request');
220+
}
221+
});
222+
204223
it('builds a prompt that labels event data as untrusted', () => {
205224
const prompt = buildUntrustedEventPrompt(request(), 'trg_1');
206225
expect(prompt).toContain('untrusted event data');

test/trigger-session-idempotency-e2e.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,18 @@ describe('triggerSessionTurn — idempotency dispatch (real stores)', () => {
138138
expect(mockForkWorker).toHaveBeenCalledTimes(1);
139139
});
140140

141+
it('same key, same instruction, but DIFFERENT options.status → 409 (requestHash covers full options)', async () => {
142+
// codex #776 round-4: status firing→resolved changes the rendered prompt; the
143+
// hash must change too, else the resolved event silently reuses the firing turn.
144+
const firing: TriggerRequest = { ...freshAsyncReq('k-status'), options: { asyncReturnSessionId: true, idempotencyKey: 'k-status', status: 'firing' } };
145+
const resolved: TriggerRequest = { ...freshAsyncReq('k-status'), options: { asyncReturnSessionId: true, idempotencyKey: 'k-status', status: 'resolved' } };
146+
await triggerSessionTurn(firing, { larkAppId: APP, activeSessions: new Map() });
147+
const res = await triggerSessionTurn(resolved, { larkAppId: APP, activeSessions: new Map() });
148+
expect(res.ok).toBe(false);
149+
expect(res.errorCode).toBe('idempotency_conflict');
150+
expect(mockForkWorker).toHaveBeenCalledTimes(1);
151+
});
152+
141153
it('fork throw AFTER the barrier → durable async failed(dispatch_unknown) + close, retry does NOT re-run', async () => {
142154
forkShouldThrow = true;
143155
const res = await triggerSessionTurn(freshAsyncReq('k-4'), { larkAppId: APP, activeSessions: new Map() });

0 commit comments

Comments
 (0)