Skip to content

fix: hoist grantedMs/grantedMin to avoid ReferenceError in ncc bundles#545

Merged
buger merged 1 commit intomainfrom
fix/granted-ms-block-scope
Mar 20, 2026
Merged

fix: hoist grantedMs/grantedMin to avoid ReferenceError in ncc bundles#545
buger merged 1 commit intomainfrom
fix/granted-ms-block-scope

Conversation

@buger
Copy link
Collaborator

@buger buger commented Mar 20, 2026

Summary

  • grantedMs and grantedMin were declared with const inside the if (decision.extend && decision.minutes > 0) block in the negotiated timeout observer, but referenced in the return statement outside that block
  • When bundled with ncc (e.g., in Visor), this causes a ReferenceError at runtime because block-scoped const variables are not visible outside the if block
  • Fix: declare both variables with let (initialized to 0) before the if block so they remain in scope for the return statement

Test plan

  • Added regression test verifying grantedMs/grantedMin are accessible in the return statement for both extend and decline paths
  • Added source verification test confirming let declarations are used instead of const
  • Existing negotiated-timeout tests continue to pass

🤖 Generated with Claude Code

…in ncc bundles

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>
@buger buger merged commit 5df01c7 into main Mar 20, 2026
13 checks passed
@probelabs
Copy link
Contributor

probelabs bot commented Mar 20, 2026

PR Overview: Fix Block-Scoping Issue in Negotiated Timeout Observer

Summary

This PR fixes a critical JavaScript scoping bug that caused ReferenceError when the ProbeAgent code was bundled with ncc (e.g., in Visor). The issue occurred because grantedMs and grantedMin were declared as block-scoped const variables inside an if block but were referenced in a return statement outside that block.

Files Changed

  • npm/src/agent/ProbeAgent.js (+5, -2)
  • npm/tests/unit/negotiated-timeout.test.js (+73, -0)

Technical Changes

Root Cause

In the negotiated timeout observer function within ProbeAgent.js, the variables grantedMs and grantedMin were declared with const inside the conditional block:

if (decision.extend && decision.minutes > 0) {
  const requestedMs = Math.min(decision.minutes, maxPerReqMin) * 60000;
  const grantedMs = Math.min(requestedMs, remainingBudgetMs, negotiatedTimeoutState.maxPerRequestMs);
  const grantedMin = Math.round(grantedMs / 60000 * 10) / 10;
  // ...
}
return {
  // ... references grantedMs and grantedMin here
};

When bundled with ncc, this created a temporal dead zone issue where the variables were not accessible in the return statement.

Fix Applied

The fix hoists the variable declarations outside the if block using let (initialized to 0):

let grantedMs = 0;
let grantedMin = 0;

if (decision.extend && decision.minutes > 0) {
  const requestedMs = Math.min(decision.minutes, maxPerReqMin) * 60000;
  grantedMs = Math.min(requestedMs, remainingBudgetMs, negotiatedTimeoutState.maxPerRequestMs);
  grantedMin = Math.round(grantedMs / 60000 * 10) / 10;
  // ...
}
return {
  // ... now can safely reference grantedMs and grantedMin
};

This ensures the variables are in scope for the return statement regardless of which code path executes.

Architecture & Impact

Component Affected

  • ProbeAgent negotiated timeout observer - The observer function that handles LLM-negotiated timeout extensions during long-running operations

Flow Diagram

graph TD
    A[Negotiated Timeout Trigger] --> B{LLM Decision}
    B -->|extend=true| C[Calculate grantedMs/grantedMin]
    B -->|extend=false| D[Skip calculation]
    C --> E[Update state]
    D --> E
    E --> F[Return result object]
    F --> G{grantedMs/grantedMin accessible?}
    G -->|Before fix| H[ReferenceError in ncc bundles]
    G -->|After fix| I[✓ Variables in scope]
Loading

Impact Scope

  • Runtime behavior: Fixes crashes in ncc-bundled environments (Visor)
  • Code paths: Both "extend" and "decline" decision paths now work correctly
  • Backward compatibility: No API changes; purely a bug fix

Test Coverage

The PR adds comprehensive regression tests:

  1. Behavioral test (simulateObserverReturn): Verifies that grantedMs and grantedMin are accessible in the return statement for both:

    • Extension path (extend=true): Variables contain calculated values
    • Decline path (extend=false): Variables remain at default 0 (not included in result)
  2. Source verification test: Reads the actual source file to confirm:

    • let grantedMs = 0 and let grantedMin = 0 declarations exist before the if block
    • No const grantedMs declarations remain (the broken pattern)

Review Notes

  • Risk level: Low - isolated scoping fix with comprehensive test coverage
  • Side effects: None - variables are initialized to 0, maintaining same default behavior
  • Bundle compatibility: Specifically addresses ncc bundling behavior with block-scoped variables
Metadata
  • Review Effort: 1 / 5
  • Primary Label: bug

Powered by Visor from Probelabs

Last updated: 2026-03-20T20:27:31.829Z | Triggered by: pr_opened | Commit: fb40ab7

💡 TIP: You can chat with Visor using /visor ask <your question>

@probelabs
Copy link
Contributor

probelabs bot commented Mar 20, 2026

\n\n

✅ Architecture Check Passed

No architecture issues found – changes LGTM.

✅ Performance Check Passed

No performance issues found – changes LGTM.

✅ Quality Check Passed

No quality issues found – changes LGTM.


Powered by Visor from Probelabs

Last updated: 2026-03-20T20:22:31.333Z | Triggered by: pr_opened | Commit: fb40ab7

💡 TIP: You can chat with Visor using /visor ask <your question>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant