fix(axi-sdk-js): route initialize and resolveContext failures through the AXI error contract - #122
Open
nathanbenn18 wants to merge 3 commits into
Open
Conversation
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Corrigir upstream o limite de tratamento de erros de axi-sdk-js identificado e reproduzido na investigação em /Users/nathanbennesby/firstmate/data/axi-opportunities-scout-a1/report.md. Falhas de initialize() e resolveContext() devem seguir o contrato público do handler: saída AXI estruturada em stdout, stderr vazio para erros de domínio e exit code apropriado. AxiError deve preservar código, mensagem e sugestões; erros desconhecidos devem ser normalizados como UNKNOWN sem stack trace exposto. Ajuda, versão, home, comando desconhecido, contexto lazy e precedência de handler não podem regredir. A investigação deve reproduzir primeiro pelo executável real usando o build atual, registrar trigger, condição e sintoma, comparar initialize e resolveContext com o handler, e inspecionar histórico e testes procurando inclusive evidência que refute a causa. Os testes de regressão devem começar vermelhos para initialize, resolveContext no home e resolveContext em comando, cobrindo AxiError e erro genérico, stdout, stderr e exit status pelo contrato público ou subprocesso. Fazer a menor mudança coerente que coloque essas fases sob o limite correto, sem duplicar formatadores nem esconder erros de programação além do contrato existente. Atualizar documentação somente se o comportamento público documentado realmente mudar; não editar CHANGELOG.md, arquivos gerados, benchmarks, hooks, update ou divergências documentais alheias. Executar build, formato, lint, testes do SDK e checks relevantes do repositório. A entrega local não basta: executar o pipeline no-mistakes completo a partir do commit, tratar seus retornos pela própria pipeline, abrir PR upstream, aguardar checks verdes e nunca fazer merge.
What Changed
runAxiCli()now awaitsoptions.initialize?.()inside an error boundary and movesresolveContextintorunHandler, so failures in either phase emit structured AXI output on stdout with the mapped exit code instead of an unhandled rejection and raw stack trace.initializeis widened to() => MaybePromise<void>, and context is still resolved only for the home view and real commands, so--help,--versionand unknown commands stay lazy.writeFormattedError()helper shared by the handler path and the built-inupdatepath;formatErrorprecedence and the existingAxiErrorcode/message/suggestion mapping are unchanged.test/cli.test.tsplus a newtest/fixtures/error-boundary-bin.mjs) asserting stdout, stderr and exit status forinitialize(sync and async) andresolveContextfailures on both the home view and a command, forAxiErrorand generic errors.Note for consumers:
initialize/resolveContextrejections no longer propagate out ofrunAxiCli(), so a tool wrapping the call in its own try/catch will no longer see them for those phases — they are formatted by the SDK, still overridable viaformatError.Risk Assessment
✅ Low: The change is a small, well-bounded error-boundary fix confined to one SDK file plus new tests, it preserves every lazy-context and dispatch-precedence invariant the intent marks as non-regressable, and it is covered by public-contract subprocess tests asserting stdout, stderr, and exit code together.
Testing
I installed deps, built the SDK, and ran the targeted
test/cli.test.tssuite plus the small axi-sdk-js package suite (all green), then proved the intent at product level: a realdemo-axiexecutable built on the compiled dist shows initialize() and resolveContext() failures now rendering structured AXI errors on stdout with empty stderr and correct exit codes (AxiError keeps code/message/suggestions, VALIDATION_ERROR maps to 2, plain errors normalize to UNKNOWN with no stack), whereas the same transcript against a dist rebuilt from the base commit reproduces the reported stack-trace-on-stderr/empty-stdout defect. The new regression tests were confirmed to start red on the base cli.ts and green on the fix, and help, version, home, command dispatch, unknown command, leading-flag error, lazy context and built-in-update precedence all behave unchanged through the real binary. Build output was removed afterwards and the worktree is clean.Evidence: Before/after comparison of the real CLI (stdout, stderr, exit) for all four failure phases
$ DEMO_FAIL=resolveContext DEMO_ERROR=axi demo-axi issue list BEFORE (93c5f33) AFTER (98b0072) stdout: <empty> stdout: error: Not authenticated to demo service code: AUTH_ERROR help[2]: Rundemo-axi auth login,Set DEMO_TOKEN in the environment stderr: AxiError stack… stderr: <empty> exit: 1 exit: 1 $ DEMO_FAIL=initialize DEMO_ERROR=generic demo-axi BEFORE: stdout <empty>, stderr "Error: socket hang up" + stack, exit 1 AFTER: stdout "error: socket hang up / code: UNKNOWN", stderr <empty>, exit 1Evidence: Full CLI transcript with the fix (failure phases + non-regression surfaces + lazy context)
Evidence: Same transcript against the pre-fix build — reproduces the reported defect
Evidence: VALIDATION_ERROR from initialize/resolveContext exits 2 with empty stderr (real binary)
$ DEMO_FAIL=resolveContext DEMO_ERROR=validation demo-axi issue list error: Missing --repo flag code: VALIDATION_ERROR help[1]: Rundemo-axi issue list --repo owner/namestderr: <empty> exit: 2Evidence: Regression tests start red on the base cli.ts (only the 8 new cases fail)
× formats 'axi' initialize failures on stdout × formats 'generic' initialize failures on stdout × formats 'axi' asynchronous initialize rejections on stdout × formats 'generic' asynchronous initialize rejections on stdout × formats 'axi' resolveContext failures for home on stdout × formats 'generic' resolveContext failures for home on stdout × formats 'axi' resolveContext failures for commands on stdout × formats 'generic' resolveContext failures for commands on stdout Tests 8 failed | 24 passed (32)/var/folders/dh/ny_k7wbn1lv56zxvwr3jz8100000gn/T/no-mistakes-evidence/01KZ28RZ3KEVVAHR61VPRAP47R/demo-axi)Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
packages/axi-sdk-js/src/cli.ts:144- Residual gap of the same class, outside this change's authorized scope:options.getCommandHelp?.(command)(line 144),options.renderUnknownCommand?.(command)(line 154), andoptions.formatErrorinsidewriteFormattedError(line 211) are still invoked with no error boundary. A tool whosegetCommandHelpthrows ontool issue --helpstill produces a raw stack trace on stderr with empty stdout and exit 1 — the exact contract violation just fixed for initialize/resolveContext. The intent scopes the fix to those two phases and asks for the smallest coherent change, so this is reported as informational only, not as an incomplete fix.packages/axi-sdk-js/test/cli.test.ts:40-runErrorBoundaryFixturere-derives the fixture path and the../node_modules/.bin/vite-nodepath plus theexecFileAsyncinvocation that the pre-existing version test already inlines at lines 545-557. Extracting onerunFixtureBin(fixtureName, args)helper that both call would remove the duplicated path resolution and the duplicatedcwdwiring. Test-only, no behavior impact.packages/axi-sdk-js/src/cli.ts:175- Observable behavior change for published-SDK consumers: previouslyinitialize/resolveContextrejections propagated out ofrunAxiCli(), so a tool wrapping the call in its own try/catch saw them. They are now caught and formatted by the SDK, so such an outer catch no longer fires for these phases. This is exactly what the intent requires and tools can still override viaformatError, so no action is needed — but it is worth one line in the PR body since it ships as a patch bump under the pre-1.0 release-please config.✅ **Test** - passed
✅ No issues found.
pnpm install --filter axi-sdk-js...thenpnpm --dir packages/axi-sdk-js run build(tsc build succeeds with the widenedinitialize?: () => MaybePromise<void>signature)vitest run test/cli.test.tsin packages/axi-sdk-js — 32 passed, including the 8 new subprocess error-boundary casesRed-baseline check:git checkout 93c5f33 -- packages/axi-sdk-js/src/cli.ts+ rebuild, thenvitest run test/cli.test.ts— exactly the 8 new tests fail (initialize sync/async, resolveContext home, resolveContext command × AxiError/generic), 24 others pass; file restored and worktree verified cleanvitest runfor the axi-sdk-js package (5 suites) to confirm errors/output/hooks/update suites are unaffected by the boundary changeManual CLI transcript over a realdemo-axiexecutable importing the freshly compileddist/index.js:bash transcript.shcapturing stdout/stderr/exit separately for initialize and resolveContext failures (AxiError and generic), handler-throw control, home,issue list,--help,--version, unknown command and leading-flag errorSame transcript re-run against a dist rebuilt from basecli.tsto reproduce the reported defect (empty stdout, stack trace on stderr, exit 1) before/after comparisonLazy-context check through the real binary:--help,--versionand an unknown command withresolveContextrigged to throw — all succeed, proving context is still resolved only for views that need itExit-code mapping check through the real binary with aVALIDATION_ERRORAxiError thrown from initialize and from resolveContext (home and command) — exit 2, stderr emptypackages/axi-sdk-js/README.md:105- Judgment call, left as-is: the publicinitializeoption is not documented in packages/axi-sdk-js/README.md, and this change widened it toMaybePromise<void>and moved initialize/resolveContext failures under the handler error boundary. No existing doc states anything now false (the README's "lazy context resolution" claim still holds), and the invariant is owned by the code comment in packages/axi-sdk-js/src/cli.ts at the boundary, so documentinginitializehere would add a new fact rather than fix a stale one. If maintainers want the option surfaced to AXI authors, a follow-up should add it to the README Reference table alongside the structured-error contract for the initialize phase.✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.