Node.js: external-tool result-delivery RPC failures are silently swallowed
Nessuno ha ancora preso questa issue.
- #2909 di @FOWEPJF255 — chiusa senza merge
Valutazione
- Difficoltà
- 4/5
- Tempo stimato
- 3-5 giorni
- Idoneità per principianti
- 42/100
- Tipo di issue
- Bug
- Chiarezza
- Specificata chiaramente
- Stato di attività
- Attiva
- Stack tecnologico
- javascript, node.js
Direzione di ricerca
Start with nodejs/src/session.ts, especially _executeToolAndRespond and _claimExternalTool, then trace the result-submission RPC and its error handling. Add regression coverage for a handler that succeeds while delivery fails, alongside the existing success and handler-error cases. Done means unexpected delivery failures reach the host through a documented diagnostics or error path while cancellation, races, and teardown retain their intended behavior.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
Summary
The Node.js SDK silently swallows an unexpected failure while submitting a completed external-tool result through session.tools.handlePendingToolCall.
The tool handler succeeds, but result submission rejects. _executeToolAndRespond then resolves successfully without surfacing the delivery error. The request has already been removed from the SDK's pending invocation tracking.
If the runtime remains active and has not accepted the result, it can remain waiting for a response. This report does not establish that this code path caused any particular production timeout.
Environment and verification
- OS: Windows x64.
- Node.js: reproduced on both 24.16.0 and 27.0.0-alpha.0.0.0.
- Reproduced with the Node.js SDK bundled in platform package
@github/[email protected]. - The same control flow exists in public SDK main at commit
99d90edf504f824c77dcdf0faa978f4b35848a87. This source comparison is static; the reproduction below was executed against the installed bundle. - The reproduction uses internal SDK seams to isolate result delivery. It starts no runtime or model request and requires no credentials or network access.
Minimal reproduction
Save as repro.mjs in an environment with @github/copilot-sdk installed, then run node --test repro.mjs.
import assert from 'node:assert/strict';
import test from 'node:test';
import { CopilotSession } from '@github/copilot-sdk';
test('unexpected result-delivery errors are surfaced', async () => {
const session = new CopilotSession('repro-session', {});
let handlerCalls = 0;
let responseAttempts = 0;
session._rpc = {
tools: {
handlePendingToolCall: async () => {
responseAttempts++;
throw new Error('simulated result delivery failure');
},
},
};
await assert.rejects(
session._executeToolAndRespond(
'repro-request',
'repro-tool',
'repro-call',
{},
async () => {
handlerCalls++;
return 'done';
},
),
/simulated result delivery failure/,
);
assert.equal(handlerCalls, 1);
assert.equal(responseAttempts, 1);
});
Actual behavior
The test fails with Missing expected rejection.
A separate probe confirms that the handler ran once, result submission was attempted once, and the delivery error was not surfaced. The invocation was no longer present in pendingExternalTools.
On both tested Node.js versions, two positive controls pass and the result-delivery regression fails. The controls cover a successful result submission and a handler exception successfully submitted as an error response.
Expected behavior
An unexpected result-delivery failure should reach the host through an explicit, documented diagnostics or error path rather than silently resolving. Rejecting the internal method is one possible implementation; a structured host diagnostic would also address the observability problem.
Expected cancellation, stale-response rejection, duplicate-client races, and session teardown should retain their intended behavior.
Public source analysis
_executeToolAndRespondcalls_claimExternalToolbefore awaiting the result-submission RPC._claimExternalToolremoves the request frompendingExternalTools.- If result submission rejects, the enclosing catch calls
_claimExternalToolagain. - The second claim returns false because the first claim removed the entry.
- The catch returns without surfacing the delivery failure.
The ownership claim appears intentional for cancellation and race safety, so simply removing it or rerunning the handler would not be a safe fix.
Suggested fix direction
- Separate tool-handler failures from result-delivery failures.
- Keep the single-owner claim and cancellation guarantees.
- Surface unexpected result-delivery failures explicitly instead of routing them through a second ownership claim that suppresses them.
- If result retry is supported, retry only the completed result and document its idempotency guarantees. Do not rerun a tool that may already have produced side effects.
- Add regression coverage for successful delivery, handler failure, unexpected delivery failure, completion races, teardown, and late responses.
Related reports
- #2872 concerns the 1,800-second external-tool timeout and configuration documentation. It is closed, with no public comment or linked fix explaining a supported timeout override.
- #2368 concerns an event dropped before SDK handler entry and is closed as fixed. This report instead starts with a handler that already completed.
- #2509 introduced the request ownership and cancellation behavior and is merged.
- #2425 proposed handler-dispatch diagnostics but was closed without merging.
- Lingua principale
- Java
- Stelle
- 10.7k
- Fork
- 1.5k
- Metriche di merge delle PR
- Nessuna PR unita negli ultimi 30g
Preparare l'ambiente
- Nessun Dockerfile né file Docker Compose
- Nessun modello di pull request
- Leggi la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di github/copilot-sdk
-
Rust: ElicitationRequest drops the tool call id from elicitation.requestedForse di nuovo libera Una pull request per questa issue è stata chiusa senza essere unita. Aperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
github/copilot-sdk#2900 ·
-
Bump zod version to 4.6.6.Aperta
Difficoltà 1/5 Meno di un'ora Idoneità per principianti 84/100
github/copilot-sdk#2876 · 1 commento · 1 reazione ·
-
Python ModelLimits drops max_output_tokens from model metadataForse di nuovo libera Una pull request per questa issue è stata chiusa senza essere unita. Apertabug
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
github/copilot-sdk#2798 · 1 commento ·
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 65/100
github/copilot-sdk#2759 ·
-
docs/AGENTS.md is repo authoring guidance, but everything in docs/ gets publishedForse di nuovo libera Una pull request per questa issue è stata chiusa senza essere unita. Apertadocumentation
Difficoltà 1/5 Meno di un'ora Idoneità per principianti 85/100
github/copilot-sdk#2758 ·
Tutte le issue di github/copilot-sdk
Issue simili
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 75/100
ls1intum/thesis-management#1473 ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 76/100
LunaDeerMC/Dominion#301 · 1 commento ·
-
[FEATURE] Expose external refresh, node-level translog and warmer statsForse già presa Una pull request collegata a questa issue è aperta o già unita. Apertauntriaged
Difficoltà 2/5 1-3 ore Idoneità per principianti 68/100
opensearch-project/opensearch-prometheus-exporter#680 ·
I maintainer di solito rispondono entro 2 giorni
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 85/100
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 66/100
I maintainer di solito rispondono entro 2 giorni