Skip to content

fix(check): await the now-async getExecutor in the known-symbols gate (#11421) - #11582

Merged
diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.51from
ntdatt812:fix/11421-known-symbols-async-getexecutor
Aug 26, 2026
Merged

diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.51from
ntdatt812:fix/11421-known-symbols-async-getexecutor

Conversation

@ntdatt812

Copy link
Copy Markdown
Contributor

The gate cannot pass

Fast Quality Gates fails on every branch. Its remaining sub-check failure is:

[known-symbols] 1 sub-checagem(ns) falharam:
[executor] 142 alias(es) registrado(s) não resolvem para um BaseExecutor válido
           (instância + execute() + getProvider()):
    ✗ antigravity
    ✗ agy
    ✗ github
    … 139 more

142 out of 142. That number is the tell — this is not 142 broken executors.

Cause

#11421 (7cfabfc5c, ~5h ago)
made the executor registry lazy, so getExecutor() is now async and hands back a
Promise; the class is imported and constructed on first use. That PR did touch this
gate — but only its source parser:

-  const start = indexSource.indexOf("const executors = {");
+  const start = indexSource.indexOf("const lazyExecutors");

The resolver half was left as it was:

const badExecutors = findNonConformingExecutors(aliases, getExecutor, isExecutorInstance);
//                                                       ^ returns a Promise now

A Promise is never instanceof BaseExecutor, so every alias is judged non-conforming.
Probed directly under bun:

total aliases parsed: 142
good: 0  bad: 142
getExecutor("antigravity") → typeof object, ctor: Promise
  instanceof BaseExecutor: false, execute: undefined, getProvider: undefined

The production side of #11421 is fine — I grepped every getExecutor( call site in
open-sse/, src/, scripts/ and bin/, and each one already awaits. Only the gate
was left behind.

Why this is worth more than a green X

While the sub-check reports 142/142 it is not just failing, it is dead. It cannot
tell a genuinely broken executor from the 141 false positives standing next to it, so
the whole class of defect it was built to catch — an alias that silently routes to the
DefaultExecutor fallback instead of failing — is currently unguarded.

The fix

  • findNonConformingExecutors awaits the resolver.
  • A rejection also counts as non-conforming. With lazy loading an alias's import()
    can now fail at runtime; that is precisely the dead symbol this sub-check exists to
    find, and the synchronous version could not have seen that failure mode at all.
  • The unit-test fixture for extractExecutorAliases still declared const executors,
    which the post-perf(executors): lazy-load the executor registry — defer class imports + construction to first use (#11220) #11421 parser cannot locate — so that test was red too. It now carries
    the lazy thunk shape the real file has.
  • Two cases added: the resolver is awaited rather than judged as a Promise, and a
    rejecting lazy load is flagged.

No production code touched.

Verification

The gate goes green, and reports what it should:

DATA_DIR=<tmp> bun scripts/check/check-known-symbols.ts
EXIT=0
[known-symbols] OK — 142 executores conformes; 20 estratégias canônicas
  (20 via despacho + 0 default(s) implícito(s)); 15 pares de tradutor vivos vs 15
  congelados; 96 tools MCP (todos com scope) vs 85 congelados …; 6 A2A skills
  (handlers↔card OK); 4 cloud agents (registry↔files OK)

It is not passing vacuously. I pointed one alias's lazy import at a module that does
not exist (auggie → ./does-not-exist.ts) and re-ran it:

EXIT=1
[executor] 1 alias(es) registrado(s) não resolvem para um BaseExecutor válido:
    ✗ auggie

Exactly one, correctly named — and that is a failure mode the pre-#11421 synchronous
check could never have detected, because a failed lazy import only surfaces on await.

Unit tests — tests/unit/check-known-symbols.test.ts:

# base branch
✖ extractExecutorAliases parses quoted and bare keys from the executors literal
ℹ tests 38  ℹ pass 37  ℹ fail 1

# with this change
ℹ tests 40  ℹ pass 40  ℹ fail 0
npx eslint <both files>     # 0 errors
npx prettier --write        # clean

Ran under Bun 1.3.14 with an isolated DATA_DIR, matching how CI invokes it
(bun scripts/check/check-known-symbols.ts).

…cheaper-inference#11421)

cheaper-inference#11421 made the executor registry lazy: `getExecutor()` became `async` and returns a
Promise so the class is imported and constructed on first use. It updated the gate's
source parser (`const executors` → `const lazyExecutors`) but not the resolver half,
which still calls `getExecutor(alias)` synchronously.

A Promise is never `instanceof BaseExecutor`, so every registered alias is reported
non-conforming — all 142 of them:

    [executor] 142 alias(es) registrado(s) não resolvem para um BaseExecutor válido

That fails `Fast Quality Gates` on every branch, and worse, the sub-check is now dead:
it cannot pass, so it can no longer distinguish a genuinely broken executor from the
141 false positives around it.

- `findNonConformingExecutors` awaits the resolver. A rejection counts as
  non-conforming too — with lazy loading an alias's import can fail at runtime, and a
  dead symbol is exactly what this sub-check exists to find. The sync version could
  not have seen that failure mode at all.
- The unit-test fixture for `extractExecutorAliases` still declared `const executors`,
  which the post-cheaper-inference#11421 parser cannot locate, so that test was red as well. It now
  carries the lazy thunk shape the real file has.
- Two cases added: the resolver is awaited rather than judged as a Promise, and a
  rejecting lazy load is flagged.

No production code touched — every runtime caller of `getExecutor` already awaits.
@diegosouzapw
diegosouzapw merged commit 1ea1345 into cheaper-inference:release/v3.8.51 Aug 26, 2026
10 of 16 checks passed
diegosouzapw pushed a commit that referenced this pull request Aug 26, 2026
Merged via /merge-batch (lote 2026-08-26 batch 2, v3.8.51). 7 conflitos, todos triviais/duplicados (mesmos base-reds já corrigidos por PRs paralelas mergeadas neste lote — #11580/#11582/#11583/#11585/#11588/#11589/#11590/#11591/#11609): mantida a versão já validada nesses casos. Validado: 68/68 testes passando. Obrigado por resolver os base-reds.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…cheaper-inference#11421) (cheaper-inference#11582)

Merged via /merge-batch (lote 2026-08-26, v3.8.51). Boarded no worktree combinado junto com outras ~30 PRs; validação única: typecheck/complexity/cognitive-complexity/changelog-integrity verdes, file-size rebaseado onde necessário (crescimento legítimo), lint com os mesmos 228 achados pré-existentes confirmados via sonda contra o tip puro (não introduzidos por este lote), e ~370 testes focados (unit + vitest) passando. Obrigado pela contribuição.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…per-inference#11608)

Merged via /merge-batch (lote 2026-08-26 batch 2, v3.8.51). 7 conflitos, todos triviais/duplicados (mesmos base-reds já corrigidos por PRs paralelas mergeadas neste lote — cheaper-inference#11580/cheaper-inference#11582/cheaper-inference#11583/cheaper-inference#11585/cheaper-inference#11588/cheaper-inference#11589/cheaper-inference#11590/cheaper-inference#11591/cheaper-inference#11609): mantida a versão já validada nesses casos. Validado: 68/68 testes passando. Obrigado por resolver os base-reds.
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.

2 participants