Verify runtime version in fedify init - #981
Open
userjmmm wants to merge 7 commits into
Open
Conversation
Fedify has minimum runtime versions, and some frameworks require even newer ones. `fedify init` did not check this, so it could scaffold a project on a runtime below the minimum, which can then fail at runtime. Verify the selected runtime version before creating any files. The base minimums (Deno 2.0.0, Node.js 22.0.0, Bun 1.1.0) mirror the values declared by `@fedify/fedify` and are defined directly in `rt.json`. Frameworks with stricter requirements, such as Astro's Node.js 22.12, raise the minimum through `minRuntimeVersions`. Assisted-by: Claude Code:claude-opus-4-8
Cover `verifyRuntimeVersion` for versions below, equal to, and above the minimum, and `resolveRequiredVersion` for framework overrides that raise the base minimum. Assisted-by: Claude Code:claude-opus-4-8
Assisted-by: Claude Code:claude-opus-4-8
✅ Deploy Preview for fedify-json-schema canceled.
|
📝 WalkthroughWalkthrough
ChangesRuntime verification
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant FedifyInit
participant fillPackageManager
participant checkAllRuntimes
participant RuntimeCommands
FedifyInit->>fillPackageManager: resolve package manager
fillPackageManager->>checkAllRuntimes: validate runtime requirements
checkAllRuntimes->>RuntimeCommands: execute version commands
RuntimeCommands-->>checkAllRuntimes: return runtime statuses
checkAllRuntimes-->>fillPackageManager: return compatible options
fillPackageManager-->>FedifyInit: continue or exit
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/init/src/ask/pm.ts`:
- Around line 40-64: Update the explicit package-manager branch before its
availability/runtime checks to validate isWfSupportsPm(options.webFramework,
pm), matching the interactive selection path. Reject unsupported combinations
using the existing error/exit behavior, while preserving the current checks and
successful return for compatible package managers.
- Around line 53-55: Update the result.status === "missing" branch to include
actionable runtime installation guidance, using runtimes[pmToRt(pm)].label and
an install URL. Extend the installer metadata or add a runtime helper to support
the getInstallUrl signature needed for this path, while preserving the existing
missing-runtime error message.
In `@packages/init/src/lib.ts`:
- Around line 221-224: Update the return type of checkRuntimeVersion to use the
imported RuntimeCheck type instead of redeclaring the equivalent union, matching
checkRuntimeRequirement and keeping the shared contract centralized.
- Around line 239-249: Update checkRuntimeVersion so unexpected runtime-check
errors are converted into the same result shape used for missing, unsupported,
and malformed runtimes instead of being rethrown. Ensure checkAllRuntimes and
askPackageManager handle that result by printing an actionable CLI message and
exiting cleanly, while preserving the existing not-found behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6e67122f-54e4-4ac4-a1fb-5b08cbe4b156
📒 Files selected for processing (9)
CHANGES.mdchanges.d/init/verify-runtime-version.mdpackages/init/src/ask/pm.tspackages/init/src/const.tspackages/init/src/json/rt.jsonpackages/init/src/lib.test.tspackages/init/src/lib.tspackages/init/src/types.tspackages/init/src/webframeworks/astro.ts
Comment on lines
+53
to
+55
| if (result.status === "missing") { | ||
| printErrorMessage`The runtime for package manager ${pm} is missing.`; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check how getInstallUrl and noticeInstallUrl are used across pm.ts.
set -euo pipefail
fd 'pm.ts' packages/init/src/ask | xargs cat -nRepository: fedify-dev/fedify
Length of output: 5877
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## candidate lib.ts files"
fd 'lib\.ts$' packages/init/src/ask | sort
echo
echo "## imports/usages of getInstallUrl pmToRt"
rg -n "getInstallUrl|pmToRt|runtimes|checkRuntimeRequirement|checkAllRuntimes" packages/init/src/ask -g '*.ts'
echo
echo "## runtime definitions/usages"
rg -n "export const runtimes|const runtimes|getInstallUrl\s*=" packages/init/src/ask packages/init/src -g '*.ts' | head -200Repository: fedify-dev/fedify
Length of output: 1198
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## packages/init/src/lib.ts around definitions"
sed -n '1,180p' packages/init/src/lib.ts | cat -n
echo
echo "## packages/init/src/types.ts runtime/package manager shape"
sed -n '1,260p' packages/init/src/types.ts | cat -n
echo
echo "## packages/init/src/webframeworks/utils.ts relevant lines"
sed -n '1,180p' packages/init/src/webframeworks/utils.ts | cat -n
echo
echo "## runtime install mappings"
rg -n "install|version|version\\]" packages/init/src -g '*.ts' | head -200Repository: fedify-dev/fedify
Length of output: 29590
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## rt.json"
python3 - <<'PY'
import json
from pathlib import Path
p = Path("packages/init/src/json/rt.json")
data = json.loads(p.read_text())
print(json.dumps(data, indent=2))
PY
echo
echo "## pmToRt behavior"
python3 - <<'PY'
def pm_to_rt(pm):
return "node" if pm not in {"deno", "bun"} else pm
for pm in ["deno", "bun", "npm", "pnpm", "yarn"]:
print(f"{pm} -> {pm_to_rt(pm)}")
PYRepository: fedify-dev/fedify
Length of output: 779
Show an install link for missing runtimes.
The “missing runtime” path exits with only printErrorMessage and no actionable install guidance, unlike the package-manager-missing path. Use the runtime label via runtimes[pmToRt(pm)].label and print an install link, extending the installer metadata or adding a runtime helper for the unsupported getInstallUrl signature.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/init/src/ask/pm.ts` around lines 53 - 55, Update the result.status
=== "missing" branch to include actionable runtime installation guidance, using
runtimes[pmToRt(pm)].label and an install URL. Extend the installer metadata or
add a runtime helper to support the getInstallUrl signature needed for this
path, while preserving the existing missing-runtime error message.
2chanhaeng
requested changes
Aug 5, 2026
2chanhaeng
left a comment
2chanhaeng
left a comment
Member
There was a problem hiding this comment.
Great work! Thanks for your contribution to Fedify. Before merging this PR, some changes are needed, and I have left suggestions to make your PR better. Please read them and consider applying them!
Comment on lines
+97
to
+111
| const reason = !isWfSupportsPm(wf, value) | ||
| ? `not supported with ${webFrameworks[wf].label}` | ||
| : check.status === "unsupported" | ||
| ? `requires ${label} ${check.required} or later` | ||
| : check.status === "missing" | ||
| ? `requires ${label} which is not installed` | ||
| : check.status === "malformed" | ||
| ? `could not detect ${label} version` | ||
| : ""; | ||
| const disabled = !isWfSupportsPm(wf, value) || check.status !== "ok"; | ||
| return { | ||
| name: disabled ? `${value} (${reason})` : value, | ||
| value, | ||
| disabled, | ||
| }; |
Member
There was a problem hiding this comment.
It would be better const reason = /* ... */ : ""; return disabled === "" ? { name, value } : { name, value, disabled } because select from @inquirer/prompts show disabled if it is string.
`askPackageManager` recomputed the runtime checks through `checkAllRuntimes` every time it built the prompt. Following the review, that computation was separated into `calculateChoices` to run once. When `fillPackageManager` already has a `packageManager`, it now looks the value up in `choices` instead of calling `checkRuntimeRequirement` again. `checkRuntimeRequirement` is no longer used outside `lib.ts`, so its export was removed. Assisted-by: Claude Code:claude-opus-4-8
When the chosen package manager was not installed, `fillPackageManager` recursed to prompt again, recomputing `calculateChoices` on every retry, so replace the recursion with a loop that reuses the `choices` computed once at the start. Assisted-by: Claude Code:claude-opus-4-8
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CHANGES.md (1)
183-195: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the PR reference.
Line 183 escapes the closing citation bracket. Line 195 links PR
#981as an issue. Remove the escape and change the reference URL tohttps://github.com/fedify-dev/fedify/pull/981.Based on PR objectives: this change is PR
#981.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CHANGES.md` around lines 183 - 195, In CHANGES.md, correct the PR `#981` citation by removing the unnecessary escape from its closing bracket, and update the `#981` reference URL from the issues endpoint to the pull endpoint. Leave the other changelog references unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@CHANGES.md`:
- Around line 183-195: In CHANGES.md, correct the PR `#981` citation by removing
the unnecessary escape from its closing bracket, and update the `#981` reference
URL from the issues endpoint to the pull endpoint. Leave the other changelog
references unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 408742d3-7138-47d9-9b17-033403cf539d
📒 Files selected for processing (4)
CHANGES.mdchanges.d/init/verify-runtime-version.mdpackages/init/src/ask/pm.tspackages/init/src/lib.ts
Author
|
I applied changes. Could you take another look? |
Codecov Report❌ Patch coverage is
... and 2 files with indirect coverage changes 🚀 New features to boost your workflow:
|
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
2 participants
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
Added runtime version verification to fedify init to enhance runtime safety by pre-checking minimum requirements.
Related issue
fedify init#964Changes
minVersion) tojson/rt.json, derived aRUNTIMElist inconst.ts, and addedRuntime/Runtimes/RuntimeChecktypes.verifyRuntimeVersion,checkRuntimeVersion,resolveRequiredVersion,checkRuntimeRequirement, andcheckAllRuntimestolib.ts.minRuntimeVersionsfield toWebFrameworkDescriptionand set Astro's Node.js 22.12 requirement.ask/pm.tsto exit with an actionable error for an unsupported runtime in non-interactive mode, and to disable the affected package managers in the interactive menu.verifyRuntimeVersionandresolveRequiredVersion.AI disclosure
Claude Code (
claude-opus-4-8) assisted with this change: it proposed the design and drafted much of the implementation, tests, and this description. I reviewed and edited the work, made the design decisions, and verified the behaviour myself by runningfedify initand the test suite.