Skip to content

Commit 5df01c7

Browse files
bugerclaude
andauthored
fix: hoist grantedMs/grantedMin declarations to avoid ReferenceError in ncc bundles (#545)
The `grantedMs` and `grantedMin` variables were declared with `const` inside the `if (decision.extend && decision.minutes > 0)` block but referenced in the `return` statement outside that block. While the conditional spread `...(decision.extend ? { granted_ms: grantedMs } : {})` logically only evaluates when `decision.extend` is true, bundlers like ncc can hoist or restructure code in ways that cause a ReferenceError at runtime because the block-scoped `const` is not visible outside the `if` block. Fix: declare both variables with `let` (initialized to 0) before the `if` block so they are in scope for the return statement. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent a29158a commit 5df01c7

2 files changed

Lines changed: 78 additions & 2 deletions

File tree

npm/src/agent/ProbeAgent.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3814,10 +3814,13 @@ or
38143814
const jsonStr = responseText.replace(/^```(?:json)?\s*/, '').replace(/\s*```$/, '');
38153815
const decision = JSON.parse(jsonStr);
38163816

3817+
let grantedMs = 0;
3818+
let grantedMin = 0;
3819+
38173820
if (decision.extend && decision.minutes > 0) {
38183821
const requestedMs = Math.min(decision.minutes, maxPerReqMin) * 60000;
3819-
const grantedMs = Math.min(requestedMs, remainingBudgetMs, negotiatedTimeoutState.maxPerRequestMs);
3820-
const grantedMin = Math.round(grantedMs / 60000 * 10) / 10;
3822+
grantedMs = Math.min(requestedMs, remainingBudgetMs, negotiatedTimeoutState.maxPerRequestMs);
3823+
grantedMin = Math.round(grantedMs / 60000 * 10) / 10;
38213824

38223825
// Update state
38233826
negotiatedTimeoutState.extensionsUsed++;

npm/tests/unit/negotiated-timeout.test.js

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -711,3 +711,76 @@ describe('Two-phase Graceful Stop', () => {
711711
if (agent._gracefulStopHardAbortId) clearTimeout(agent._gracefulStopHardAbortId);
712712
});
713713
});
714+
715+
// ---- 14. grantedMs / grantedMin scoping regression (block-scope fix) --------
716+
717+
describe('grantedMs / grantedMin block-scope regression', () => {
718+
test('grantedMs and grantedMin are accessible in return statement after extend block', () => {
719+
// This test replicates the observer function's control flow to verify
720+
// that grantedMs / grantedMin are accessible outside the if-block.
721+
// Before the fix, they were declared with `const` inside the if-block,
722+
// causing a ReferenceError when ncc bundled the code and the return
723+
// statement (outside the block) tried to reference them.
724+
725+
function simulateObserverReturn(decision, remainingBudgetMs, maxPerRequestMs, maxPerReqMin) {
726+
let grantedMs = 0;
727+
let grantedMin = 0;
728+
729+
if (decision.extend && decision.minutes > 0) {
730+
const requestedMs = Math.min(decision.minutes, maxPerReqMin) * 60000;
731+
grantedMs = Math.min(requestedMs, remainingBudgetMs, maxPerRequestMs);
732+
grantedMin = Math.round(grantedMs / 60000 * 10) / 10;
733+
}
734+
735+
return {
736+
decision: decision.extend ? 'extended' : 'declined',
737+
reason: decision.reason || '',
738+
...(decision.extend ? {
739+
granted_ms: grantedMs,
740+
granted_min: grantedMin,
741+
budget_remaining_ms: remainingBudgetMs - grantedMs,
742+
} : {}),
743+
};
744+
}
745+
746+
// Case 1: extend = true — grantedMs / grantedMin should have real values
747+
const extended = simulateObserverReturn(
748+
{ extend: true, minutes: 3, reason: 'work in progress' },
749+
600000, 600000, 10,
750+
);
751+
expect(extended.decision).toBe('extended');
752+
expect(extended.granted_ms).toBe(180000);
753+
expect(extended.granted_min).toBe(3);
754+
expect(extended.budget_remaining_ms).toBe(420000);
755+
756+
// Case 2: extend = false — grantedMs / grantedMin should default to 0
757+
// and not appear in the result (spread is empty object)
758+
const declined = simulateObserverReturn(
759+
{ extend: false, reason: 'task complete' },
760+
600000, 600000, 10,
761+
);
762+
expect(declined.decision).toBe('declined');
763+
expect(declined.granted_ms).toBeUndefined();
764+
expect(declined.granted_min).toBeUndefined();
765+
expect(declined.budget_remaining_ms).toBeUndefined();
766+
});
767+
768+
test('actual ProbeAgent source uses let for grantedMs/grantedMin (not const inside if-block)', async () => {
769+
// Read the source and verify the fix is in place — grantedMs/grantedMin
770+
// must be declared with let before the if-block, not const inside it.
771+
const fs = await import('fs');
772+
const path = await import('path');
773+
const sourceFile = path.resolve(
774+
new URL('../../src/agent/ProbeAgent.js', import.meta.url).pathname,
775+
);
776+
const source = fs.readFileSync(sourceFile, 'utf8');
777+
778+
// Should find "let grantedMs = 0" BEFORE the if-block
779+
const letPattern = /let grantedMs\s*=\s*0;\s*\n\s*let grantedMin\s*=\s*0;/;
780+
expect(source).toMatch(letPattern);
781+
782+
// Should NOT find "const grantedMs" (the old broken pattern)
783+
const constPattern = /const grantedMs\s*=/;
784+
expect(source).not.toMatch(constPattern);
785+
});
786+
});

0 commit comments

Comments
 (0)