feat(cli): read a debug handler from loaded integrations - #5998
Merged
Conversation
josephfarina
requested review from
cixzhang and
imdreamrunner
as code owners
September 4, 2026 00:51
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
PR Analysis Report📚 Storybook PreviewView Storybook for this PR 🧪 Sandbox PreviewView Sandbox for this PR No new or modified components detected. Bundle Size SummaryNo component packages changed. Accessibility AuditStatus: No accessibility violations detected. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
josephfarina
force-pushed
the
integration-debug-handlers
branch
from
September 4, 2026 03:15
cbe73f5 to
32f50be
Compare
github-actions
Bot
requested review from
cvkxx,
ernestt,
kentonquatman and
rubyycheung
September 4, 2026 03:16
Install an integration and its debug logs turns on. No line in the app's astryx.config, no codemod, nothing in the app at all. An integration exports `debug` from astryx.integration.*. It is a NAMED export, not a manifest key: a key has to be understood by every CLI already installed against that integration, and an older one rejects an unknown key by discarding the whole manifest - components, templates and codemods with it (#5119). A named export is simply not read by a CLI that does not know about it. Both handlers run. An app that sets its own `debug` still gets every event and so does every integration. Fallback semantics would mean an app that adds a handler to debug itself silently drops out of the integration's debug logs, and nobody would ever notice - the failure is a handler that is never called. Order is the app's own first, then integrations in config order, each with its own copy of the event inside the existing isolation, so one that throws, prints, or calls process.exit cannot reach the command or the other handlers. Also widens the pre-parse config gate, which is what makes this reach anything. It opened only on the word `debug`, and an app that just installs an integration has that word nowhere in its config - so the handler would only have fired for commands that load a Project for their own reasons, and never for --version, --help, `theme *`, or a parse error. It now opens on `integrations` too. Measured cost on an integration with a TypeScript manifest: ~50ms on those cheap commands, nothing on commands that were loading the project anyway, and nothing at all for a project that declares no integrations. Opt out with {"astryx": {"inheritDebug": false}} in package.json - inherited handlers only; the app's own `debug` is untouched.
josephfarina
force-pushed
the
integration-debug-handlers
branch
from
September 4, 2026 19:14
32f50be to
a2c773f
Compare
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.
What changes for people
Installing an integration now turns on its debug logs. Nothing in the app changes. No line in
astryx.config, no codemod, no coordinated rollout across the ~4,242 Nest apps that load@nest/xds-meta. The integration exports a handler; every app that already lists it starts reporting runs.Before this,
debugin the app's ownastryx.configwas the only way to attach a run handler, and the integration manifest schema had no way to contribute one. Getting fleet coverage meant editing every app.How
An integration exports
debugfrom itsastryx.integration.*module:A named export, not a manifest key — this is the whole reason the change is safe. A manifest key has to be understood by every CLI version already installed against that integration, and an older one rejects an unknown key by discarding the entire manifest: components, templates, codemods, docs, all of it, silently. That is #5119, and it cost
@nest/xds-metaa week of invisibility. A named export is simply not read by a CLI that does not know to look for it, which is whytransformTemplateSourcealready travels that way.So this is purely additive. An integration that adds a
debugexport keeps working unchanged on every CLI released before today.Both handlers run — this is the design decision worth arguing about
The app's own
debugand every integration's handler all receive every event.The tempting alternative is "app config wins, integration is the fallback". That is a trap: any app that adds a
debughandler to debug itself would silently remove itself from the integration's debug logs. Nobody would ever notice, because the failure mode is a handler that is never called — and fleet coverage would erode as more people used the hook for its ordinary purpose.structuredCloneof the sealed event and its own guarded call, reusing the containment feat(cli): report every command run to adebugfunction in astryx.config #4812 already built: one that throws, writes to stdout, or callsprocess.exitcannot change the command's exit code, corrupt a--jsonenvelope, or stop the handlers after it.Project.loadis a plain factory and a single command can run it twice (the pre-parse load, then the command's own), so registration replaces the set rather than growing it.The gate widening, without which this PR does nothing
loadProjectDebugHandlerruns before Commander parses, and it is gated on the config file text containing the stringdebug— deliberately, soastryx --versiondoes not evaluate a project's config for a project that never opted in.An app that just installs an integration has the word
debugnowhere in its config. Measured on a scratch app, with only the named-export plumbing and this gate untouched:component,search,docsProjectfor their own reasons--version,--help,theme list, parse errorsPartial coverage biased toward heavy commands is not what anyone wants out of a usage dataset, so the gate now also opens on
integrations. An app that lists integrations has asked for those packages' code to run — that is what an integration is.Cost, measured, 5-run medians on an integration whose manifest is TypeScript (the expensive case — jiti):
astryx --version(project declares integrations)astryx docs <topic>(already loaded the project)~50 ms, only on commands that never touched the config, only for projects that declare integrations.
e2e-smoke.test.mjsis updated to encode the new contract: a config mentioning neither word is still left unevaluated.Opting out
{"astryx": {"inheritDebug": false}}in the app's
package.json. It suppresses inherited handlers only — the app's owndebugstill runs.package.jsonrather thanastryx.configbecause config parsing is strict, so an unknown config key is a hard error on an older CLI, while anastryxblock inpackage.jsonis inert to every version that does not read it.Verified in a real app, not just in tests
A scratch app whose
astryx.config.mjscontains no occurrence of the worddebug, with fake integrations exportingdebug:--version,--help,theme list,docs,component, and an unknown-command parse error. Exactly one row per run.debugin the config and an integration exporting one, both are called, app first.process.exitCode = 77and callsprocess.exit(3): exit code stayed 0, stdout was byte-identical (cmp) to the same command in a clean app, the write was diverted to stderr, and the other two handlers still fired.--jsonstays parseable with a handler printing garbage mid-flight.Repo gates, all green on this branch:
pnpm build,pnpm test(14920 passed, 0 failed),pnpm lint:strict(0 errors),check:cli-structure,typecheck:authoring,typecheck:strict,readme:check,cli-api-types-verify,check:repo,check-knowledge. 26 new tests across three files.Also in this PR: one line that unbreaks
pnpm buildon mainmaindoes not build right now, and it is not something this PR caused. #5320 addedmatched/totalto whatscoreQueryreturns; #5289 added an early return for the exact-keyword promote tier. Both landed, and the combination does not type-check, sosync:api-types— and thereforepnpm build, and therefore this PR's CI — fails. It is a one-line fix in its own commit (52556ee), reusing theasFullhelper that already means "this hit answered the whole query".Context
Consumer of the
DebugEventcontract merged in #5971. The@nest/xds-metaside (an internal diff) already exportsdebugfrom its integration manifest and maps the event to Scuba; it is waiting on this.