fix(vscode): treat missing dependencies as a state, not an error - #29
Merged
Conversation
A nested rstack.config.* whose own dependencies are never installed (create-rstack's template-* beside their generator) made the Rstest stack log '[error] Failed to initialize project config' with a full stack trace per template, on every detection pass. 'Not installed' is now reported uniformly across the three stacks (new AGENTS.md rule): a disabled status whose reason names the restart command as the way out, plus one warn line in the output channel — never a crashed status, a stack trace, or a notification. - shared/notInstalled.ts owns the wording for all three stacks (the formatVersionMismatch precedent); the restart hint derives from the new stackCommandTitle, checked against the manifest in tests. - The Rstest worker classifies a config import failure on Node's own error code (the IPC channel drops it) and returns the verdict as data (NormalizedConfigResult); Project branches on it and latches a per-project disabled status that installs clear and dispose forgets. - StatusHolder gains a notInstalled latch ranked below crash and version mismatch, idempotent across refresh repaints. - Lint's report moves wholly into the onDocumentFailure hook, so the upstream-tracked RuntimeManager only defers to it; missing rstack logs one warn line instead of an error with a stack. - Missing @rstest/core now reports through the same path (warn + disabled status) at all three master resolution sites.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6230de53f3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Review follow-ups on the uniform not-installed policy: - lint: a missing native @rslint/core is the not-installed state, not a crash; the code-to-package mapping (missingPackageOf) is shared by the status and the warn line, and the warn names the runtime a document keeps. A misconfigured rslint corePath now throws invalid-package so a wrong setting is never reported as "install your dependencies". - rstest worker: @rstest/core is loaded before the classified config load, so a broken core install reports its real error instead of "a config dependency is missing". - rstest bridge: the not-installed latch clears the moment the rstack package resolves, and an install that ships no Rstest shim latches a version-mismatch instead of painting the folder healthy.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a35984a528
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Second review round on the uniform not-installed policy: - worker: missingDependencyCauseOf replaces isMissingDependencyError — Node's code alone also covers a typo'd relative import or a missing generated file, which installing dependencies cannot fix, so only a bare (package-name) specifier counts and anything else keeps the full error report. The returned cause is the message's first line, keeping the warn to one line without the CJS require stack. - shared: the config-dependency log line moves into shared/notInstalled (formatConfigDependencyMissingLog), deriving its consequence from STACK_LABELS, so no stack owns its own wording. - rstest status: StatusHolder latches now supersede each other per source (one source, one verdict) — a stale higher-ranked crash or mismatch can no longer paint over a newer not-installed observation, and raise sites need no manual cross-latch cleanup.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9066a5b1dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Third review round on the uniform not-installed policy:
- rstest bridge: the missing-rstack warning goes through the shared
formatNotInstalledLog instead of its own sentence.
- rstest status: a package-state observation (mismatch or not-installed)
restates its root — it retires the other kind AND a stale crash,
whose only other exit (workerSpawned) cannot fire while the package
is unusable. The config-dependency verdict moves to its own
config-deps: latch key (the nodeRuntimeStatusSource precedent), so it
coexists with the core version check instead of erasing it.
- worker classifier: a bare-looking subpath of an installed package
(require('pkg/missing')) is a source error, not the not-installed
state — confirmed against the physical node_modules with the same
uncached walk-up the rest of the stack resolves packages with.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b832e7612
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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
1 participant
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.
Summary
Opening a repository that contains nested
rstack.config.*files whose own dependencies are never installed — e.g.create-rstack'stemplate-*directories beside their generator — made the Rstest stack log[error] Failed to initialize project configwith a full stack trace per template, on every detection pass. The dependencies-not-installed situation is the normal state of a fresh clone and of scaffolded templates, not an error.This PR makes "not installed" a state, reported uniformly by all three stacks (new AGENTS.md rule): a
disabledstatus whose reason names the restart command as the way out, plus onewarnline in the output channel — never acrashedstatus, a stack trace, or a notification.shared/notInstalled.tsowns the status and log sentences for all three stacks (theformatVersionMismatchprecedent). The restart hint derives from the newstackCommandTitleintypes.ts, andtests/extension.test.tschecks it against the manifest so a renamed command cannot drift silently.codeinside the worker (the IPC channel's advanced serialization dropscode), and the verdict travels as data (NormalizedConfigResult) end to end;Projectbranches on it and latches a per-projectdisabledstatus that a successful resolution clears anddisposeforgets.StatusHoldergains anotInstalledlatch ranked belowcrashedandversion-mismatch(matching the fmt/lint folds), idempotent across refresh repaints. A missing@rstest/coreand a bridge directory withoutrstacknow report through the same path.onDocumentFailurehook instacks/lint/index.ts, so the upstream-trackedRuntimeManageronly defers to it; a bridged folder withoutrstacklogs onewarnline instead of an error with a stack.Related Links
N/A
Checklist