[#1951] Added a command to automatically create installer ascii video and PNG screenshot. #1963
Conversation
WalkthroughAdds tooling and docs to record and embed a Vortex installer asciinema: a script to run/build the installer and produce cast/SVG/PNG assets, GitHub Actions steps to invoke it, an Ahoy command, a React AsciinemaPlayer and tests/fixtures, updated docs pages/assets, and a docs .gitignore entry. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev/CI
participant Script as update-installer-video.sh
participant Tools as (asciinema / expect / php / npx)
participant Build as Installer build (composer)
participant Expect as Expect driver
participant Asci as asciinema
participant Convert as svg-term / sharp-cli
participant Docs as Docs assets
Dev/CI->>Script: invoke
Script->>Tools: check deps
alt prebuilt PHAR exists
Script->>Docs: copy installer.phar
else
Script->>Build: build installer PHAR
Build-->>Script: installer.phar
Script->>Docs: copy installer.phar
end
Script->>Expect: create expect script
Script->>Asci: record session (spawn php installer via expect)
Asci-->>Script: cast.json
Script->>Convert: convert cast -> svg + poster.png
Convert-->>Script: svg/png
Script->>Docs: move installer.json, installer.svg, installer.png
Script-->>Dev/CI: done
sequenceDiagram
autonumber
participant Browser
participant Page as installation.mdx
participant Player as AsciinemaPlayer
participant CDN as CDN (asciinema)
participant Asset as /img/installer.json + poster
Browser->>Page: request page
Page->>Player: hydrate client component
Player->>CDN: load CSS/JS (if missing)
Player->>Asset: fetch installer.json + poster
Asset-->>Player: data
Player-->>Browser: render interactive playback UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1963 +/- ##
===========================================
+ Coverage 77.57% 77.68% +0.11%
===========================================
Files 89 90 +1
Lines 5515 5549 +34
Branches 35 44 +9
===========================================
+ Hits 4278 4311 +33
- Misses 1237 1238 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.vortex/docs/content/contributing/maintenance/release.mdx (1)
17-19: Grammar: fix article/phrasing in step 11.-11. Create a new release notes using [release template](./release_template.md). +11. Create new release notes using the [release template](./release_template.md)..vortex/.ahoy.yml (1)
116-122: Entry point arguments are inconsistent (double “-c”, stray “-e”).As written, Ahoy will invoke:
bash -c -e "bash -e -c "$0" "$@"… which is error-prone and nests shells unnecessarily. Use a single Bash with-lcand internalset -e.entrypoint: - - bash - - "-c" - - "-e" - - bash -e -c "$0" "$@" - - '{{cmd}}' - - '{{name}}' + - bash + - -lc + - | + set -e + "$0" "$@" + - '{{cmd}}' + - '{{name}}'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
.vortex/docs/static/img/installer.pngis excluded by!**/*.png.vortex/docs/static/img/installer.svgis excluded by!**/*.svg
📒 Files selected for processing (11)
.github/workflows/vortex-release-docs.yml(1 hunks).github/workflows/vortex-test-installer.yml(2 hunks).vortex/.ahoy.yml(1 hunks).vortex/docs/.gitignore(1 hunks).vortex/docs/.utils/update-installer-video.sh(1 hunks).vortex/docs/content/contributing/maintenance/release.mdx(1 hunks).vortex/docs/content/getting-started/installation.mdx(2 hunks).vortex/docs/src/components/AsciinemaPlayer/AsciinemaPlayer.js(1 hunks).vortex/docs/src/components/AsciinemaPlayer/index.js(1 hunks).vortex/docs/static/img/installer.json(1 hunks)README.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
54-54: Inline HTML
Element: img
(MD033, no-inline-html)
🪛 GitHub Check: vortex-test-docs
.vortex/docs/src/components/AsciinemaPlayer/AsciinemaPlayer.js
[failure] 9-9:
Expected { after 'if' condition
🪛 GitHub Actions: Vortex - Test docs
.vortex/docs/src/components/AsciinemaPlayer/AsciinemaPlayer.js
[error] 9-9: ESLint: Expected '{' after 'if' condition. Command: 'eslint src/'
🪛 LanguageTool
.vortex/docs/content/contributing/maintenance/release.mdx
[grammar] ~18-~18: Use articles correctly
Context: ...hoy update-installer-video`. 11. Create a new release notes using [release templa...
(QB_NEW_EN_OTHER_ERROR_IDS_11)
[grammar] ~18-~18: There might be a mistake here.
Context: ...elease template](./release_template.md).
(QB_NEW_EN)
.vortex/docs/content/getting-started/installation.mdx
[grammar] ~5-~5: Use correct spacing
Context: ... '@site/src/components/AsciinemaPlayer'; # Installation The installation process c...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🪛 Biome (2.1.2)
.vortex/docs/static/img/installer.json
[error] 1-4: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 4-4: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 4-6: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 6-6: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 6-8: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 8-8: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 8-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-36: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 36-36: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 36-38: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 38-38: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 38-38: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 38-40: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 40-44: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 44-44: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 44-46: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 46-46: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 46-46: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 46-49: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 49-49: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 49-50: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 50-51: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 51-56: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 56-56: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 56-56: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 56-56: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 56-59: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 59-59: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 59-62: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 62-62: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 62-62: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 62-65: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 65-68: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 68-68: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 68-68: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 68-70: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 70-70: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 70-70: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 70-71: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 71-71: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 71-75: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 75-77: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 77-77: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 77-77: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 77-80: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 80-80: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 80-80: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 80-82: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 82-82: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 82-84: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 84-86: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 86-86: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 86-89: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 89-91: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 91-94: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 94-94: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 94-94: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 94-96: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 96-96: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 96-97: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 97-100: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 100-100: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 100-102: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 102-102: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 102-105: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 105-105: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 105-107: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 107-107: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 107-109: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 109-109: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 109-111: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 111-111: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 111-111: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 111-112: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 112-114: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 114-114: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 114-114: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 114-116: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 116-120: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 120-122: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 122-122: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 122-122: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 122-124: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 124-125: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 125-135: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 135-136: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 136-137: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 137-146: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 146-146: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 146-150: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 150-150: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 150-151: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 151-151: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.3)
🔇 Additional comments (5)
.vortex/docs/.utils/update-installer-video.sh (1)
185-190: Expect script uses 'sleep', which isn’t an Expect/Tcl builtin. Use 'after'.'sleep' will error; 'after' expects milliseconds.
Apply inside the heredoc:
- sleep $prompt_delay + after [expr {int($prompt_delay * 1000)}]Likely an incorrect or invalid review comment.
README.md (1)
54-55: Replace inline HTML with Markdown image (README.md Lines 54–55)
Asset.vortex/docs/static/img/installer.svgis present.-<img src=".vortex/docs/static/img/installer.svg" alt="Vortex installer screenshot" /> +.vortex/docs/src/components/AsciinemaPlayer/index.js (1)
1-1: LGTM.Barrel default export is correct.
.vortex/docs/content/getting-started/installation.mdx (1)
27-35: Improve URL handling and accessibility in installation guide
- Replace the hardcoded
src="/img/installer.json"with a base-URL–aware path, e.g.
src={useBaseUrl('/img/installer.cast')}(or confirm thatAsciinemaPlayeralready callsuseBaseUrlinternally).- Add an accessible label prop:
aria-label="Interactive Vortex installer walkthrough".- Verify renaming
installer.json→installer.castis intentional and aligns with issue #1951’s SVG vs. interactive-cast scope.
[vortex/docs/content/getting-started/installation.mdx].vortex/docs/static/img/installer.json (1)
1-152: Executed a local scan to confirm:
.vortex/docs/static/img/installer.jsonexists- No existing
.biomeignoreentry for it- References in MDX/HTML point at
/img/installer.jsonThis validates the recommendation to either rename the file or add a Biome ignore.
| - name: Setup Node.js | ||
| uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 | ||
| with: | ||
| node-version: 22.17.1 | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Prefer a moving LTS range for Node to reduce churn.
Keeps builds current without manual patch bumps.
- node-version: 22.17.1
+ node-version: '22.x'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Setup Node.js | |
| uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 | |
| with: | |
| node-version: 22.17.1 | |
| - name: Setup Node.js | |
| uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 | |
| with: | |
| node-version: '22.x' |
🤖 Prompt for AI Agents
.github/workflows/vortex-test-installer.yml around lines 33 to 37: the workflow
pins node-version to a specific patch (22.17.1) which forces frequent manual
updates; change the node-version input to a moving LTS range (for example "22.x"
or "lts/*") so the runner picks current security/patch releases automatically,
update the value in the actions/setup-node step accordingly and keep the rest of
the step unchanged.
| - name: Generate video for installer (not used in the final artifact) | ||
| run: | | ||
| sudo apt install asciinema expect | ||
| ./update-installer-video.sh | ||
| working-directory: .vortex/docs/.utils | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Same apt reliability fix as release workflow.
Prevent hangs and 404s; add timeout.
- name: Generate video for installer (not used in the final artifact)
run: |
- sudo apt install asciinema expect
- ./update-installer-video.sh
+ export DEBIAN_FRONTEND=noninteractive
+ sudo apt-get update -y
+ sudo apt-get install -y --no-install-recommends asciinema expect
+ timeout 15m bash ./update-installer-video.sh
working-directory: .vortex/docs/.utils📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Generate video for installer (not used in the final artifact) | |
| run: | | |
| sudo apt install asciinema expect | |
| ./update-installer-video.sh | |
| working-directory: .vortex/docs/.utils | |
| - name: Generate video for installer (not used in the final artifact) | |
| run: | | |
| export DEBIAN_FRONTEND=noninteractive | |
| sudo apt-get update -y | |
| sudo apt-get install -y --no-install-recommends asciinema expect | |
| timeout 15m bash ./update-installer-video.sh | |
| working-directory: .vortex/docs/.utils |
🤖 Prompt for AI Agents
.github/workflows/vortex-test-installer.yml around lines 59-64: the apt install
step can hang or hit 404s and the script can run forever; make the install
deterministic and add timeouts by running apt-get update first, set
DEBIAN_FRONTEND=noninteractive, use apt-get install -y --no-install-recommends
with Acquire::Retries (e.g. -o Acquire::Retries=3) to reduce transient failures,
and run the update-installer-video.sh under a timeout (e.g. timeout 5m) so the
workflow won't hang indefinitely.
| set -eu | ||
| [ "${VORTEX_DEBUG-}" = "1" ] && set -x | ||
|
|
There was a problem hiding this comment.
Dangerous default WORKSPACE_DIR + unconditional rm -rf can wipe user data. Use mktemp and guarded cleanup.
Current default points to $HOME/www/demo and is always deleted at exit.
Apply:
-set -eu
+set -euo pipefail
[ "${VORTEX_DEBUG-}" = "1" ] && set -x
@@
-# Directory for temporary workspace.
-WORKSPACE_DIR="${WORKSPACE_DIR:-$HOME/www/demo}"
+# Directory for temporary workspace (set in main; may be overridden via env).
+# WORKSPACE_DIR is intentionally not defaulted here to avoid deleting user dirs.- WORKSPACE_DIR="${WORKSPACE_DIR:-"$(mktemp -d -t vortex-installer-recording-XXXXXX)"}"
- [ ! -d "$WORKSPACE_DIR" ] && mkdir -p "$WORKSPACE_DIR"
+ local CREATED_TMPDIR=""
+ if [[ -z "${WORKSPACE_DIR:-}" ]]; then
+ CREATED_TMPDIR="$(mktemp -d -t vortex-installer-recording-XXXXXX)"
+ WORKSPACE_DIR="$CREATED_TMPDIR"
+ else
+ mkdir -p "$WORKSPACE_DIR"
+ fi- cleanup() {
- info "Cleaning up workspace directory"
- note "Removing: ${WORKSPACE_DIR}"
- rm -rf "$WORKSPACE_DIR"
- }
+ cleanup() {
+ # Only remove if we created it in this run
+ if [[ -n "$CREATED_TMPDIR" && "$WORKSPACE_DIR" == "$CREATED_TMPDIR" ]]; then
+ info "Cleaning up workspace directory"
+ note "Removing: ${WORKSPACE_DIR}"
+ rm -rf "$WORKSPACE_DIR"
+ fi
+ }Also applies to: 18-18, 376-381, 386-391
🤖 Prompt for AI Agents
In .vortex/docs/.utils/update-installer-video.sh around lines 12-14 (and also
adjust the same pattern at 18, 376-381, 386-391): the script currently defaults
WORKSPACE_DIR to a persistent path ($HOME/www/demo) and unconditionally rm -rf's
it at exit, which can delete user data; change to create a temporary workspace
with mktemp when no WORKSPACE_DIR is provided, track whether the script created
the dir (e.g., a boolean flag), and only perform cleanup (rm -rf) for that temp
directory in the trap; if WORKSPACE_DIR was provided by the user, do not delete
it; also add defensive checks before deletion to ensure the target is not empty
or a root-like path (e.g., ensure it starts with a temp-dir prefix or is under
/tmp) to prevent accidental catastrophic deletes.
| sidebar_position: 3 | ||
| --- | ||
|
|
||
| import AsciinemaPlayer from '@site/src/components/AsciinemaPlayer'; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Ensure client-only rendering to avoid SSR hydration issues.
If AsciinemaPlayer touches window/DOM during render, wrap usage with Docusaurus BrowserOnly or make the component client-only. Otherwise, dev/build may warn or break during SSR.
Example (MDX):
+import BrowserOnly from '@docusaurus/BrowserOnly';
import AsciinemaPlayer from '@site/src/components/AsciinemaPlayer';And around the player:
-<AsciinemaPlayer ... />
+<BrowserOnly>{() => <AsciinemaPlayer ... />}</BrowserOnly>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import AsciinemaPlayer from '@site/src/components/AsciinemaPlayer'; | |
| import BrowserOnly from '@docusaurus/BrowserOnly'; | |
| import AsciinemaPlayer from '@site/src/components/AsciinemaPlayer'; | |
| {/* …later in your MDX content… */} | |
| <BrowserOnly> | |
| {() => <AsciinemaPlayer /* props here */ />} | |
| </BrowserOnly> |
🧰 Tools
🪛 LanguageTool
[grammar] ~5-~5: Use correct spacing
Context: ... '@site/src/components/AsciinemaPlayer'; # Installation The installation process c...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🤖 Prompt for AI Agents
.vortex/docs/content/getting-started/installation.mdx line 5: the
AsciinemaPlayer is imported at module scope which can cause SSR hydration errors
if it accesses window/DOM; to fix, make the player client-only by either
removing the top-level import and wrapping its usage with Docusaurus BrowserOnly
(import BrowserOnly from '@docusaurus/BrowserOnly' and render the
AsciinemaPlayer inside BrowserOnly) or change AsciinemaPlayer to a client-only
component (lazy/dynamic import or guard for window in its render) so no
DOM/window is accessed during SSR.
| if (!window.AsciinemaPlayer) { | ||
| const script = document.createElement('script'); | ||
| script.src = 'https://cdn.jsdelivr.net/npm/asciinema-player@3/dist/bundle/asciinema-player.min.js'; | ||
| script.onload = () => { | ||
| if (containerRef.current && window.AsciinemaPlayer) { | ||
| const options = { | ||
| autoPlay, | ||
| loop, | ||
| preload, | ||
| controls, | ||
| theme | ||
| }; | ||
|
|
||
| // Add poster if provided | ||
| if (poster) { | ||
| options.poster = poster; | ||
| } | ||
|
|
||
| // Add startAt if provided (for poster frame selection) | ||
| if (startAt !== undefined) { | ||
| options.startAt = startAt; | ||
| } | ||
|
|
||
| window.AsciinemaPlayer.create(src, containerRef.current, options); | ||
| } | ||
| }; | ||
| document.head.appendChild(script); | ||
| } else { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden dynamic loader: avoid races and surface load errors.
Multiple mounts can inject multiple scripts if the first hasn’t finished loading yet. Also, no onerror handler.
Apply:
- if (!window.AsciinemaPlayer) {
+ if (!window.AsciinemaPlayer) {
+ // Reuse existing pending script if present
+ const existing = document.querySelector('script[src*="asciinema-player.min.js"]');
+ if (existing) {
+ existing.addEventListener('load', () => {
+ if (containerRef.current && window.AsciinemaPlayer) {
+ const options = { autoPlay, loop, preload, controls, theme, ...(poster ? { poster } : {}), ...(startAt !== undefined ? { startAt } : {}) };
+ containerRef.current.innerHTML = '';
+ window.AsciinemaPlayer.create(src, containerRef.current, options);
+ }
+ }, { once: true });
+ return;
+ }
const script = document.createElement('script');
script.src = 'https://cdn.jsdelivr.net/npm/asciinema-player@3/dist/bundle/asciinema-player.min.js';
+ script.async = true;
script.onload = () => {
if (containerRef.current && window.AsciinemaPlayer) {
const options = {
autoPlay,
loop,
preload,
controls,
theme
};
// Add poster if provided
if (poster) {
options.poster = poster;
}
// Add startAt if provided (for poster frame selection)
if (startAt !== undefined) {
options.startAt = startAt;
}
- window.AsciinemaPlayer.create(src, containerRef.current, options);
+ containerRef.current.innerHTML = '';
+ window.AsciinemaPlayer.create(src, containerRef.current, options);
}
};
+ script.onerror = () => console.error('Failed to load Asciinema player script');
document.head.appendChild(script);
} else {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!window.AsciinemaPlayer) { | |
| const script = document.createElement('script'); | |
| script.src = 'https://cdn.jsdelivr.net/npm/asciinema-player@3/dist/bundle/asciinema-player.min.js'; | |
| script.onload = () => { | |
| if (containerRef.current && window.AsciinemaPlayer) { | |
| const options = { | |
| autoPlay, | |
| loop, | |
| preload, | |
| controls, | |
| theme | |
| }; | |
| // Add poster if provided | |
| if (poster) { | |
| options.poster = poster; | |
| } | |
| // Add startAt if provided (for poster frame selection) | |
| if (startAt !== undefined) { | |
| options.startAt = startAt; | |
| } | |
| window.AsciinemaPlayer.create(src, containerRef.current, options); | |
| } | |
| }; | |
| document.head.appendChild(script); | |
| } else { | |
| if (!window.AsciinemaPlayer) { | |
| // Reuse existing pending script if present | |
| const existing = document.querySelector('script[src*="asciinema-player.min.js"]'); | |
| if (existing) { | |
| existing.addEventListener('load', () => { | |
| if (containerRef.current && window.AsciinemaPlayer) { | |
| const options = { autoPlay, loop, preload, controls, theme, ...(poster ? { poster } : {}), ...(startAt !== undefined ? { startAt } : {}) }; | |
| containerRef.current.innerHTML = ''; | |
| window.AsciinemaPlayer.create(src, containerRef.current, options); | |
| } | |
| }, { once: true }); | |
| return; | |
| } | |
| const script = document.createElement('script'); | |
| script.src = 'https://cdn.jsdelivr.net/npm/asciinema-player@3/dist/bundle/asciinema-player.min.js'; | |
| script.async = true; | |
| script.onload = () => { | |
| if (containerRef.current && window.AsciinemaPlayer) { | |
| const options = { | |
| autoPlay, | |
| loop, | |
| preload, | |
| controls, | |
| theme | |
| }; | |
| // Add poster if provided | |
| if (poster) { | |
| options.poster = poster; | |
| } | |
| // Add startAt if provided (for poster frame selection) | |
| if (startAt !== undefined) { | |
| options.startAt = startAt; | |
| } | |
| containerRef.current.innerHTML = ''; | |
| window.AsciinemaPlayer.create(src, containerRef.current, options); | |
| } | |
| }; | |
| script.onerror = () => console.error('Failed to load Asciinema player script'); | |
| document.head.appendChild(script); | |
| } else { |
🤖 Prompt for AI Agents
.vortex/docs/src/components/AsciinemaPlayer/AsciinemaPlayer.js around lines
21-48: The dynamic loader can race when multiple mounts run before the script
loads and currently has no onerror handling; change the loader to first look for
an existing script element (by the exact src or a unique id) and if found
attach/await its existing load/error handlers (or reuse a loader Promise stored
on window, e.g. window.__asciinemaLoaderPromise) instead of injecting a
duplicate script; when creating a new script set both onload and onerror
handlers (surface error via console.error or reject the loader Promise) and
store the Promise on window so subsequent mounts reuse it, ensuring only one
script is injected and load failures are surfaced to the component.
| const options = { | ||
| autoPlay, | ||
| loop, | ||
| preload, | ||
| controls, | ||
| theme | ||
| }; | ||
|
|
||
| // Add poster if provided | ||
| if (poster) { | ||
| options.poster = poster; | ||
| } | ||
|
|
||
| // Add startAt if provided (for poster frame selection) | ||
| if (startAt !== undefined) { | ||
| options.startAt = startAt; | ||
| } | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Minor: reduce repetition building options.
Build options once to avoid drift.
Apply:
- const options = {
- autoPlay,
- loop,
- preload,
- controls,
- theme
- };
- // Add poster if provided
- if (poster) {
- options.poster = poster;
- }
- // Add startAt if provided (for poster frame selection)
- if (startAt !== undefined) {
- options.startAt = startAt;
- }
+ const options = { autoPlay, loop, preload, controls, theme, ...(poster ? { poster } : {}), ...(startAt !== undefined ? { startAt } : {}) };(and likewise in the already-loaded branch)
Also applies to: 51-68
🤖 Prompt for AI Agents
In .vortex/docs/src/components/AsciinemaPlayer/AsciinemaPlayer.js around lines
26-43 (and similarly update lines 51-68), the options object is built with
repeated conditional assignments (poster, startAt) which risks drift; instead
construct the options once using a single object literal that conditionally
includes poster and startAt (e.g., spread or computed properties) so both
branches use the same concise creation pattern and avoid duplication; apply the
same refactor to the already-loaded branch at lines 51-68.
| window.AsciinemaPlayer.create(src, containerRef.current, options); | ||
| } | ||
| }; | ||
| document.head.appendChild(script); | ||
| } else { | ||
| // AsciinemaPlayer is already loaded | ||
| if (containerRef.current && window.AsciinemaPlayer) { | ||
| const options = { | ||
| autoPlay, | ||
| loop, | ||
| preload, | ||
| controls, | ||
| theme | ||
| }; | ||
|
|
||
| // Add poster if provided | ||
| if (poster) { | ||
| options.poster = poster; | ||
| } | ||
|
|
||
| // Add startAt if provided (for poster frame selection) | ||
| if (startAt !== undefined) { | ||
| options.startAt = startAt; | ||
| } | ||
|
|
||
| window.AsciinemaPlayer.create(src, containerRef.current, options); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent duplicate players and leaks by clearing the container and adding effect cleanup.
Re-creates the player on every prop change without tearing down the previous instance; this can stack DOM nodes. Clear before create and on unmount.
Apply:
- window.AsciinemaPlayer.create(src, containerRef.current, options);
+ // Clear any previous render before creating a new player
+ containerRef.current.innerHTML = '';
+ window.AsciinemaPlayer.create(src, containerRef.current, options);- window.AsciinemaPlayer.create(src, containerRef.current, options);
+ containerRef.current.innerHTML = '';
+ window.AsciinemaPlayer.create(src, containerRef.current, options);- loadAsciinemaPlayer();
- }, [src, poster, startAt, autoPlay, loop, preload, controls, theme]);
+ loadAsciinemaPlayer();
+ return () => {
+ if (containerRef.current) containerRef.current.innerHTML = '';
+ };
+ }, [src, poster, startAt, autoPlay, loop, preload, controls, theme]);Also applies to: 77-79
AlexSkrypnyk
force-pushed
the
feature/1951-ascii-cinema
branch
from
60339c5 to
1070c96
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.vortex/docs/content/contributing/maintenance/release.mdx (1)
17-19: Grammar: “Create new release notes”.-11. Create a new release notes using [release template](./release_template.md). +11. Create new release notes using [release template](./release_template.md)..vortex/docs/content/getting-started/installation.mdx (2)
13-15: Fix typo in user-facing docs ("in interactive" → "an interactive").User-facing grammar issue.
-We automated the first part by providing an in interactive installer. The second +We automated the first part by providing an interactive installer. The second
97-100: Remove double space before “and”.Minor readability polish.
-Depending on the services you are using, you will need to set up the -integrations with your hosting provider, continuous integration provider, and +Depending on the services you are using, you will need to set up the +integrations with your hosting provider, continuous integration provider, and
♻️ Duplicate comments (18)
.vortex/docs/.gitignore (1)
24-25: Clarify intent: is static/install a file or a directory?Without a trailing slash it matches both; be explicit to avoid surprises.
If it’s a directory:
-static/install +static/install/If it’s a single copied file (per the comment), consider a clearer name:
-# Copied installer file. -static/install +# Copied installer binary. +static/installerUpdate any scripts accordingly if you rename.
.vortex/docs/src/components/AsciinemaPlayer/AsciinemaPlayer.js (2)
91-93: Add effect cleanup to avoid leaks on unmount.Clear container to release DOM and listeners.
- loadAsciinemaPlayer(); - }, [src, poster, startAt, autoPlay, loop, preload, controls, theme]); + loadAsciinemaPlayer(); + return () => { + if (containerRef.current) containerRef.current.innerHTML = ''; + }; + }, [src, poster, startAt, autoPlay, loop, preload, controls, theme]);
34-61: Dedupe dynamic script injection and add error handling.Multiple mounts before load can append duplicate scripts; no onerror handler; script not async.
Apply:
- // Dynamically load the JavaScript - if (!window.AsciinemaPlayer) { - const script = document.createElement('script'); - script.src = - 'https://cdn.jsdelivr.net/npm/asciinema-player@3/dist/bundle/asciinema-player.min.js'; - script.onload = () => { - if (containerRef.current && window.AsciinemaPlayer) { - const options = { - autoPlay, - loop, - preload, - controls, - theme, - }; - - // Add poster if provided - if (poster) { - options.poster = poster; - } - - // Add startAt if provided (for poster frame selection) - if (startAt !== undefined) { - options.startAt = startAt; - } - - window.AsciinemaPlayer.create(src, containerRef.current, options); - } - }; - document.head.appendChild(script); + // Dynamically load the JavaScript + if (!window.AsciinemaPlayer) { + // Reuse a pending script if present to avoid duplicates. + const existing = document.getElementById('asciinema-player-cdn'); + if (existing) { + existing.addEventListener('load', () => { + if (containerRef.current && window.AsciinemaPlayer) { + const options = { autoPlay, loop, preload, controls, theme, ...(poster ? { poster } : {}), ...(startAt !== undefined ? { startAt } : {}) }; + containerRef.current.innerHTML = ''; + window.AsciinemaPlayer.create(src, containerRef.current, options); + } + }, { once: true }); + existing.addEventListener('error', () => console.error('Failed to load Asciinema player script'), { once: true }); + return; + } + const script = document.createElement('script'); + script.id = 'asciinema-player-cdn'; + script.async = true; + script.src = 'https://cdn.jsdelivr.net/npm/asciinema-player@3/dist/bundle/asciinema-player.min.js'; + script.onload = () => { + if (containerRef.current && window.AsciinemaPlayer) { + const options = { autoPlay, loop, preload, controls, theme, ...(poster ? { poster } : {}), ...(startAt !== undefined ? { startAt } : {}) }; + containerRef.current.innerHTML = ''; + window.AsciinemaPlayer.create(src, containerRef.current, options); + } + }; + script.onerror = () => console.error('Failed to load Asciinema player script'); + document.head.appendChild(script);.vortex/docs/.utils/update-installer-video.sh (9)
12-18: Harden shell options and remove dangerous default WORKSPACE_DIR.-set -eu +set -euo pipefail @@ -# Directory for temporary workspace. -WORKSPACE_DIR="${WORKSPACE_DIR:-$HOME/www/demo}" +# Directory for temporary workspace (set in main; may be overridden via env). +# WORKSPACE_DIR is not defaulted here to avoid deleting user dirs accidentally.
50-69: Add node to deps and provide install hints; keep CI non-interactive.- local required_commands=("asciinema" "expect" "php" "composer" "npx") + local required_commands=("asciinema" "expect" "php" "composer" "node" "npx") @@ - echo " brew install asciinema expect composer # macOS" >&2 - echo " apt-get install asciinema expect-dev composer # Ubuntu/Debian" >&2 + echo " brew install asciinema expect composer node # macOS" >&2 + echo " apt-get update -y && apt-get install -y --no-install-recommends asciinema expect-dev composer nodejs npm # Ubuntu/Debian" >&2
76-83: Ensure target dir exists before copying installer.- cp "$INSTALLER_BUILD" "$INSTALLER_DOCS" + mkdir -p "$(dirname "$INSTALLER_DOCS")" + cp "$INSTALLER_BUILD" "$INSTALLER_DOCS"
95-110: Make composer commands CI-safe (non-interactive, faster).- if ! composer install; then + if ! composer install --no-interaction --prefer-dist --no-progress --no-ansi; then @@ - if ! composer build; then + if ! composer build -n; then
113-120: Ensure target dir exists before copying built installer.- cp "$INSTALLER_BUILD" "$INSTALLER_DOCS" + mkdir -p "$(dirname "$INSTALLER_DOCS")" + cp "$INSTALLER_BUILD" "$INSTALLER_DOCS"
286-291: Guard copying installer into workspace.- cp "$INSTALLER_DOCS" "$workspace_dir/installer.php" + if [ ! -f "$INSTALLER_DOCS" ]; then + fail "Installer not found at: $INSTALLER_DOCS" + return 1 + fi + cp "$INSTALLER_DOCS" "$workspace_dir/installer.php"
317-319: Broaden path anonymization and keep sed portable on macOS.- sed_opts=(-i) && [ "$(uname)" = "Darwin" ] && sed_opts=(-i '') - sed "${sed_opts[@]}" -E 's|/Users/[^/]+/|/home/user/|g' "$output_cast" + sed_opts=(-i); [ "$(uname)" = "Darwin" ] && sed_opts=(-i '') + sed "${sed_opts[@]}" -E 's|/(Users|home)/[^/]+/|/home/user/|g' "$output_cast"
323-341: Make all npx calls non-interactive (-y) to avoid CI prompts.- if ! npx svg-term --out="$output_svg" <"$output_cast"; then + if ! npx -y svg-term --out="$output_svg" <"$output_cast"; then @@ - if ! npx svg-term --at=1000 --out="$output_png.svg" <"$output_cast"; then + if ! npx -y svg-term --at=1000 --out="$output_png.svg" <"$output_cast"; then @@ - if ! npx sharp-cli -i "$output_png.svg" -o "$output_png" -f png resize 1280; then + if ! npx -y sharp-cli -i "$output_png.svg" -o "$output_png" -f png resize 1280; then
376-391: Create temp workspace safely; only delete if we created it.- info "Creating temporary workspace" - WORKSPACE_DIR="${WORKSPACE_DIR:-"$(mktemp -d -t vortex-installer-recording-XXXXXX)"}" - [ ! -d "$WORKSPACE_DIR" ] && mkdir -p "$WORKSPACE_DIR" + info "Creating temporary workspace" + local CREATED_TMPDIR="" + if [[ -z "${WORKSPACE_DIR:-}" ]]; then + CREATED_TMPDIR="$(mktemp -d -t vortex-installer-recording-XXXXXX)" + WORKSPACE_DIR="$CREATED_TMPDIR" + else + mkdir -p "$WORKSPACE_DIR" + fi @@ - cleanup() { - info "Cleaning up workspace directory" - note "Removing: ${WORKSPACE_DIR}" - rm -rf "$WORKSPACE_DIR" - } + cleanup() { + if [[ -n "$CREATED_TMPDIR" && "$WORKSPACE_DIR" == "$CREATED_TMPDIR" && -n "$WORKSPACE_DIR" && "$WORKSPACE_DIR" == /tmp/* ]]; then + info "Cleaning up workspace directory" + note "Removing: ${WORKSPACE_DIR}" + rm -rf "$WORKSPACE_DIR" + fi + }.github/workflows/vortex-test-installer.yml (2)
33-37: Prefer moving LTS range for Node to reduce patch churn.- node-version: 22.17.1 + node-version: '22.x'
59-64: Harden apt install and bound script runtime to avoid hangs.- - name: Generate video for installer (not used in the final artifact) - run: | - sudo apt install asciinema expect - ./update-installer-video.sh + - name: Generate video for installer (not used in the final artifact) + run: | + export DEBIAN_FRONTEND=noninteractive + sudo apt-get update -y + sudo apt-get install -y --no-install-recommends asciinema expect + timeout 15m bash ./update-installer-video.sh working-directory: .vortex/docs/.utils.github/workflows/vortex-release-docs.yml (1)
135-140: Make apt non-interactive and guard the long-running script; optionally upload assets.Use apt-get with update/-y and --no-install-recommends, set DEBIAN_FRONTEND to avoid prompts, and run the script under a timeout to prevent hangs. Optionally, upload generated assets for debugging.
- name: Generate video for installer (not used in the final artifact) run: | - sudo apt install asciinema expect - ./update-installer-video.sh + export DEBIAN_FRONTEND=noninteractive + sudo apt-get update -y + sudo apt-get install -y --no-install-recommends asciinema expect + timeout 15m bash ./update-installer-video.sh working-directory: .vortex/docs/.utils + + - name: Upload installer video assets (debug) + if: ${{ always() }} + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 + with: + name: installer-video-assets + path: | + .vortex/docs/static/img/installer.json + .vortex/docs/static/img/installer.svg + .vortex/docs/static/img/installer.png.vortex/docs/content/getting-started/installation.mdx (2)
5-5: Wrap client-only player with BrowserOnly to avoid SSR hydration issues.Import BrowserOnly and render the player inside it.
+import BrowserOnly from '@docusaurus/BrowserOnly'; import AsciinemaPlayer from '@site/src/components/AsciinemaPlayer';
27-35: Render player inside BrowserOnly.- <AsciinemaPlayer - src="/img/installer.json" - poster="npt:0:1" - autoPlay={false} - loop={false} - preload={true} - controls={true} - theme="asciinema" - /> + <BrowserOnly> + {() => ( + <AsciinemaPlayer + src="/img/installer.json" + poster="npt:0:1" + autoPlay={false} + loop={false} + preload={true} + controls={true} + theme="asciinema" + /> + )} + </BrowserOnly>.vortex/.ahoy.yml (1)
110-115: Harden the command wrapper and avoid fragile relative paths.Fail fast, use pipefail, resolve repo root, and run via absolute path.
update-installer-video: usage: Update installer video in the documentation. cmd: | - cd docs/.utils - ./update-installer-video.sh + set -euo pipefail + REPO_ROOT="$(git rev-parse --show-toplevel)" + cd "${REPO_ROOT}/docs/.utils" + bash ./update-installer-video.sh
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
.vortex/docs/static/img/installer.pngis excluded by!**/*.png.vortex/docs/static/img/installer.svgis excluded by!**/*.svg
📒 Files selected for processing (13)
.github/workflows/vortex-release-docs.yml(1 hunks).github/workflows/vortex-test-installer.yml(2 hunks).vortex/.ahoy.yml(1 hunks).vortex/docs/.gitignore(1 hunks).vortex/docs/.utils/update-installer-video.sh(1 hunks).vortex/docs/content/contributing/maintenance/release.mdx(1 hunks).vortex/docs/content/getting-started/installation.mdx(2 hunks).vortex/docs/content/tools/drush.mdx(1 hunks).vortex/docs/content/workflows/deployment.mdx(1 hunks).vortex/docs/src/components/AsciinemaPlayer/AsciinemaPlayer.js(1 hunks).vortex/docs/src/components/AsciinemaPlayer/index.js(1 hunks).vortex/docs/static/img/installer.json(1 hunks)README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-29T12:15:32.188Z
Learnt from: AlexSkrypnyk
PR: drevops/vortex#0
File: :0-0
Timestamp: 2025-05-29T12:15:32.188Z
Learning: Do not review files in `.vortex/installer/tests/Fixtures/install` directory as they are test fixtures.
Applied to files:
.vortex/docs/.gitignore
🪛 Biome (2.1.2)
.vortex/docs/static/img/installer.json
[error] 1-4: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 4-4: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 4-6: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 6-6: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 6-8: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 8-8: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 8-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-36: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 36-36: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 36-38: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 38-38: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 38-38: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 38-40: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 40-44: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 44-44: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 44-46: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 46-46: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 46-46: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 46-49: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 49-49: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 49-50: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 50-51: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 51-56: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 56-56: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 56-56: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 56-56: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 56-59: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 59-59: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 59-62: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 62-62: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 62-62: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 62-65: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 65-68: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 68-68: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 68-68: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 68-70: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 70-70: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 70-70: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 70-71: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 71-71: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 71-75: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 75-77: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 77-77: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 77-77: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 77-80: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 80-80: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 80-80: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 80-82: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 82-82: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 82-84: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 84-86: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 86-86: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 86-89: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 89-91: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 91-94: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 94-94: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 94-94: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 94-96: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 96-96: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 96-97: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 97-100: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 100-100: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 100-102: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 102-102: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 102-105: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 105-105: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 105-107: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 107-107: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 107-109: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 109-109: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 109-111: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 111-111: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 111-111: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 111-112: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 112-114: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 114-114: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 114-114: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 114-116: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 116-120: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 120-122: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 122-122: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 122-122: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 122-124: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 124-125: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 125-135: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 135-136: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 136-137: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 137-146: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 146-146: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 146-150: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 150-150: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 150-151: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 151-151: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🪛 LanguageTool
.vortex/docs/content/getting-started/installation.mdx
[grammar] ~5-~5: Use correct spacing
Context: ... '@site/src/components/AsciinemaPlayer'; # Installation The installation process c...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
.vortex/docs/content/contributing/maintenance/release.mdx
[grammar] ~18-~18: Use articles correctly
Context: ...hoy update-installer-video`. 11. Create a new release notes using [release templa...
(QB_NEW_EN_OTHER_ERROR_IDS_11)
[grammar] ~18-~18: There might be a mistake here.
Context: ...elease template](./release_template.md).
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
README.md
54-54: Inline HTML
Element: img
(MD033, no-inline-html)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: build (0)
- GitHub Check: build (1)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.2)
🔇 Additional comments (5)
.vortex/docs/content/workflows/deployment.mdx (1)
32-32: LGTM: spacing tweak is fine.Blank line correctly separates the paragraph from the list; no content change.
.vortex/docs/content/tools/drush.mdx (1)
55-55: LGTM: improves Markdown rendering before the code block.The extra blank line prevents accidental paragraph/code fusion.
README.md (1)
54-55: Replace inline HTML with Markdown image syntax in README.md (lines 54–55)-<img src=".vortex/docs/static/img/installer.svg" alt="Vortex installer screenshot" /> +Asset path
.vortex/docs/static/img/installer.svgverified..vortex/docs/src/components/AsciinemaPlayer/AsciinemaPlayer.js (1)
7-11: No changes required for preload and controls Verified thatpreloadandcontrolsare valid Asciinema Player v3 configuration options..vortex/docs/src/components/AsciinemaPlayer/index.js (1)
1-1: LGTM — concise barrel export.
| proc wait_and_enter {} { | ||
| global prompt_delay | ||
| wait_for_quiet 0.25 | ||
| sleep \$prompt_delay | ||
| safe_send "\r" | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use Expect/Tcl ‘after’ instead of ‘sleep’ for portability.
- sleep $prompt_delay
+ after [expr {int($prompt_delay * 1000)}]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| proc wait_and_enter {} { | |
| global prompt_delay | |
| wait_for_quiet 0.25 | |
| sleep \$prompt_delay | |
| safe_send "\r" | |
| } | |
| proc wait_and_enter {} { | |
| global prompt_delay | |
| wait_for_quiet 0.25 | |
| # use Expect/Tcl’s built-in after (ms) instead of sleep for portability | |
| after [expr {int($prompt_delay * 1000)}] | |
| safe_send "\r" | |
| } |
🤖 Prompt for AI Agents
In .vortex/docs/.utils/update-installer-video.sh around lines 185 to 190, the
function wait_and_enter uses sleep \$prompt_delay which is not portable in
Expect/Tcl; replace the blocking sleep call with after to schedule the delay in
milliseconds (e.g. after [expr {$prompt_delay * 1000}]) and then perform
safe_send \"\r\" so the script uses Tcl's event loop rather than POSIX sleep;
ensure prompt_delay is treated as seconds (convert to ms) and remove the sleep
line.
| # Copy the processed files. | ||
| info "Copying processed files" | ||
| note "JSON cast: $temp_cast → $CAST_FILE" | ||
| cp "$temp_cast" "$CAST_FILE" | ||
| note "SVG image: $temp_svg → $SVG_FILE" | ||
| cp "$temp_svg" "$SVG_FILE" | ||
| note "PNG image: $temp_png → $PNG_FILE" | ||
| cp "$temp_png" "$PNG_FILE" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure destination dirs exist before copying outputs.
- cp "$temp_cast" "$CAST_FILE"
+ mkdir -p "$(dirname "$CAST_FILE")"
+ cp "$temp_cast" "$CAST_FILE"
@@
- cp "$temp_svg" "$SVG_FILE"
+ mkdir -p "$(dirname "$SVG_FILE")"
+ cp "$temp_svg" "$SVG_FILE"
@@
- cp "$temp_png" "$PNG_FILE"
+ mkdir -p "$(dirname "$PNG_FILE")"
+ cp "$temp_png" "$PNG_FILE"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Copy the processed files. | |
| info "Copying processed files" | |
| note "JSON cast: $temp_cast → $CAST_FILE" | |
| cp "$temp_cast" "$CAST_FILE" | |
| note "SVG image: $temp_svg → $SVG_FILE" | |
| cp "$temp_svg" "$SVG_FILE" | |
| note "PNG image: $temp_png → $PNG_FILE" | |
| cp "$temp_png" "$PNG_FILE" | |
| # Copy the processed files. | |
| info "Copying processed files" | |
| note "JSON cast: $temp_cast → $CAST_FILE" | |
| mkdir -p "$(dirname "$CAST_FILE")" | |
| cp "$temp_cast" "$CAST_FILE" | |
| note "SVG image: $temp_svg → $SVG_FILE" | |
| mkdir -p "$(dirname "$SVG_FILE")" | |
| cp "$temp_svg" "$SVG_FILE" | |
| note "PNG image: $temp_png → $PNG_FILE" | |
| mkdir -p "$(dirname "$PNG_FILE")" | |
| cp "$temp_png" "$PNG_FILE" |
🤖 Prompt for AI Agents
In .vortex/docs/.utils/update-installer-video.sh around lines 424 to 432, the
script copies temp files to their destination paths but does not ensure the
destination directories exist; before each cp call, create the parent directory
for $CAST_FILE, $SVG_FILE and $PNG_FILE (e.g. use mkdir -p "$(dirname
"$CAST_FILE")" etc.), then perform the copy and propagate failures (exit
non‑zero or check cp return) so missing dirs won't cause silent failures.
| const link = document.createElement('link'); | ||
| link.rel = 'stylesheet'; | ||
| link.href = | ||
| 'https://cdn.jsdelivr.net/npm/asciinema-player@3/dist/bundle/asciinema-player.css'; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Pin CDN version to a specific patch for reproducibility.
Floating @3 can change behavior unexpectedly; pin to an exact version or vend via npm.
Would you like me to update both URLs to the latest stable (e.g., @3.x.y) after confirming the current version?
Also applies to: 37-37
🌐 Web query:
What is the latest stable version of asciinema-player (v3) on jsDelivr/NPM?
💡 Result:
The latest stable asciinema-player v3 is 3.10.0. [1][2][3]
Sources:
- npm package page (shows v3.10.0). [1]
- jsDelivr package page (version 3.10.0). [2]
- asciinema announcement (v3.10 release). [3]
Pin CDN URLs to asciinema-player v3.10.0
The latest stable v3 is 3.10.0; update both CDN imports in .vortex/docs/src/components/AsciinemaPlayer/AsciinemaPlayer.js (lines 29 & 37) from @3 to @3.10.0.
🤖 Prompt for AI Agents
.vortex/docs/src/components/AsciinemaPlayer/AsciinemaPlayer.js around lines 29
and 37: the CDN imports currently use the open major tag `@3`; update both
import URLs to pin the exact stable version `@3.10.0` instead of `@3` so the
file imports the specific asciinema-player v3.10.0 assets; ensure both
occurrences (line 29 and line 37) are changed to use `@3.10.0`.
| const options = { | ||
| autoPlay, | ||
| loop, | ||
| preload, | ||
| controls, | ||
| theme, | ||
| }; | ||
|
|
||
| // Add poster if provided | ||
| if (poster) { | ||
| options.poster = poster; | ||
| } | ||
|
|
||
| // Add startAt if provided (for poster frame selection) | ||
| if (startAt !== undefined) { | ||
| options.startAt = startAt; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Deduplicate option-building.
Build the options object once with conditional spreads to avoid drift.
Apply (covered in previous diff for first branch); do similarly here:
- if (containerRef.current && window.AsciinemaPlayer) {
- const options = {
- autoPlay,
- loop,
- preload,
- controls,
- theme,
- };
-
- // Add poster if provided
- if (poster) {
- options.poster = poster;
- }
-
- // Add startAt if provided (for poster frame selection)
- if (startAt !== undefined) {
- options.startAt = startAt;
- }
-
- window.AsciinemaPlayer.create(src, containerRef.current, options);
- }
+ if (containerRef.current && window.AsciinemaPlayer) {
+ const options = { autoPlay, loop, preload, controls, theme, ...(poster ? { poster } : {}), ...(startAt !== undefined ? { startAt } : {}) };
+ containerRef.current.innerHTML = '';
+ window.AsciinemaPlayer.create(src, containerRef.current, options);
+ }Also applies to: 65-82
🤖 Prompt for AI Agents
.vortex/docs/src/components/AsciinemaPlayer/AsciinemaPlayer.js around lines 40
to 56 (and similarly 65 to 82): the options object is being built in stages
which can drift; instead construct it once using a single object literal with
conditional spreads (e.g. { autoPlay, loop, preload, controls, theme, ...(poster
? { poster } : {}), ...(startAt !== undefined ? { startAt } : {}) }) so remove
the multi-step mutation and replace with a single declaration that conditionally
includes poster and startAt.
| options.startAt = startAt; | ||
| } | ||
|
|
||
| window.AsciinemaPlayer.create(src, containerRef.current, options); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent duplicate DOM nodes: clear container before creating the player.
Without clearing, repeated renders stack players.
Apply (included in previous diffs):
- window.AsciinemaPlayer.create(src, containerRef.current, options);
+ containerRef.current.innerHTML = '';
+ window.AsciinemaPlayer.create(src, containerRef.current, options);(and same in the already-loaded branch)
Also applies to: 83-83
🤖 Prompt for AI Agents
In .vortex/docs/src/components/AsciinemaPlayer/AsciinemaPlayer.js around line 58
(and similarly at line 83), the Asciinema player is created into the container
without clearing existing DOM nodes, causing duplicate stacked players on
repeated renders; before calling window.AsciinemaPlayer.create(src,
containerRef.current, options) clear the container (remove all child nodes or
set containerRef.current.innerHTML = ''), then proceed to create the player so
only a single player DOM exists.
1070c96 to
22c2360
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
.vortex/docs/content/tools/drush.mdx (1)
55-60: Add pygmy prerequisite note
Pygmy must be installed to usepygmy addkey—it’s Lagoon’s lightweight local development tool (lagoon.sh).# or -pygmy addkey --key ~/.ssh/path/to/key # Run on the host to execute remote Drush commands from inside the container (including ahoy drush). +pygmy addkey --key ~/.ssh/path/to/key # Requires pygmy (Lagoon local tooling). Run on the host to execute remote Drush commands from inside the container (including ahoy drush)..vortex/docs/content/contributing/maintenance/release.mdx (1)
17-19: Fix grammar in Step 11.Use plural without “a” and add “the” before the template.
-11. Create a new release notes using [release template](./release_template.md). +11. Create new release notes using [the release template](./release_template.md)..vortex/docs/content/getting-started/installation.mdx (2)
13-15: Fix grammar (“an in interactive” → “an interactive”).-We automated the first part by providing an in interactive installer. The second +We automated the first part by providing an interactive installer. The second
1-4: Silence cspell for domain-specific terms (“Asciinema”).Fixes current “Vortex - Test docs” pipeline failures.
--- sidebar_position: 3 --- + +<!-- cspell:words Asciinema asciinema -->
♻️ Duplicate comments (19)
.vortex/docs/.utils/update-installer-video.sh (11)
17-19: Dangerous default WORKSPACE_DIR + unconditional rm -rf can wipe user data. Guard creation/cleanup.Don’t default to $HOME/www/demo; only delete directories the script created.
-# Directory for temporary workspace. -WORKSPACE_DIR="${WORKSPACE_DIR:-$HOME/www/demo}" +# Directory for temporary workspace (set in main; may be overridden via env). +# WORKSPACE_DIR is intentionally not defaulted here to avoid deleting user dirs.- WORKSPACE_DIR="${WORKSPACE_DIR:-"$(mktemp -d -t vortex-installer-recording-XXXXXX)"}" - [ ! -d "$WORKSPACE_DIR" ] && mkdir -p "$WORKSPACE_DIR" + local CREATED_TMPDIR="" + if [[ -z "${WORKSPACE_DIR:-}" ]]; then + CREATED_TMPDIR="$(mktemp -d -t vortex-installer-recording-XXXXXX)" + WORKSPACE_DIR="$CREATED_TMPDIR" + else + mkdir -p "$WORKSPACE_DIR" + fi @@ - cleanup() { - info "Cleaning up workspace directory" - note "Removing: ${WORKSPACE_DIR}" - rm -rf "$WORKSPACE_DIR" - } + cleanup() { + # Only remove if we created it in this run + if [[ -n "$CREATED_TMPDIR" && "$WORKSPACE_DIR" == "$CREATED_TMPDIR" ]]; then + info "Cleaning up workspace directory" + note "Removing: ${WORKSPACE_DIR}" + rm -rf "$WORKSPACE_DIR" + fi + }Also applies to: 372-387
50-69: Add node to dependency checks and improve install hints.npx requires node; add it and show CI-friendly install commands.
- local required_commands=("asciinema" "expect" "php" "composer" "npx") + local required_commands=("asciinema" "expect" "php" "composer" "node" "npx") @@ - echo " brew install asciinema expect composer # macOS" >&2 - echo " apt-get install asciinema expect-dev composer # Ubuntu/Debian" >&2 + echo " brew install asciinema expect composer node # macOS" >&2 + echo " apt-get update -y && apt-get install -y --no-install-recommends asciinema expect-dev composer nodejs npm # Ubuntu/Debian" >&2
76-83: Ensure destination dir exists before copy.Prevents failures when parent dir is missing.
- cp "$INSTALLER_BUILD" "$INSTALLER_DOCS" + mkdir -p "$(dirname "$INSTALLER_DOCS")" + cp "$INSTALLER_BUILD" "$INSTALLER_DOCS"
95-110: Harden composer for CI (non-interactive, quieter).Avoids interactivity and noisy logs.
- info "Running composer install..." - if ! composer install; then + info "Running composer install..." + if ! composer install --no-interaction --no-ansi --no-progress --prefer-dist; then @@ - info "Running composer build..." - if ! composer build; then + info "Running composer build..." + if ! composer build -n; then
113-120: Ensure destination dir exists before copy (post-build).- cp "$INSTALLER_BUILD" "$INSTALLER_DOCS" + mkdir -p "$(dirname "$INSTALLER_DOCS")" + cp "$INSTALLER_BUILD" "$INSTALLER_DOCS"
185-190: Use Expect/Tcl ‘after’ instead of ‘sleep’ for portability/precision.- sleep $prompt_delay + after [expr {int($prompt_delay * 1000)}]
286-291: Guard copy: fail early if installer source missing.- cp "$INSTALLER_DOCS" "$workspace_dir/installer.php" + if [ ! -f "$INSTALLER_DOCS" ]; then + fail "Installer not found at: $INSTALLER_DOCS" + return 1 + fi + cp "$INSTALLER_DOCS" "$workspace_dir/installer.php"
311-319: Make sed replacement broader and -i portable (macOS/Linux).- sed_opts=(-i) && [ "$(uname)" = "Darwin" ] && sed_opts=(-i '') - sed "${sed_opts[@]}" -E 's|/Users/[^/]+/|/home/user/|g' "$output_cast" + sed_opts=(-i) && [ "$(uname)" = "Darwin" ] && sed_opts=(-i '') + sed "${sed_opts[@]}" -E 's|/(Users|home)/[^/]+/|/home/user/|g' "$output_cast"
320-341: Make all npx calls non-interactive (-y) for CI stability.- if ! npx svg-term --out="$output_svg" <"$output_cast"; then + if ! npx -y svg-term --out="$output_svg" <"$output_cast"; then @@ - if ! npx svg-term --at=1000 --out="$output_png.svg" <"$output_cast"; then + if ! npx -y svg-term --at=1000 --out="$output_png.svg" <"$output_cast"; then @@ - if ! npx sharp-cli -i "$output_png.svg" -o "$output_png" -f png resize 1280; then + if ! npx -y sharp-cli -i "$output_png.svg" -o "$output_png" -f png resize 1280; then
418-424: Create destination dirs before copying outputs.- cp "$temp_cast" "$CAST_FILE" + mkdir -p "$(dirname "$CAST_FILE")" + cp "$temp_cast" "$CAST_FILE" @@ - cp "$temp_svg" "$SVG_FILE" + mkdir -p "$(dirname "$SVG_FILE")" + cp "$temp_svg" "$SVG_FILE" @@ - cp "$temp_png" "$PNG_FILE" + mkdir -p "$(dirname "$PNG_FILE")" + cp "$temp_png" "$PNG_FILE"
12-13: Enable pipefail for safer pipelines.Prevents silent failures when any command in a pipeline fails.
-set -eu +set -euo pipefail.github/workflows/vortex-release-docs.yml (1)
135-140: Make apt non-interactive/reliable and guard long-running script with timeout.Prevents hangs and flaky installs on runners.
- name: Generate video for installer (not used in the final artifact) run: | - sudo apt install asciinema expect - ./update-installer-video.sh + export DEBIAN_FRONTEND=noninteractive + sudo apt-get update -y + sudo apt-get install -y --no-install-recommends asciinema expect + timeout 15m bash ./update-installer-video.sh working-directory: .vortex/docs/.utils.github/workflows/vortex-test-installer.yml (2)
33-37: Prefer a moving LTS range for Node to reduce churn.- node-version: 22.17.1 + node-version: '22.x'
59-64: Same apt reliability/timeout fixes as release workflow.- name: Generate video for installer (not used in the final artifact) run: | - sudo apt install asciinema expect - ./update-installer-video.sh + export DEBIAN_FRONTEND=noninteractive + sudo apt-get update -y + sudo apt-get install -y --no-install-recommends asciinema expect + timeout 15m bash ./update-installer-video.sh working-directory: .vortex/docs/.utils.vortex/docs/content/getting-started/installation.mdx (2)
5-5: Avoid SSR issues: render player client-side with BrowserOnly.Prevents hydration warnings if the player touches window/DOM.
-import AsciinemaPlayer from '@site/src/components/AsciinemaPlayer'; +import BrowserOnly from '@docusaurus/BrowserOnly'; +import AsciinemaPlayer from '@site/src/components/AsciinemaPlayer';
27-35: Wrap AsciinemaPlayer with BrowserOnly.- <AsciinemaPlayer - src="/img/installer.json" - poster="npt:0:1" - autoPlay={false} - loop={false} - preload={true} - controls={true} - theme="asciinema" - /> + <BrowserOnly> + {() => ( + <AsciinemaPlayer + src="/img/installer.json" + poster="npt:0:1" + autoPlay={false} + loop={false} + preload={true} + controls={true} + theme="asciinema" + /> + )} + </BrowserOnly>.vortex/.ahoy.yml (1)
110-115: Harden the command wrapper and avoid fragile relative paths.Fail fast and resolve the script from repo root so it works from any CWD.
update-installer-video: usage: Update installer video in the documentation. cmd: | - cd docs/.utils - ./update-installer-video.sh + set -euo pipefail + cd "$(git rev-parse --show-toplevel)/docs/.utils" + ./update-installer-video.sh.vortex/docs/src/components/AsciinemaPlayer/AsciinemaPlayer.js (1)
91-93: Add effect cleanup to prevent duplicate players and memory leaks.- loadAsciinemaPlayer(); - }, [src, poster, startAt, autoPlay, loop, preload, controls, theme]); + loadAsciinemaPlayer(); + return () => { + if (containerRef.current) containerRef.current.innerHTML = ''; + }; + }, [src, poster, startAt, autoPlay, loop, preload, controls, theme]);.vortex/docs/static/img/installer.json (1)
1-6: Fix Biome parse failures: rename .json (JSON Lines) to .cast or ignore the file.This asciinema v2 recording is newline-delimited JSON, not a single JSON document; Biome flags it.
Option A (preferred): rename and update references
- src="/img/installer.json" + src="/img/installer.cast"Option B: ignore in Biome (new file at repo root)
+.biomeignore +/.vortex/docs/static/img/installer.json
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
.vortex/docs/static/img/installer.pngis excluded by!**/*.png.vortex/docs/static/img/installer.svgis excluded by!**/*.svg
📒 Files selected for processing (13)
.github/workflows/vortex-release-docs.yml(1 hunks).github/workflows/vortex-test-installer.yml(2 hunks).vortex/.ahoy.yml(1 hunks).vortex/docs/.gitignore(1 hunks).vortex/docs/.utils/update-installer-video.sh(1 hunks).vortex/docs/content/contributing/maintenance/release.mdx(1 hunks).vortex/docs/content/getting-started/installation.mdx(2 hunks).vortex/docs/content/tools/drush.mdx(1 hunks).vortex/docs/content/workflows/deployment.mdx(1 hunks).vortex/docs/src/components/AsciinemaPlayer/AsciinemaPlayer.js(1 hunks).vortex/docs/src/components/AsciinemaPlayer/index.js(1 hunks).vortex/docs/static/img/installer.json(1 hunks)README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-29T12:15:32.188Z
Learnt from: AlexSkrypnyk
PR: drevops/vortex#0
File: :0-0
Timestamp: 2025-05-29T12:15:32.188Z
Learning: Do not review files in `.vortex/installer/tests/Fixtures/install` directory as they are test fixtures.
Applied to files:
.vortex/docs/.gitignore
🪛 LanguageTool
.vortex/docs/content/contributing/maintenance/release.mdx
[grammar] ~18-~18: Use articles correctly
Context: ...hoy update-installer-video`. 11. Create a new release notes using [release templa...
(QB_NEW_EN_OTHER_ERROR_IDS_11)
[grammar] ~18-~18: There might be a mistake here.
Context: ...elease template](./release_template.md).
(QB_NEW_EN)
.vortex/docs/content/getting-started/installation.mdx
[grammar] ~5-~5: Use correct spacing
Context: ... '@site/src/components/AsciinemaPlayer'; # Installation The installation process c...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🪛 Biome (2.1.2)
.vortex/docs/static/img/installer.json
[error] 1-4: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 4-4: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 4-6: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 6-6: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 6-8: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 8-8: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 8-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-36: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 36-36: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 36-38: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 38-38: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 38-38: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 38-40: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 40-44: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 44-44: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 44-46: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 46-46: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 46-46: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 46-49: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 49-49: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 49-50: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 50-51: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 51-56: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 56-56: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 56-56: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 56-56: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 56-59: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 59-59: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 59-62: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 62-62: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 62-62: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 62-65: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 65-68: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 68-68: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 68-68: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 68-70: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 70-70: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 70-70: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 70-71: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 71-71: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 71-75: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 75-77: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 77-77: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 77-77: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 77-80: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 80-80: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 80-80: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 80-82: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 82-82: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 82-84: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 84-86: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 86-86: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 86-89: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 89-91: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 91-94: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 94-94: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 94-94: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 94-96: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 96-96: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 96-97: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 97-100: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 100-100: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 100-102: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 102-102: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 102-105: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 105-105: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 105-107: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 107-107: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 107-109: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 109-109: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 109-111: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 111-111: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 111-111: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 111-112: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 112-114: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 114-114: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 114-114: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 114-116: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 116-120: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 120-122: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 122-122: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 122-122: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 122-124: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 124-125: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 125-135: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 135-136: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 136-137: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 137-146: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 146-146: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 146-150: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 150-150: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 150-151: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 151-151: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🪛 markdownlint-cli2 (0.17.2)
README.md
54-54: Inline HTML
Element: img
(MD033, no-inline-html)
🪛 GitHub Actions: Vortex - Test docs
.vortex/docs/content/getting-started/installation.mdx
[error] 5-5: CSpell: Unknown word 'Asciinema' detected on line 5, column 8.
[error] 5-5: CSpell: Unknown word 'Asciinema' detected on line 5, column 51.
[error] 27-27: CSpell: Unknown word 'Asciinema' detected on line 27, column 6.
[error] 34-34: CSpell: Unknown word 'asciinema' detected on line 34, column 14.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-installer (8.4)
🔇 Additional comments (3)
.vortex/docs/content/workflows/deployment.mdx (1)
30-33: Blank line before list is correct.The added blank line before the branches list fixes Markdown rendering edge cases. LGTM.
.vortex/docs/.gitignore (1)
24-26: Rule is clear and scoped.Ignoring the copied installer file under docs is reasonable and the explanatory comment helps. LGTM.
.vortex/docs/src/components/AsciinemaPlayer/index.js (1)
1-1: LGTM: clean barrel export.
| if (!document.querySelector('link[href*="asciinema-player.css"]')) { | ||
| const link = document.createElement('link'); | ||
| link.rel = 'stylesheet'; | ||
| link.href = | ||
| 'https://cdn.jsdelivr.net/npm/asciinema-player@3/dist/bundle/asciinema-player.css'; | ||
| document.head.appendChild(link); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Pin CSS CDN to an exact version for reproducibility.
- link.href =
- 'https://cdn.jsdelivr.net/npm/asciinema-player@3/dist/bundle/asciinema-player.css';
+ link.href =
+ 'https://cdn.jsdelivr.net/npm/asciinema-player@3.10.0/dist/bundle/asciinema-player.css';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!document.querySelector('link[href*="asciinema-player.css"]')) { | |
| const link = document.createElement('link'); | |
| link.rel = 'stylesheet'; | |
| link.href = | |
| 'https://cdn.jsdelivr.net/npm/asciinema-player@3/dist/bundle/asciinema-player.css'; | |
| document.head.appendChild(link); | |
| } | |
| if (!document.querySelector('link[href*="asciinema-player.css"]')) { | |
| const link = document.createElement('link'); | |
| link.rel = 'stylesheet'; | |
| link.href = | |
| 'https://cdn.jsdelivr.net/npm/asciinema-player@3.10.0/dist/bundle/asciinema-player.css'; | |
| document.head.appendChild(link); | |
| } |
🤖 Prompt for AI Agents
.vortex/docs/src/components/AsciinemaPlayer/AsciinemaPlayer.js around lines 25
to 31: the CSS CDN URL currently uses a floating range (@3) which can change
over time; update the link.href to reference an exact published version (replace
"@3" with a specific semver like "@3.1.0" or the project-approved release tag)
so the CSS is reproducible, and commit that pinned URL (and update any comment
noting the pinned version to remind to update intentionally).
| <img src=".vortex/docs/static/img/installer.svg" alt="Vortex installer screenshot" /> | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Silence MD033 or switch to Markdown image.
Inline HTML triggers markdownlint MD033. Either disable the rule for this line or use Markdown image syntax.
Option A (disable rule, keep HTML for styling control):
+<!-- markdownlint-disable-next-line MD033 -->
<img src=".vortex/docs/static/img/installer.svg" alt="Vortex installer screenshot" />Option B (Markdown image):
-<img src=".vortex/docs/static/img/installer.svg" alt="Vortex installer screenshot" />
+📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <img src=".vortex/docs/static/img/installer.svg" alt="Vortex installer screenshot" /> | |
|  |
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
54-54: Inline HTML
Element: img
(MD033, no-inline-html)
🤖 Prompt for AI Agents
In README.md around lines 54-55, the inline HTML image tag triggers markdownlint
rule MD033; either disable MD033 for that line with an HTML comment marker or
replace the HTML with standard Markdown image syntax () to avoid the
lint error — choose the approach you prefer, add the corresponding change on
that line, and if disabling the rule include a brief comment (e.g. <!--
markdownlint-disable MD033 --> before or on the same line) so the linter skips
it.
22c2360 to
f832e49
Compare
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
.vortex/docs/tests/jest-setup.js (1)
4-28: Do not override native EventTarget if present.JSDOM increasingly provides
EventTarget; unconditional override can break libraries relying on the native interface. Guard the polyfill.Apply:
-// Polyfill for EventTarget in jsdom environment -global.EventTarget = class EventTarget { +// Polyfill for EventTarget in jsdom environment +if (typeof global.EventTarget === 'undefined') { + global.EventTarget = class EventTarget { constructor() { this.listeners = {}; } addEventListener(type, listener) { if (!this.listeners[type]) { this.listeners[type] = []; } this.listeners[type].push(listener); } removeEventListener(type, listener) { if (this.listeners[type]) { this.listeners[type] = this.listeners[type].filter(l => l !== listener); } } dispatchEvent(event) { if (this.listeners[event.type]) { this.listeners[event.type].forEach(listener => listener(event)); } return true; } -}; + }; +}.vortex/docs/content/tools/drush.mdx (1)
55-60: Clarify pygmy usage context in the SSH key example.Readers may not have pygmy installed; add a note inline to indicate it’s optional and requires pygmy.
```shell ssh-add ~/.ssh/path/to/key # Run on host to run remote Drush commands from host. # or -pygmy addkey --key ~/.ssh/path/to/key # Run on the host to execute remote Drush commands from inside the container (including ahoy drush). +pygmy addkey --key ~/.ssh/path/to/key # Requires pygmy; use if your stack includes pygmy for forwarding keys into containers.</blockquote></details> <details> <summary>.vortex/docs/tests/unit/ContentExtractor/ContentExtractor.test.js (1)</summary><blockquote> `475-493`: **Rework or remove the null-ref test (tests/unit/ContentExtractor/ContentExtractor.test.js:475-493)** This test renders a standalone component and never exercises the `if (!contentRef.current)` guard in ContentExtractor.js (lines 41–42), so it’s redundant. Rewrite it to render `ContentExtractor` with a null ref to cover that branch or remove it. </blockquote></details> <details> <summary>.vortex/docs/tests/unit/Card/CardGrid.test.js (2)</summary><blockquote> `201-204`: **Avoid brittle assertion on trailing space in className.** Comparing raw className with a trailing space is fragile and implementation-dependent. Assert classes instead. Apply: ```diff - expect(gridElement.className).toBe('cards-grid '); + expect(gridElement).toHaveClass('cards-grid'); + expect(gridElement.classList).toHaveLength(1);
206-216: Test name mentions boolean but only asserts numeric className.Either add a boolean case or rename the test to reflect what’s exercised.
Apply either:
- test('handles numeric and boolean className values', () => { + test('handles numeric className values', () => {Or add a boolean check:
- const { container } = render( + const { container, rerender } = render( <CardGrid className={123}> <div>Test child</div> </CardGrid> ); + rerender( + <CardGrid className={true}> + <div>Test child</div> + </CardGrid> + ); + expect(container.querySelector('.cards-grid')).toHaveClass('true');.github/workflows/vortex-release-docs.yml (2)
115-119: Fix artifact path: default download-artifact extracts into a folder named after the artifact.cp is pointing to ../installer.phar, but the file will be at ../vortex-installer/installer.phar by default.
Apply:
- name: Copy installer to docs run: | - cp ../installer.phar ../.vortex/docs/static/install + cp ../vortex-installer/installer.phar ../.vortex/docs/static/install php ../.vortex/docs/static/install --version working-directory: .vortex/docs
102-106: Use a moving Node range to reduce maintenance.Prefer 22.x or lts/* to auto-receive patch updates.
- name: Setup Node.js uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: - node-version: 22.17.1 + node-version: '22.x'.vortex/docs/content/getting-started/installation.mdx (2)
13-15: Fix typo and spacing in user-facing docs.“an in interactive” → “an interactive”.
-We automated the first part by providing an in interactive installer. The second +We automated the first part by providing an interactive installer. The second
98-99: Minor spacing nit.Remove double space before “and”.
-Depending on the services you are using, you will need to set up the -integrations with your hosting provider, continuous integration provider, and +Depending on the services you are using, you will need to set up the +integrations with your hosting provider, continuous integration provider, and
♻️ Duplicate comments (18)
.vortex/docs/.gitignore (1)
24-25: Disambiguate whether “static/install” is a file or directory.If it’s a directory, add a trailing slash; if it’s a single file, keep as-is. This mirrors a prior review point.
- static/install + static/install/.vortex/docs/src/components/AsciinemaPlayer/AsciinemaPlayer.js (3)
23-29: Pin CDN assets and dedupe via stable selectors.Floating @3 can drift; pin to an exact version and give the CSS link a stable id so it isn’t injected multiple times.
Apply:
- if (!document.querySelector('link[href*="asciinema-player.css"]')) { + if (!document.getElementById('asciinema-player-css')) { const link = document.createElement('link'); link.rel = 'stylesheet'; - link.href = - 'https://cdn.jsdelivr.net/npm/asciinema-player@3/dist/bundle/asciinema-player.css'; + link.id = 'asciinema-player-css'; + link.href = + 'https://cdn.jsdelivr.net/npm/asciinema-player@3.10.0/dist/bundle/asciinema-player.css'; document.head.appendChild(link); }- script.src = - 'https://cdn.jsdelivr.net/npm/asciinema-player@3/dist/bundle/asciinema-player.min.js'; + script.src = + 'https://cdn.jsdelivr.net/npm/asciinema-player@3.10.0/dist/bundle/asciinema-player.min.js'; + script.async = true;What is the latest stable version of asciinema-player v3 on npm/jsDelivr?Also applies to: 33-35
31-77: Harden dynamic loader: dedupe, await load/error, clear container, and add cleanup.Prevents duplicate scripts, races, and stacked players; surfaces load failures; tears down on unmount.
Apply:
- if (!window.AsciinemaPlayer) { - const script = document.createElement('script'); - script.src = - 'https://cdn.jsdelivr.net/npm/asciinema-player@3.10.0/dist/bundle/asciinema-player.min.js'; - script.onload = () => { - if (containerRef.current && window.AsciinemaPlayer) { - const options = { - autoPlay, - loop, - preload, - controls, - theme, - }; - - if (poster) { - options.poster = poster; - } - - if (startAt !== undefined) { - options.startAt = startAt; - } - - window.AsciinemaPlayer.create(src, containerRef.current, options); - } - }; - document.head.appendChild(script); - } else { - if (containerRef.current && window.AsciinemaPlayer) { - const options = { - autoPlay, - loop, - preload, - controls, - theme, - }; - - if (poster) { - options.poster = poster; - } - - if (startAt !== undefined) { - options.startAt = startAt; - } - - window.AsciinemaPlayer.create(src, containerRef.current, options); - } - } + if (!window.AsciinemaPlayer) { + if (!window.__asciinemaLoader) { + const existing = document.querySelector('script[src*="asciinema-player.min.js"]'); + window.__asciinemaLoader = new Promise((resolve, reject) => { + const onLoad = () => resolve(); + const onError = () => reject(new Error('Failed to load Asciinema player script')); + if (existing) { + existing.addEventListener('load', onLoad, { once: true }); + existing.addEventListener('error', onError, { once: true }); + } else { + const script = document.createElement('script'); + script.src = 'https://cdn.jsdelivr.net/npm/asciinema-player@3.10.0/dist/bundle/asciinema-player.min.js'; + script.async = true; + script.onload = onLoad; + script.onerror = onError; + document.head.appendChild(script); + } + }); + } + try { + await window.__asciinemaLoader; + } catch (e) { + console.error(e); + return; + } + } + + if (containerRef.current && window.AsciinemaPlayer) { + const options = { + autoPlay, loop, preload, controls, theme, + ...(poster ? { poster } : {}), + ...(startAt !== undefined ? { startAt } : {}), + }; + containerRef.current.innerHTML = ''; + window.AsciinemaPlayer.create(src, containerRef.current, options); + } @@ - loadAsciinemaPlayer(); - }, [src, poster, startAt, autoPlay, loop, preload, controls, theme]); + loadAsciinemaPlayer(); + return () => { + if (containerRef.current) containerRef.current.innerHTML = ''; + }; + }, [src, poster, startAt, autoPlay, loop, preload, controls, theme]);Also applies to: 83-86
37-51: Deduplicate option building.Construct options once using conditional spreads in both branches.
Apply (already included in previous diff): create a single options object with conditional poster/startAt and reuse it.
Also applies to: 59-75
.vortex/docs/static/img/installer.json (1)
1-152: Biome parse errors: asciinema v2 is JSON Lines, not JSON. Rename to .cast or ignore.This NDJSON format breaks JSON linters. Prefer renaming to installer.cast and update references; otherwise add to .biomeignore.
Apply one of:
- Rename + update references (preferred):
- src="/img/installer.json" + src="/img/installer.cast"
- Ignore file (repo root .biomeignore):
+/.vortex/docs/static/img/installer.json#!/bin/bash # Find references to the current path to update them after rename. rg -n "/img/installer\.json" -C2 .vortex/docs.vortex/docs/.utils/update-installer-video.sh (7)
50-69: Include node in dependency checks and update install hints.npx depends on node; add it and make hints actionable.
Apply:
- local required_commands=("asciinema" "expect" "php" "composer" "npx") + local required_commands=("asciinema" "expect" "php" "composer" "node" "npx") @@ - echo " brew install asciinema expect composer # macOS" >&2 - echo " apt-get install asciinema expect-dev composer # Ubuntu/Debian" >&2 + echo " brew install asciinema expect composer node # macOS" >&2 + echo " apt-get update && apt-get install -y asciinema expect-dev composer nodejs npm # Ubuntu/Debian" >&2
76-83: Ensure destination directories exist before copying.Prevents cp failures when target dirs are missing.
Apply:
- cp "$INSTALLER_BUILD" "$INSTALLER_DOCS" + mkdir -p "$(dirname "$INSTALLER_DOCS")" + cp "$INSTALLER_BUILD" "$INSTALLER_DOCS" @@ - cp "$INSTALLER_BUILD" "$INSTALLER_DOCS" + mkdir -p "$(dirname "$INSTALLER_DOCS")" + cp "$INSTALLER_BUILD" "$INSTALLER_DOCS" @@ - cp "$temp_cast" "$CAST_FILE" + mkdir -p "$(dirname "$CAST_FILE")" + cp "$temp_cast" "$CAST_FILE" @@ - cp "$temp_svg" "$SVG_FILE" + mkdir -p "$(dirname "$SVG_FILE")" + cp "$temp_svg" "$SVG_FILE" @@ - cp "$temp_png" "$PNG_FILE" + mkdir -p "$(dirname "$PNG_FILE")" + cp "$temp_png" "$PNG_FILE"Also applies to: 113-120, 418-425
95-105: Harden composer for CI (non-interactive, quieter).Avoid prompts and reduce noise.
Apply:
- if ! composer install; then + if ! composer install --no-interaction --no-ansi --no-progress; then @@ - if ! composer build; then + if ! composer build -n; then
286-290: Guard copy into workspace.Fail clearly if installer is missing.
Apply:
- cp "$INSTALLER_DOCS" "$workspace_dir/installer.php" + if [ ! -f "$INSTALLER_DOCS" ]; then + fail "Installer not found at: $INSTALLER_DOCS" + return 1 + fi + cp "$INSTALLER_DOCS" "$workspace_dir/installer.php"
317-319: Make sed replacement more robust (mac/Linux) and less specific to home path.Covers /Users and /home, preserves -i semantics.
Apply:
- sed_opts=(-i) && [ "$(uname)" = "Darwin" ] && sed_opts=(-i '') - sed "${sed_opts[@]}" -E 's|/Users/[^/]+/|/home/user/|g' "$output_cast" + sed_opts=(-i) && [ "$(uname)" = "Darwin" ] && sed_opts=(-i '') + sed "${sed_opts[@]}" -E 's|/(Users|home)/[^/]+/|/home/user/|g' "$output_cast"
323-341: Make all npx calls non-interactive to avoid CI stalls.Add -y to auto-accept installs.
Apply:
- if ! npx svg-term --out="$output_svg" <"$output_cast"; then + if ! npx -y svg-term --out="$output_svg" <"$output_cast"; then @@ - if ! npx svg-term --at=1000 --out="$output_png.svg" <"$output_cast"; then + if ! npx -y svg-term --at=1000 --out="$output_png.svg" <"$output_cast"; then @@ - if ! npx sharp-cli -i "$output_png.svg" -o "$output_png" -f png resize 1280; then + if ! npx -y sharp-cli -i "$output_png.svg" -o "$output_png" -f png resize 1280; then
185-190: Use Expect ‘after’ instead of ‘sleep’ inside the generated script.sleep isn’t portable here; ‘after’ works reliably.
Apply (inside heredoc):
- sleep $prompt_delay + after [expr {int($prompt_delay * 1000)}]README.md (1)
54-55: markdownlint MD033: replace inline HTML img or disable rule.Switch to Markdown image syntax or silence MD033 for this line.
Apply one of:
-<img src=".vortex/docs/static/img/installer.svg" alt="Vortex installer screenshot" /> +or
+<!-- markdownlint-disable-next-line MD033 --> <img src=".vortex/docs/static/img/installer.svg" alt="Vortex installer screenshot" />.github/workflows/vortex-release-docs.yml (1)
135-139: Make apt non-interactive and guard the long-running script with a timeout.Prevents hangs/404s on runners; ensures the step doesn’t block the job indefinitely.
Apply:
- name: Generate video for installer (not used in the final artifact) run: | - sudo apt install asciinema expect - ./update-installer-video.sh + export DEBIAN_FRONTEND=noninteractive + sudo apt-get update -y + sudo apt-get install -y --no-install-recommends asciinema expect + timeout 15m bash ./update-installer-video.sh working-directory: .vortex/docs/.utils.github/workflows/vortex-test-installer.yml (2)
33-37: Prefer a moving Node range.- name: Setup Node.js uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: - node-version: 22.17.1 + node-version: '22.x'
59-63: Make apt non-interactive and add timeout for reliability.- name: Generate video for installer (not used in the final artifact) run: | - sudo apt install asciinema expect - ./update-installer-video.sh + export DEBIAN_FRONTEND=noninteractive + sudo apt-get update -y + sudo apt-get install -y --no-install-recommends asciinema expect + timeout 15m bash ./update-installer-video.sh working-directory: .vortex/docs/.utils.vortex/docs/content/getting-started/installation.mdx (1)
5-5: Avoid SSR hydration issues: render the player client-only.Wrap with Docusaurus BrowserOnly (or ensure the component is SSR-safe).
+import BrowserOnly from '@docusaurus/BrowserOnly'; import AsciinemaPlayer from '@site/src/components/AsciinemaPlayer'; @@ - <AsciinemaPlayer - src="/img/installer.json" - poster="npt:0:1" - autoPlay={false} - loop={false} - preload={true} - controls={true} - theme="asciinema" - /> + <BrowserOnly> + {() => ( + <AsciinemaPlayer + src="/img/installer.json" + poster="npt:0:1" + autoPlay={false} + loop={false} + preload={true} + controls={true} + theme="asciinema" + /> + )} + </BrowserOnly>Also applies to: 27-35
.vortex/.ahoy.yml (1)
110-115: Harden wrapper: use strict modes and absolute path; forward args.Current command assumes CWD and lacks fail-fast. Make it robust and pass through args.
update-installer-video: usage: Update installer video in the documentation. cmd: | - cd docs/.utils - ./update-installer-video.sh + set -euo pipefail + repo_root="$(git rev-parse --show-toplevel)" + cd "${repo_root}/docs/.utils" + exec ./update-installer-video.sh "$@"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
.vortex/docs/static/img/installer.pngis excluded by!**/*.png.vortex/docs/static/img/installer.svgis excluded by!**/*.svg
📒 Files selected for processing (27)
.github/workflows/vortex-release-docs.yml(1 hunks).github/workflows/vortex-test-installer.yml(2 hunks).vortex/.ahoy.yml(1 hunks).vortex/docs/.gitignore(1 hunks).vortex/docs/.prettierignore(1 hunks).vortex/docs/.utils/update-installer-video.sh(1 hunks).vortex/docs/content/contributing/maintenance/release.mdx(1 hunks).vortex/docs/content/getting-started/installation.mdx(2 hunks).vortex/docs/content/tools/drush.mdx(1 hunks).vortex/docs/content/workflows/deployment.mdx(1 hunks).vortex/docs/cspell.json(1 hunks).vortex/docs/package.json(1 hunks).vortex/docs/src/components/AsciinemaPlayer/AsciinemaPlayer.js(1 hunks).vortex/docs/src/components/AsciinemaPlayer/index.js(1 hunks).vortex/docs/static/img/installer.json(1 hunks).vortex/docs/tests/fixtures/test-cast.json(1 hunks).vortex/docs/tests/jest-setup.js(1 hunks).vortex/docs/tests/unit/AsciinemaPlayer/AsciinemaPlayer.test.js(1 hunks).vortex/docs/tests/unit/Card/Card.integration.test.js(10 hunks).vortex/docs/tests/unit/Card/Card.test.js(16 hunks).vortex/docs/tests/unit/Card/CardGrid.test.js(10 hunks).vortex/docs/tests/unit/Card/index.test.js(1 hunks).vortex/docs/tests/unit/ContentExtractor/ContentExtractor.test.js(23 hunks).vortex/docs/tests/unit/VerticalTabs/MarkerComponents.test.js(2 hunks).vortex/docs/tests/unit/VerticalTabs/VerticalTabs.test.js(6 hunks).vortex/docs/tests/unit/VerticalTabs/index.test.js(2 hunks)README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-29T12:15:32.188Z
Learnt from: AlexSkrypnyk
PR: drevops/vortex#0
File: :0-0
Timestamp: 2025-05-29T12:15:32.188Z
Learning: Do not review files in `.vortex/installer/tests/Fixtures/install` directory as they are test fixtures.
Applied to files:
.vortex/docs/.prettierignore.vortex/docs/.gitignore
🧬 Code graph analysis (5)
.vortex/docs/tests/unit/VerticalTabs/MarkerComponents.test.js (2)
.vortex/docs/src/components/VerticalTabs/VerticalTab.js (1)
VerticalTab(7-10).vortex/docs/src/components/VerticalTabs/VerticalTabPanel.js (1)
VerticalTabPanel(7-10)
.vortex/docs/tests/unit/AsciinemaPlayer/AsciinemaPlayer.test.js (1)
.vortex/docs/src/components/AsciinemaPlayer/AsciinemaPlayer.js (1)
AsciinemaPlayer(3-87)
.vortex/docs/tests/unit/VerticalTabs/index.test.js (3)
.vortex/docs/src/components/VerticalTabs/VerticalTabs.js (1)
VerticalTabs(83-210).vortex/docs/src/components/VerticalTabs/VerticalTab.js (1)
VerticalTab(7-10).vortex/docs/src/components/VerticalTabs/VerticalTabPanel.js (1)
VerticalTabPanel(7-10)
.vortex/docs/.utils/update-installer-video.sh (2)
.vortex/installer/src/Utils/Config.php (1)
set(82-93).vortex/installer/src/Utils/Validator.php (1)
dirname(32-34)
.vortex/docs/tests/unit/ContentExtractor/ContentExtractor.test.js (1)
.vortex/docs/src/components/ContentExtractor.js (1)
ContentExtractor(29-135)
🪛 LanguageTool
.vortex/docs/content/contributing/maintenance/release.mdx
[grammar] ~18-~18: Use articles correctly
Context: ...hoy update-installer-video`. 11. Create a new release notes using [release templa...
(QB_NEW_EN_OTHER_ERROR_IDS_11)
[grammar] ~18-~18: There might be a mistake here.
Context: ...elease template](./release_template.md).
(QB_NEW_EN)
.vortex/docs/content/getting-started/installation.mdx
[grammar] ~5-~5: Use correct spacing
Context: ... '@site/src/components/AsciinemaPlayer'; # Installation The installation process c...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🪛 Biome (2.1.2)
.vortex/docs/tests/fixtures/test-cast.json
[error] 1-2: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 2-3: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 3-4: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 4-5: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 5-6: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 6-7: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 7-8: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 8-9: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 9-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 10-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
.vortex/docs/static/img/installer.json
[error] 1-4: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 4-4: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 4-6: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 6-6: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 6-8: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 8-8: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 8-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-36: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 36-36: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 36-38: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 38-38: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 38-38: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 38-40: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 40-44: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 44-44: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 44-46: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 46-46: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 46-46: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 46-49: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 49-49: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 49-50: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 50-51: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 51-56: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 56-56: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 56-56: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 56-56: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 56-59: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 59-59: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 59-62: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 62-62: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 62-62: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 62-65: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 65-68: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 68-68: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 68-68: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 68-70: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 70-70: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 70-70: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 70-71: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 71-71: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 71-75: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 75-77: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 77-77: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 77-77: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 77-80: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 80-80: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 80-80: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 80-82: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 82-82: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 82-84: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 84-86: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 86-86: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 86-89: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 89-91: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 91-94: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 94-94: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 94-94: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 94-96: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 96-96: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 96-97: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 97-100: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 100-100: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 100-102: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 102-102: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 102-105: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 105-105: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 105-107: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 107-107: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 107-109: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 109-109: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 109-111: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 111-111: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 111-111: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 111-112: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 112-114: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 114-114: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 114-114: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 114-116: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 116-120: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 120-122: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 122-122: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 122-122: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 122-124: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 124-125: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 125-135: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 135-136: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 136-137: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 137-146: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 146-146: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 146-150: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 150-150: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 150-151: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 151-151: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🪛 markdownlint-cli2 (0.17.2)
README.md
54-54: Inline HTML
Element: img
(MD033, no-inline-html)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (0)
🔇 Additional comments (17)
.vortex/docs/content/workflows/deployment.mdx (1)
32-32: LGTM: readability tweak.The added blank line improves list separation; no content changes.
.vortex/docs/tests/unit/VerticalTabs/VerticalTabs.test.js (5)
2-8: LGTM: import formatting.Readability improved; all imported symbols are used.
448-450: LGTM: multi-line chaining for clarity.No behavioral change; selection remains precise.
505-508: LGTM: explicit expectation arguments.Clearer assertion for event listener wiring.
546-549: LGTM: consistent selection pattern.Matches the earlier style for active tab checks.
646-649: LGTM: listener add/remove assertions.Good coverage of mount/unmount behavior.
Also applies to: 655-658
.vortex/docs/tests/unit/VerticalTabs/MarkerComponents.test.js (1)
31-36: LGTM — formatting-only edits.Whitespace adjustments don’t affect behavior; tests still validate marker components correctly.
Also applies to: 65-70, 72-72
.vortex/docs/tests/unit/Card/index.test.js (1)
33-33: LGTM — trailing newline only.No semantic changes.
.vortex/docs/tests/unit/Card/Card.test.js (1)
20-23: LGTM — formatting-only refactors across tests.Line wraps and JSX inlining don’t change behavior; assertions remain equivalent.
Also applies to: 41-41, 44-44, 51-51, 55-55, 71-71, 93-93, 95-95, 97-97, 99-101, 129-132, 142-145, 148-148, 179-186, 191-198, 205-205, 217-217, 279-279, 309-309, 339-339, 348-351, 364-368, 376-380, 394-394, 426-426
.vortex/docs/tests/unit/Card/Card.integration.test.js (1)
46-48: LGTM – formatting-only edits.No behavioral changes; readability improved.
Also applies to: 67-67, 171-173, 229-229, 251-254, 405-406
.vortex/docs/tests/unit/VerticalTabs/index.test.js (1)
1-1: LGTM – whitespace/formatting cleanup.No logic changes; tests remain clear and focused on export shape.
Also applies to: 20-20
.vortex/docs/package.json (1)
21-22: Prettier ignore covers Asciinema fixture
The existing .vortex/docs/.prettierignore already includes tests/fixtures/*.json, so test-cast.json is excluded from Prettier and will not cause lint failures.Likely an incorrect or invalid review comment.
.vortex/docs/tests/unit/ContentExtractor/ContentExtractor.test.js (1)
10-12: LGTM – assertions updated to reflect multi-line intro text and new filtering cases.The tests align with implementation (intro substring match, selector-based slicing, defaults). Good coverage improvements.
Also applies to: 77-87, 118-126, 146-149, 166-171, 186-191, 209-215, 232-237, 246-250, 261-267, 280-285, 302-313, 324-331, 355-361, 374-380, 386-394, 434-446, 463-471, 478-481, 503-507, 512-520, 531-543, 553-561
.vortex/docs/tests/unit/Card/CardGrid.test.js (1)
61-69: Formatting-only refactor looks good.Inline Card props, fragment/array tweaks, and conditional wrapping don’t change behavior. Tests remain readable and deterministic.
Also applies to: 112-113, 221-229, 255-259, 272-290, 308-314, 451-452
.vortex/docs/src/components/AsciinemaPlayer/index.js (1)
1-1: Barrel export looks good.Simple, conventional re-export. No issues.
.vortex/docs/tests/unit/AsciinemaPlayer/AsciinemaPlayer.test.js (2)
536-567: Good coverage of the “library already exists” path and options wiring.Asserts poster/startAt/theme/booleans are forwarded correctly when AsciinemaPlayer is preloaded. Looks solid.
1-3: Ensure '@testing-library/jest-dom' is loaded globally
I didn’t find any import of jest-dom matchers or a Jest setup configuration in the repo; please verify that jest-dom is configured (for example, by addingimport '@testing-library/jest-dom'to your Jest setup file and listing it undersetupFilesAfterEnv).
| package-lock.json | ||
| *.md | ||
| *.mdx | ||
| *.mdx |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Confirm intent to ignore MDX formatting.
Ignoring *.mdx prevents Prettier from standardizing docs formatting. If this wasn’t intentional, drop this line; otherwise, keep as-is.
🤖 Prompt for AI Agents
.vortex/docs/.prettierignore line 8: the repository currently ignores all .mdx
files from Prettier; confirm intent by either removing the "*.mdx" line if you
want Prettier to format MDX files, or keep it but add a short inline comment
above that line explaining why MDX are excluded (e.g., third-party generated,
formatting breaks content) or narrow the pattern to only specific paths that
should remain unformatted.
| set -eu | ||
| [ "${VORTEX_DEBUG-}" = "1" ] && set -x | ||
|
|
||
| #------------------------------------------------------------------------------- | ||
|
|
||
| # Directory for temporary workspace. | ||
| WORKSPACE_DIR="${WORKSPACE_DIR:-$HOME/www/demo}" | ||
|
|
There was a problem hiding this comment.
Critical: default WORKSPACE_DIR + rm -rf can delete user data. Use mktemp and guarded cleanup.
Avoid defaulting to $HOME/www/demo and only delete dirs created by this run.
Apply:
-set -eu
+set -euo pipefail
@@
-# Directory for temporary workspace.
-WORKSPACE_DIR="${WORKSPACE_DIR:-$HOME/www/demo}"
+# Directory for temporary workspace (set in main; may be overridden via env).
+# WORKSPACE_DIR is not defaulted here to avoid deleting user dirs.
@@
- info "Creating temporary workspace"
- WORKSPACE_DIR="${WORKSPACE_DIR:-"$(mktemp -d -t vortex-installer-recording-XXXXXX)"}"
- [ ! -d "$WORKSPACE_DIR" ] && mkdir -p "$WORKSPACE_DIR"
+ info "Creating temporary workspace"
+ local CREATED_TMPDIR=""
+ if [[ -z "${WORKSPACE_DIR:-}" ]]; then
+ CREATED_TMPDIR="$(mktemp -d -t vortex-installer-recording-XXXXXX)"
+ WORKSPACE_DIR="$CREATED_TMPDIR"
+ else
+ mkdir -p "$WORKSPACE_DIR"
+ fi
@@
- cleanup() {
- info "Cleaning up workspace directory"
- note "Removing: ${WORKSPACE_DIR}"
- rm -rf "$WORKSPACE_DIR"
- }
+ cleanup() {
+ if [[ -n "$CREATED_TMPDIR" && "$WORKSPACE_DIR" == "$CREATED_TMPDIR" ]]; then
+ info "Cleaning up workspace directory"
+ note "Removing: ${WORKSPACE_DIR}"
+ rm -rf "$WORKSPACE_DIR"
+ fi
+ }Also applies to: 372-387
🤖 Prompt for AI Agents
.vortex/docs/.utils/update-installer-video.sh lines 12-19 (and similarly at
372-387): the script currently defaults WORKSPACE_DIR to $HOME/www/demo and
later uses rm -rf which risks deleting user data; change to create a unique
temporary workspace with mktemp (e.g., mktemp -d) when WORKSPACE_DIR is not
explicitly provided, store a flag indicating the script created it, and on exit
only remove the directory if that flag is set (guarded cleanup); also validate
that WORKSPACE_DIR is not empty and not a root/home path before any destructive
operation to prevent accidental deletion.
| "softwareTerms" | ||
| ], | ||
| "words": [ | ||
| "Asciinema", |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add common variants to reduce false positives.
Consider adding lowercase and related terms used in tooling/docs:
"words": [
"Asciinema",
+ "asciinema",
+ "asciicast",
+ "svgterm"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "Asciinema", | |
| "Asciinema", | |
| "asciinema", | |
| "asciicast", | |
| "svgterm" |
🤖 Prompt for AI Agents
.vortex/docs/cspell.json around line 10: the word "Asciinema" is listed but
lacks common variants causing false positives; add lowercase "asciinema" and any
related terms or common capitalizations used in docs/tooling (e.g., "asciicast")
to the cspell words array so spellchecker recognizes these variants
consistently.
| test('loads CSS and JS assets', async () => { | ||
| render(<AsciinemaPlayer src="/fixtures/test-cast.json" />); | ||
|
|
||
| // Wait for useEffect | ||
| await new Promise(resolve => setTimeout(resolve, 100)); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace arbitrary timeouts with waitFor/fake timers to remove flakiness.
setTimeout-based waits are brittle and slow. Prefer Testing Library’s waitFor (or fake timers) to await side effects deterministically.
Example for one test:
- // Wait for useEffect
- await new Promise(resolve => setTimeout(resolve, 100));
+ await waitFor(() =>
+ expect(
+ document.head.querySelector('link[href*="asciinema-player.css"]')
+ ).toBeInTheDocument()
+ );Apply similarly to other tests using setTimeout.
Also applies to: 479-489, 503-504, 552-553, 578-579, 600-601, 631-632, 653-654
🤖 Prompt for AI Agents
.vortex/docs/tests/unit/AsciinemaPlayer/AsciinemaPlayer.test.js around lines
451-456 (and similarly at 479-489, 503-504, 552-553, 578-579, 600-601, 631-632,
653-654): tests use an arbitrary await new Promise(resolve => setTimeout(...))
which is flaky; replace those timeouts by using Testing Library’s waitFor
(import it if needed) to wait for the expected DOM changes or effects (e.g.,
await waitFor(() => expect(...).toBeInTheDocument()) or await waitFor(() => /*
check asset load side-effect */)), or alternatively switch the test to use jest
fake timers and advanceTimersByTime/advanceTimersToNextTimer when appropriate;
apply the same replacement pattern to all listed occurrences so waits are
deterministic and faster.
| test('does not duplicate CSS links', async () => { | ||
| // First render | ||
| render( | ||
| <AsciinemaPlayer | ||
| src="/fixtures/test-cast.json" | ||
| data-testid="player-1" | ||
| /> | ||
| ); | ||
| await new Promise(resolve => setTimeout(resolve, 50)); | ||
|
|
||
| // Second render | ||
| render( | ||
| <AsciinemaPlayer | ||
| src="/fixtures/test-cast.json" | ||
| data-testid="player-2" | ||
| /> | ||
| ); | ||
| await new Promise(resolve => setTimeout(resolve, 50)); | ||
|
|
||
| // Should only have one CSS link | ||
| const cssLinks = document.head.querySelectorAll( | ||
| 'link[href*="asciinema-player.css"]' | ||
| ); | ||
| expect(cssLinks).toHaveLength(1); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Also assert JS bundle is not duplicated.
You verify CSS de-duplication; add a symmetric check for the script tag.
// Should only have one CSS link
const cssLinks = document.head.querySelectorAll(
'link[href*="asciinema-player.css"]'
);
expect(cssLinks).toHaveLength(1);
+
+ // Should only have one JS script
+ const jsScripts = document.head.querySelectorAll(
+ 'script[src*="asciinema-player.min.js"]'
+ );
+ expect(jsScripts).toHaveLength(1);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test('does not duplicate CSS links', async () => { | |
| // First render | |
| render( | |
| <AsciinemaPlayer | |
| src="/fixtures/test-cast.json" | |
| data-testid="player-1" | |
| /> | |
| ); | |
| await new Promise(resolve => setTimeout(resolve, 50)); | |
| // Second render | |
| render( | |
| <AsciinemaPlayer | |
| src="/fixtures/test-cast.json" | |
| data-testid="player-2" | |
| /> | |
| ); | |
| await new Promise(resolve => setTimeout(resolve, 50)); | |
| // Should only have one CSS link | |
| const cssLinks = document.head.querySelectorAll( | |
| 'link[href*="asciinema-player.css"]' | |
| ); | |
| expect(cssLinks).toHaveLength(1); | |
| }); | |
| test('does not duplicate CSS links', async () => { | |
| // First render | |
| render( | |
| <AsciinemaPlayer | |
| src="/fixtures/test-cast.json" | |
| data-testid="player-1" | |
| /> | |
| ); | |
| await new Promise(resolve => setTimeout(resolve, 50)); | |
| // Second render | |
| render( | |
| <AsciinemaPlayer | |
| src="/fixtures/test-cast.json" | |
| data-testid="player-2" | |
| /> | |
| ); | |
| await new Promise(resolve => setTimeout(resolve, 50)); | |
| // Should only have one CSS link | |
| const cssLinks = document.head.querySelectorAll( | |
| 'link[href*="asciinema-player.css"]' | |
| ); | |
| expect(cssLinks).toHaveLength(1); | |
| // Should only have one JS script | |
| const jsScripts = document.head.querySelectorAll( | |
| 'script[src*="asciinema-player.min.js"]' | |
| ); | |
| expect(jsScripts).toHaveLength(1); | |
| }); |
🤖 Prompt for AI Agents
.vortex/docs/tests/unit/AsciinemaPlayer/AsciinemaPlayer.test.js around lines 471
to 495: add a symmetric assertion to ensure the JS bundle isn't duplicated;
after the CSS link check, query document.head for script tags matching the
Asciinema player bundle (e.g. 'script[src*="asciinema-player.js"]' or the exact
bundle name used in the component) and assert the NodeList length is 1 so the
script tag is only injected once.
| expect(links[1]).toHaveAttribute( | ||
| 'href', | ||
| '/features#hosting-integrations' | ||
| ); | ||
| expect(links[2]).toHaveAttribute( | ||
| 'href', | ||
| '/features#continuous-integration' | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Avoid index-based link assertions to reduce fragility.
Select links by accessible name so DOM order changes don’t break tests.
- expect(links[1]).toHaveAttribute(
- 'href',
- '/features#hosting-integrations'
- );
- expect(links[2]).toHaveAttribute(
- 'href',
- '/features#continuous-integration'
- );
+ expect(
+ screen.getByRole('link', { name: 'Hosting' })
+ ).toHaveAttribute('href', '/features#hosting-integrations');
+ expect(
+ screen.getByRole('link', { name: 'Continuous Integration' })
+ ).toHaveAttribute('href', '/features#continuous-integration');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| expect(links[1]).toHaveAttribute( | |
| 'href', | |
| '/features#hosting-integrations' | |
| ); | |
| expect(links[2]).toHaveAttribute( | |
| 'href', | |
| '/features#continuous-integration' | |
| ); | |
| expect( | |
| screen.getByRole('link', { name: 'Hosting' }) | |
| ).toHaveAttribute('href', '/features#hosting-integrations'); | |
| expect( | |
| screen.getByRole('link', { name: 'Continuous Integration' }) | |
| ).toHaveAttribute('href', '/features#continuous-integration'); |
🤖 Prompt for AI Agents
.vortex/docs/tests/unit/Card/Card.integration.test.js around lines 136 to 143:
the test currently asserts links by array indices which is fragile; instead
query the links by accessible name (e.g., getByRole('link', { name: /Hosting
Integrations/i }) and getByRole('link', { name: /Continuous Integration/i }) or
use getByText for visible link text) and assert their hrefs, so DOM order
changes won’t break the test; replace the index-based expects with
role/text-based queries that then assert the same href values.
| const nonClickableCard = screen | ||
| .getByText('Non-clickable Card') | ||
| .closest('div'); | ||
| expect(nonClickableCard.tagName).toBe('DIV'); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Assert “not linked” more directly.
Check absence of an anchor rather than relying on tagName of a parent.
- const nonClickableCard = screen
- .getByText('Non-clickable Card')
- .closest('div');
- expect(nonClickableCard.tagName).toBe('DIV');
+ expect(
+ screen.getByText('Non-clickable Card').closest('a')
+ ).toBeNull();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const nonClickableCard = screen | |
| .getByText('Non-clickable Card') | |
| .closest('div'); | |
| expect(nonClickableCard.tagName).toBe('DIV'); | |
| expect( | |
| screen.getByText('Non-clickable Card').closest('a') | |
| ).toBeNull(); |
🤖 Prompt for AI Agents
.vortex/docs/tests/unit/Card/Card.integration.test.js around lines 218 to 221:
the test currently infers "not linked" by checking the parent element's tagName;
instead assert absence of an anchor element directly. Replace the tagName
assertion with an assertion that there is no <a> descendant for the
"Non-clickable Card" text — e.g., locate the element (or its container) and
assert query for an anchor or role="link" returns null/undefined, ensuring the
test explicitly confirms no link is present.
| expect( | ||
| screen.getByText(/Mixed content:.*42.*array values.*nested elements/) | ||
| ).toBeInTheDocument(); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Make regex assertion more resilient to whitespace/nesting.
getByText with .* can be brittle across nested nodes/newlines. Prefer toHaveTextContent with \s*.
Apply:
-expect(
- screen.getByText(/Mixed content:.*42.*array values.*nested elements/)
-).toBeInTheDocument();
+expect(document.body).toHaveTextContent(
+ /Mixed content:\s*42\s*array values\s*nested elements/i
+);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| expect( | |
| screen.getByText(/Mixed content:.*42.*array values.*nested elements/) | |
| ).toBeInTheDocument(); | |
| }); | |
| expect(document.body).toHaveTextContent( | |
| /Mixed content:\s*42\s*array values\s*nested elements/i | |
| ); | |
| }); |
🤖 Prompt for AI Agents
In .vortex/docs/tests/unit/VerticalTabs/VerticalTabs.test.js around lines 697 to
700, the current assertion uses screen.getByText with a fragile ".*" regex;
instead, grab the element (e.g. const el = screen.getByText(/Mixed content:/) or
an appropriate query) and assert its text using toHaveTextContent with a
whitespace-tolerant regex like /Mixed content:\s*42\s*array values\s*nested
elements/ so the test tolerates newlines and nested nodes.
… and PNG screenshot.
AlexSkrypnyk
force-pushed
the
feature/1951-ascii-cinema
branch
from
f832e49 to
4736e98
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
.vortex/docs/content/tools/drush.mdx (1)
55-60: Tighten phrasing in SSH key instructions.Minor grammar/clarity tweaks in comments.
```shell -ssh-add ~/.ssh/path/to/key # Run on host to run remote Drush commands from host. +ssh-add ~/.ssh/path/to/key # On the host: execute remote Drush commands from the host. # or -pygmy addkey --key ~/.ssh/path/to/key # Run on the host to execute remote Drush commands from inside the container (including ahoy drush). +pygmy addkey --key ~/.ssh/path/to/key # On the host: execute remote Drush commands from inside the container (e.g., via "ahoy drush")..vortex/docs/tests/unit/Card/Card.test.js (2)
356-368: Replace brittle onclick check with explicit “no anchor present” assertion.React’s synthetic events won’t populate element.onclick; the current check can pass even if click handlers exist. Assert absence of an anchor within the card instead.
Apply:
- const cardElement = screen.getByText('Non-clickable Card').closest('div'); - - // Should not have click handlers - expect(cardElement.onclick).toBeFalsy(); + const wrapper = screen.getByText('Non-clickable Card').closest('.card'); + // Should not contain a link + expect(wrapper.querySelector('a')).toBeNull();
280-283: Prefer userEvent for keyboard focus.Use user-event to simulate real keyboard nav instead of programmatic focus.
Apply:
-import { render, screen, fireEvent } from '@testing-library/react'; +import { render, screen, fireEvent } from '@testing-library/react'; +import userEvent from '@testing-library/user-event';- // Should be focusable - linkElement.focus(); + // Should be focusable via keyboard + await userEvent.tab(); expect(document.activeElement).toBe(linkElement);Also applies to: 2-2
.vortex/docs/tests/unit/ContentExtractor/ContentExtractor.test.js (1)
306-327: Handle nested selector matches for startFromSelector/endAtSelectorTests currently only cover direct‐child selectors; any nested match inside the container is ignored. Add unit tests for selectors matching deeper nodes (e.g. an
<h2>wrapped in a<div>) and update ContentExtractor (src/components/ContentExtractor.js, around lines 57–63 and 74–79) to climb from the matched element up to its direct child undercontentDivbefore computing the start/end index:let node = matchElement; while (node && node.parentElement !== contentDiv) node = node.parentElement; const index = node ? workingElements.indexOf(node) : -1;.vortex/docs/content/getting-started/installation.mdx (1)
13-15: Fix article: “in interactive” → “an interactive”.-We automated the first part by providing an in interactive installer. The second +We automated the first part by providing an interactive installer. The second
♻️ Duplicate comments (29)
.vortex/docs/.prettierignore (2)
8-8: Confirm intent to ignore MDX.If intentional, add a brief comment explaining why to avoid future churn; otherwise, drop it so Prettier formats MDX.
Example:
+# MDX formatted by docs toolchain; keep Prettier off. *.mdx
9-9: Broaden ignore to nested fixture JSON.Covers subfolders under tests/fixtures.
-tests/fixtures/*.json +tests/fixtures/**/*.jsonREADME.md (1)
54-55: Silence MD033 or switch to Markdown image.Prefer Markdown syntax for lint cleanliness.
Option A (Markdown):
-<img src=".vortex/docs/static/img/installer.svg" alt="Vortex installer screenshot" /> +Option B (keep HTML, disable rule on this line):
+<!-- markdownlint-disable-next-line MD033 --> <img src=".vortex/docs/static/img/installer.svg" alt="Vortex installer screenshot" />.vortex/docs/tests/unit/VerticalTabs/VerticalTabs.test.js (1)
697-699: Make regex assertion resilient to whitespace/nesting.Prefer toHaveTextContent with a whitespace-tolerant regex to avoid brittle getByText matching across nested nodes/newlines.
Apply:
-expect( - screen.getByText(/Mixed content:.*42.*array values.*nested elements/) -).toBeInTheDocument(); +expect(document.body).toHaveTextContent( + /Mixed content:\s*42\s*array values\s*nested elements/i +);.vortex/docs/cspell.json (1)
10-10: Add common variants to reduce false positives.Include frequent terms used in docs/tooling.
"words": [ "Asciinema", + "asciinema", + "asciicast", + "svgterm",.vortex/docs/tests/unit/Card/Card.integration.test.js (2)
132-143: Avoid index-based link assertions; assert by accessible name.Index-based assertions are fragile to DOM order changes. Use role+name.
Apply:
- const links = screen.getAllByRole('link'); - expect(links).toHaveLength(3); - expect(links[0]).toHaveAttribute('href', '/features#drupal-foundation'); - expect(links[1]).toHaveAttribute( - 'href', - '/features#hosting-integrations' - ); - expect(links[2]).toHaveAttribute( - 'href', - '/features#continuous-integration' - ); + const links = screen.getAllByRole('link'); + expect(links).toHaveLength(3); + expect( + screen.getByRole('link', { name: 'Drupal' }) + ).toHaveAttribute('href', '/features#drupal-foundation'); + expect( + screen.getByRole('link', { name: 'Hosting' }) + ).toHaveAttribute('href', '/features#hosting-integrations'); + expect( + screen.getByRole('link', { name: 'Continuous Integration' }) + ).toHaveAttribute('href', '/features#continuous-integration');
218-221: Assert “not linked” directly, not via tagName.Check absence of an anchor rather than parent tagName.
Apply:
- const nonClickableCard = screen - .getByText('Non-clickable Card') - .closest('div'); - expect(nonClickableCard.tagName).toBe('DIV'); + expect( + screen.getByText('Non-clickable Card').closest('a') + ).toBeNull();.vortex/docs/tests/fixtures/test-cast.json (1)
1-17: Asciinema cast is NDJSON, not JSON — rename or ignore to stop formatter/linter errors.Keep as text: rename to .cast or .jsonl and parse per-line in tests, or add this path to .prettierignore. Current .json triggers Biome parse failures.
Option A (ignore formatters):
# .vortex/docs/.prettierignore +tests/fixtures/*.jsonOption B (rename + load as text in tests):
- Rename file to test-cast.cast (or .jsonl)
- Update test loader (example):
import fs from 'node:fs'; const lines = fs.readFileSync(require.resolve('../../fixtures/test-cast.cast'),'utf8') .trim().split('\n').map(JSON.parse);#!/bin/bash # Verify no JSON imports remain for the cast fixture and that prettier ignore is set. rg -nP "(import|require).*test-cast\.json" -g ".vortex/docs/**" test -f .vortex/docs/.prettierignore && rg -n "tests/fixtures/\*\.json" .vortex/docs/.prettierignore || echo ".prettierignore entry missing".vortex/docs/src/components/AsciinemaPlayer/AsciinemaPlayer.js (2)
31-77: Harden dynamic loader: dedupe scripts, add onerror, async, and clear container before create.Prevents races, silent failures, and duplicate DOM.
- if (!window.AsciinemaPlayer) { - const script = document.createElement('script'); - script.src = - 'https://cdn.jsdelivr.net/npm/asciinema-player@3/dist/bundle/asciinema-player.min.js'; - script.onload = () => { - if (containerRef.current && window.AsciinemaPlayer) { - const options = { - autoPlay, - loop, - preload, - controls, - theme, - }; - - if (poster) { - options.poster = poster; - } - - if (startAt !== undefined) { - options.startAt = startAt; - } - - window.AsciinemaPlayer.create(src, containerRef.current, options); - } - }; - document.head.appendChild(script); - } else { + if (!window.AsciinemaPlayer) { + // Reuse in-flight loader or existing script + if (!window.__asciinemaLoader) { + const existing = document.querySelector('script[src*="asciinema-player.min.js"]'); + window.__asciinemaLoader = new Promise((resolve, reject) => { + const onLoad = () => resolve(); + const onError = () => reject(new Error('Failed to load Asciinema player')); + if (existing) { + existing.addEventListener('load', onLoad, { once: true }); + existing.addEventListener('error', onError, { once: true }); + } else { + const script = document.createElement('script'); + script.src = + 'https://cdn.jsdelivr.net/npm/asciinema-player@3.10.0/dist/bundle/asciinema-player.min.js'; + script.async = true; + script.onload = onLoad; + script.onerror = onError; + document.head.appendChild(script); + } + }); + } + try { + await window.__asciinemaLoader; + } catch (e) { + console.error(e); + return; + } + } else { if (containerRef.current && window.AsciinemaPlayer) { - const options = { - autoPlay, - loop, - preload, - controls, - theme, - }; - - if (poster) { - options.poster = poster; - } - - if (startAt !== undefined) { - options.startAt = startAt; - } - - window.AsciinemaPlayer.create(src, containerRef.current, options); + const options = { + autoPlay, loop, preload, controls, theme, + ...(poster ? { poster } : {}), + ...(startAt !== undefined ? { startAt } : {}), + }; + containerRef.current.innerHTML = ''; + window.AsciinemaPlayer.create(src, containerRef.current, options); } }Also add effect cleanup:
- loadAsciinemaPlayer(); - }, [src, poster, startAt, autoPlay, loop, preload, controls, theme]); + loadAsciinemaPlayer(); + return () => { + if (containerRef.current) containerRef.current.innerHTML = ''; + }; + }, [src, poster, startAt, autoPlay, loop, preload, controls, theme]);
26-29: Pin AsciinemaPlayer CDN assets to v3.10.0
Replace the floating@3specifier with the latest stable patch release to prevent unexpected upgrades.- link.href = - 'https://cdn.jsdelivr.net/npm/asciinema-player@3/dist/bundle/asciinema-player.css'; + link.href = + 'https://cdn.jsdelivr.net/npm/asciinema-player@3.10.0/dist/bundle/asciinema-player.css'; - script.src = - 'https://cdn.jsdelivr.net/npm/asciinema-player@3/dist/bundle/asciinema-player.min.js'; + script.src = + 'https://cdn.jsdelivr.net/npm/asciinema-player@3.10.0/dist/bundle/asciinema-player.min.js';Applies to lines 26–29 and 33–35.
.github/workflows/vortex-test-installer.yml (2)
33-36: Use a moving Node range (or LTS) to reduce churn.Prefer '22.x' (or 'lts/*') over a pinned patch.
- node-version: 22.17.1 + node-version: '22.x'
59-64: Make apt installs deterministic and bound script runtime.Add update, noninteractive, no-recommends, and a timeout for the generator.
- name: Generate video for installer (not used in the final artifact) run: | - sudo apt install asciinema expect - npm i -g svg-term-cli sharp-cli - ./update-installer-video.sh + export DEBIAN_FRONTEND=noninteractive + sudo apt-get update -y + sudo apt-get install -y --no-install-recommends asciinema expect + npm i -g svg-term-cli sharp-cli + timeout 15m bash ./update-installer-video.sh working-directory: .vortex/docs/.utils.github/workflows/vortex-release-docs.yml (1)
135-140: Make apt non-interactive and avoid global npm; guard the long-running script.Use apt-get with update/-y, add a timeout, and prefer npx for tools to avoid global installs.
Apply:
- name: Generate video for installer (not used in the final artifact) run: | - sudo apt install asciinema expect - npm i -g svg-term-cli sharp-cli - ./update-installer-video.sh + export DEBIAN_FRONTEND=noninteractive + sudo apt-get update -y + sudo apt-get install -y --no-install-recommends asciinema expect + # Use npx to avoid global installs; pin majors for stability. + npx -y svg-term-cli@^3.0.0 --help >/dev/null + npx -y sharp-cli@^4.2.0 --help >/dev/null + timeout 15m bash ./update-installer-video.sh working-directory: .vortex/docs/.utils.vortex/docs/content/getting-started/installation.mdx (1)
5-5: Wrap AsciinemaPlayer in BrowserOnly to avoid SSR hydration issues.Prevents access to window/DOM during SSR.
Apply:
+import BrowserOnly from '@docusaurus/BrowserOnly'; -import AsciinemaPlayer from '@site/src/components/AsciinemaPlayer'; +import AsciinemaPlayer from '@site/src/components/AsciinemaPlayer'; @@ - <AsciinemaPlayer - src="/img/installer.json" - poster="npt:0:1" - autoPlay={false} - loop={false} - preload={true} - controls={true} - theme="asciinema" - /> + <BrowserOnly> + {() => ( + <AsciinemaPlayer + src="/img/installer.json" + poster="npt:0:1" + autoPlay={false} + loop={false} + preload={true} + controls={true} + theme="asciinema" + /> + )} + </BrowserOnly>Also applies to: 27-35
.vortex/docs/static/img/installer.json (1)
1-6: Rename cast to .cast or ignore in Biome; update references.This is asciinema v2 JSON Lines; linters treat .json as invalid.
Option A (preferred):
- Rename file to .vortex/docs/static/img/installer.cast.
- Update usages:
- src="/img/installer.json" + src="/img/installer.cast"Option B:
- Add a repository-root .biomeignore entry:
/.vortex/docs/static/img/installer.json#!/bin/bash # Find references to the old JSON path to update them if you choose Option A. rg -n --glob '!.git' '/img/installer\.json' -C2.vortex/docs/tests/unit/AsciinemaPlayer/AsciinemaPlayer.test.js (7)
1-1: Drop unused React import (if using automatic JSX runtime).Removes noise and unused-import warnings.
- import React from 'react';
16-21: Guard: do not initialize player without src (test).Replace this smoke test with an assertion that AsciinemaPlayer.create is not called when src is missing.
- test('renders without src prop', () => { - const { container } = render(<AsciinemaPlayer />); - expect(container.firstChild).toBeInTheDocument(); - expect(container.firstChild.tagName).toBe('DIV'); - }); + test('does not initialize player without src', async () => { + const mockCreate = jest.fn(); + window.AsciinemaPlayer = { create: mockCreate }; + const { container } = render(<AsciinemaPlayer />); + expect(container.firstChild).toBeInTheDocument(); + await Promise.resolve(); + expect(mockCreate).not.toHaveBeenCalled(); + });Follow-up (component change outside this file):
- if (containerRef.current && window.AsciinemaPlayer) { + if (containerRef.current && window.AsciinemaPlayer && src) {
96-160: Collapse repetitive variant tests with test.each.Reduces noise and runtime; a single parameterized table covers these boolean/theme/poster/startAt/combo permutations.
-describe('Prop Variants - Boolean Props', () => { - test('renders with autoPlay true', () => { ... }); - ... -}); +describe('Prop Variants - Boolean Props', () => { + test.each([ + [{ autoPlay: true }], [{ autoPlay: false }], + [{ loop: true }], [{ loop: false }], + [{ controls: true }], [{ controls: false }], + [{ preload: true }], [{ preload: false }], + ])('renders with %p', (props) => { + const { container } = render(<AsciinemaPlayer src="/fixtures/test-cast.json" {...props} />); + expect(container.firstChild).toBeInTheDocument(); + }); +});Apply the same pattern to Theme, Poster, StartAt, and Combinations blocks.
Also applies to: 162-208, 210-242, 244-264, 266-359
361-374: Add SSR test: no DOM injection when window is undefined.Covers the early-return branch.
test('does nothing when window is undefined (SSR)', async () => { const realWindow = global.window; // @ts-ignore delete global.window; render(<AsciinemaPlayer src="/fixtures/test-cast.json" />); expect(document.head.querySelector('link[href*="asciinema-player.css"]')).toBeNull(); expect(document.head.querySelector('script[src*="asciinema-player.min.js"]')).toBeNull(); global.window = realWindow; });
439-449: Move cleanup to afterEach; keep beforeEach minimal.Ensures teardown runs even when a test fails.
-describe('Library Integration', () => { - beforeEach(() => { +describe('Library Integration', () => { + beforeEach(() => { if (typeof window !== 'undefined' && window.AsciinemaPlayer) { delete window.AsciinemaPlayer; } if (typeof document !== 'undefined') { document.head.innerHTML = ''; } jest.clearAllMocks(); - }); + }); + afterEach(() => { + jest.restoreAllMocks(); + if (typeof window !== 'undefined' && window.AsciinemaPlayer) { + delete window.AsciinemaPlayer; + } + document.head.innerHTML = ''; + });
451-456: Replace arbitrary setTimeout waits with waitFor (or fake timers).Removes flakiness and speeds up tests. Add waitFor import.
- import { render, screen } from '@testing-library/react'; + import { render, screen, waitFor } from '@testing-library/react';Example change (apply similarly to other occurrences listed):
- // Wait for useEffect - await new Promise(resolve => setTimeout(resolve, 100)); + await waitFor(() => + expect( + document.head.querySelector('link[href*="asciinema-player.css"]') + ).toBeInTheDocument() + );Also applies to: 479-489, 503-504, 552-553, 578-579, 600-601, 631-632, 653-654
471-495: Also assert JS bundle is not duplicated.Symmetric to the CSS check.
const cssLinks = document.head.querySelectorAll( 'link[href*="asciinema-player.css"]' ); expect(cssLinks).toHaveLength(1); + const jsScripts = document.head.querySelectorAll( + 'script[src*="asciinema-player.min.js"]' + ); + expect(jsScripts).toHaveLength(1);.vortex/docs/.utils/update-installer-video.sh (7)
50-69: Add node to dependency checks and update install hints.npx depends on node; also provide CI-friendly hints.
- local required_commands=("asciinema" "expect" "php" "composer" "npx") + local required_commands=("asciinema" "expect" "php" "composer" "node" "npx") @@ - echo " brew install asciinema expect composer # macOS" >&2 - echo " apt-get install asciinema expect-dev composer # Ubuntu/Debian" >&2 + echo " brew install asciinema expect composer node # macOS" >&2 + echo " apt-get install asciinema expect-dev composer nodejs npm # Ubuntu/Debian" >&2
95-105: Harden composer commands for CI (non-interactive/quiet).Prevents prompts and reduces log noise.
- if ! composer install; then + if ! composer install --no-interaction --no-ansi --no-progress; then- if ! composer build; then + if ! composer build -n; then
311-319: Make sed replacement portable and safer.Handle both /Users and /home paths; keep -i portability.
- sed_opts=(-i) && [ "$(uname)" = "Darwin" ] && sed_opts=(-i '') - sed "${sed_opts[@]}" -E 's|/Users/[^/]+/|/home/user/|g' "$output_cast" + sed_opts=(-i) && [ "$(uname)" = "Darwin" ] && sed_opts=(-i '') + sed "${sed_opts[@]}" -E 's|/(Users|home)/[^/]+/|/home/user/|g' "$output_cast"
328-344: Make all npx invocations non-interactive (-y) for CI.Avoid stalls on “Need to install…” prompts.
- if ! npx svg-term --out="$output_svg" <"$output_cast"; then + if ! npx -y svg-term --out="$output_svg" <"$output_cast"; then- if ! npx svg-term --at=1000 --out="$output_png.svg" <"$output_cast"; then + if ! npx -y svg-term --at=1000 --out="$output_png.svg" <"$output_cast"; then- if ! npx sharp-cli -i "$output_png.svg" -o "$output_png" -f png resize 1280; then + if ! npx -y sharp-cli -i "$output_png.svg" -o "$output_png" -f png resize 1280; thenAlso applies to: 338-346
184-191: Expect script bug: replace sleep with Tcl after.sleep isn’t an Expect/Tcl command here; use after with ms conversion.
-proc wait_and_enter {} { +proc wait_and_enter {} { global prompt_delay wait_for_quiet 0.25 - sleep $prompt_delay + after [expr {int($prompt_delay * 1000)}] safe_send "\r" }
423-429: Ensure destination directories exist before copying outputs.Prevents failures when static dirs are missing.
- note "JSON cast: $temp_cast → $CAST_FILE" - cp "$temp_cast" "$CAST_FILE" + note "JSON cast: $temp_cast → $CAST_FILE" + mkdir -p "$(dirname "$CAST_FILE")" + cp "$temp_cast" "$CAST_FILE" @@ - note "SVG image: $temp_svg → $SVG_FILE" - cp "$temp_svg" "$SVG_FILE" + note "SVG image: $temp_svg → $SVG_FILE" + mkdir -p "$(dirname "$SVG_FILE")" + cp "$temp_svg" "$SVG_FILE" @@ - note "PNG image: $temp_png → $PNG_FILE" - cp "$temp_png" "$PNG_FILE" + note "PNG image: $temp_png → $PNG_FILE" + mkdir -p "$(dirname "$PNG_FILE")" + cp "$temp_png" "$PNG_FILE"
12-12: Enable safer bash options.Add pipefail and nounset is already covered; keeps failures from being masked in pipelines.
-set -eu +set -euo pipefail
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
.vortex/docs/static/img/installer.pngis excluded by!**/*.png.vortex/docs/static/img/installer.svgis excluded by!**/*.svg
📒 Files selected for processing (27)
.github/workflows/vortex-release-docs.yml(1 hunks).github/workflows/vortex-test-installer.yml(2 hunks).vortex/.ahoy.yml(1 hunks).vortex/docs/.gitignore(1 hunks).vortex/docs/.prettierignore(1 hunks).vortex/docs/.utils/update-installer-video.sh(1 hunks).vortex/docs/content/contributing/maintenance/release.mdx(1 hunks).vortex/docs/content/getting-started/installation.mdx(2 hunks).vortex/docs/content/tools/drush.mdx(1 hunks).vortex/docs/content/workflows/deployment.mdx(1 hunks).vortex/docs/cspell.json(1 hunks).vortex/docs/package.json(1 hunks).vortex/docs/src/components/AsciinemaPlayer/AsciinemaPlayer.js(1 hunks).vortex/docs/src/components/AsciinemaPlayer/index.js(1 hunks).vortex/docs/static/img/installer.json(1 hunks).vortex/docs/tests/fixtures/test-cast.json(1 hunks).vortex/docs/tests/jest-setup.js(1 hunks).vortex/docs/tests/unit/AsciinemaPlayer/AsciinemaPlayer.test.js(1 hunks).vortex/docs/tests/unit/Card/Card.integration.test.js(10 hunks).vortex/docs/tests/unit/Card/Card.test.js(16 hunks).vortex/docs/tests/unit/Card/CardGrid.test.js(10 hunks).vortex/docs/tests/unit/Card/index.test.js(1 hunks).vortex/docs/tests/unit/ContentExtractor/ContentExtractor.test.js(22 hunks).vortex/docs/tests/unit/VerticalTabs/MarkerComponents.test.js(2 hunks).vortex/docs/tests/unit/VerticalTabs/VerticalTabs.test.js(6 hunks).vortex/docs/tests/unit/VerticalTabs/index.test.js(2 hunks)README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-05-29T12:15:32.188Z
Learnt from: AlexSkrypnyk
PR: drevops/vortex#0
File: :0-0
Timestamp: 2025-05-29T12:15:32.188Z
Learning: Do not review files in `.vortex/installer/tests/Fixtures/install` directory as they are test fixtures.
Applied to files:
.vortex/docs/.gitignore.vortex/docs/.prettierignore
📚 Learning: 2025-06-02T00:41:36.321Z
Learnt from: AlexSkrypnyk
PR: drevops/vortex#1697
File: .vortex/installer/tests/Fixtures/install/provision_database_lagoon/docs/onboarding.md:16-17
Timestamp: 2025-06-02T00:41:36.321Z
Learning: DO NOT EVER REVIEW FILES IN .vortex/installer/tests/Fixtures/ directory - these are test fixtures and should not be reviewed under any circumstances.
Applied to files:
.vortex/docs/.prettierignore
📚 Learning: 2025-06-01T08:09:18.993Z
Learnt from: AlexSkrypnyk
PR: drevops/vortex#1693
File: .vortex/installer/tests/Fixtures/install/hosting_acquia/docs/deployment.md:6-8
Timestamp: 2025-06-01T08:09:18.993Z
Learning: Do not review files in test fixture directories (e.g., files under paths containing `/tests/Fixtures/` or similar test fixture patterns). These files are used for testing purposes and should remain as-is.
Applied to files:
.vortex/docs/.prettierignore
📚 Learning: 2025-06-01T08:08:59.071Z
Learnt from: AlexSkrypnyk
PR: drevops/vortex#1693
File: .vortex/installer/tests/Fixtures/install/deploy_type_none_gha/.github/workflows/build-test-deploy.yml:1-4
Timestamp: 2025-06-01T08:08:59.071Z
Learning: Test fixture files (typically located in directories containing "fixture" or "test") should not be reviewed for code quality issues as they may intentionally contain incomplete, invalid, or malformed data for testing purposes.
Applied to files:
.vortex/docs/.prettierignore
🧬 Code graph analysis (4)
.vortex/docs/tests/unit/VerticalTabs/MarkerComponents.test.js (2)
.vortex/docs/src/components/VerticalTabs/VerticalTab.js (1)
VerticalTab(7-10).vortex/docs/src/components/VerticalTabs/VerticalTabPanel.js (1)
VerticalTabPanel(7-10)
.vortex/docs/tests/unit/AsciinemaPlayer/AsciinemaPlayer.test.js (1)
.vortex/docs/src/components/AsciinemaPlayer/AsciinemaPlayer.js (1)
AsciinemaPlayer(3-87)
.vortex/docs/tests/unit/VerticalTabs/index.test.js (3)
.vortex/docs/src/components/VerticalTabs/VerticalTabs.js (1)
VerticalTabs(83-210).vortex/docs/src/components/VerticalTabs/VerticalTab.js (1)
VerticalTab(7-10).vortex/docs/src/components/VerticalTabs/VerticalTabPanel.js (1)
VerticalTabPanel(7-10)
.vortex/docs/tests/unit/ContentExtractor/ContentExtractor.test.js (1)
.vortex/docs/src/components/ContentExtractor.js (1)
ContentExtractor(29-135)
🪛 markdownlint-cli2 (0.17.2)
README.md
54-54: Inline HTML
Element: img
(MD033, no-inline-html)
🪛 LanguageTool
.vortex/docs/content/getting-started/installation.mdx
[grammar] ~5-~5: Use correct spacing
Context: ... '@site/src/components/AsciinemaPlayer'; # Installation The installation process c...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🪛 Biome (2.1.2)
.vortex/docs/static/img/installer.json
[error] 1-4: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 4-4: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 4-6: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 6-6: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 6-8: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 8-8: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 8-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-36: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 36-36: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 36-38: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 38-38: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 38-38: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 38-40: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 40-44: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 44-44: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 44-46: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 46-46: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 46-46: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 46-49: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 49-49: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 49-50: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 50-51: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 51-56: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 56-56: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 56-56: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 56-56: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 56-59: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 59-59: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 59-62: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 62-62: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 62-62: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 62-65: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 65-68: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 68-68: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 68-68: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 68-70: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 70-70: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 70-70: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 70-71: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 71-71: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 71-75: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 75-77: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 77-77: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 77-77: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 77-80: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 80-80: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 80-80: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 80-82: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 82-82: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 82-84: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 84-86: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 86-86: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 86-89: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 89-91: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 91-94: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 94-94: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 94-94: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 94-96: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 96-96: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 96-97: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 97-100: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 100-100: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 100-102: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 102-102: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 102-105: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 105-105: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 105-107: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 107-107: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 107-109: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 109-109: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 109-111: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 111-111: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 111-111: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 111-112: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 112-114: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 114-114: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 114-114: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 114-116: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 116-120: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 120-122: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 122-122: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 122-122: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 122-124: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 124-125: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 125-135: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 135-136: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 136-137: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 137-146: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 146-146: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 146-150: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 150-150: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 150-151: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 151-151: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
.vortex/docs/tests/fixtures/test-cast.json
[error] 1-2: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 2-3: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 3-4: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 4-5: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 5-6: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 6-7: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 7-8: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 8-9: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 9-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 10-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-docs
🔇 Additional comments (23)
.vortex/docs/content/workflows/deployment.mdx (1)
32-33: LGTM: spacing fix improves list rendering..vortex/docs/.gitignore (1)
24-26: LGTM: clarified intent for installer artifact.Comment makes it clear this is a file, not a directory.
.vortex/docs/tests/unit/Card/index.test.js (1)
33-33: LGTM: add trailing newline.Conforms to standard formatting.
.vortex/docs/tests/unit/VerticalTabs/MarkerComponents.test.js (1)
31-34: Whitespace-only cleanup — OK.Formatting changes look good and keep tests readable.
Also applies to: 65-69, 72-72
.vortex/docs/tests/unit/VerticalTabs/VerticalTabs.test.js (6)
2-8: Reformatted imports — OK.Multi-line import improves readability without behavior change.
448-450: Readability tweak — OK.Line-wrapped query/closest chain is fine.
505-508: Assertion formatting — OK.Expanded expect() args for clarity.
546-549: Readability tweak — OK.Line-wrapped query/closest chain is fine.
646-649: Assertion formatting — OK.Expanded expect() args for clarity.
655-658: Assertion formatting — OK.Expanded expect() args for clarity.
.vortex/docs/tests/jest-setup.js (1)
8-22: Whitespace-only reformat — OK.No functional changes to the EventTarget polyfill.
.vortex/docs/tests/unit/Card/Card.test.js (1)
20-22: Formatting-only changes look good.Multiline wrapping and JSX inlining do not alter behavior. Nice consistency.
Also applies to: 129-132, 142-145, 179-186, 191-198, 229-229, 251-254, 340-340, 404-406
.vortex/docs/tests/unit/Card/Card.integration.test.js (1)
46-48: LGTM on reflow/formatting.No behavioral differences detected.
Also applies to: 67-67, 170-173, 229-229, 251-254, 340-340, 404-406, 417-417
.vortex/docs/tests/unit/VerticalTabs/index.test.js (1)
1-1: Whitespace-only changes — OK.Exports and identifiers checks remain intact.
Also applies to: 14-20
.vortex/docs/tests/unit/ContentExtractor/ContentExtractor.test.js (1)
10-12: Formatting-only changes — OK.Readability improved; behavior unchanged.
Also applies to: 68-76, 113-116, 136-139, 156-161, 176-181, 199-205, 222-227, 236-240, 251-257, 270-275, 292-303, 316-321, 344-351, 355-362, 389-401, 414-422, 521-533
.vortex/docs/tests/unit/Card/CardGrid.test.js (2)
61-69: LGTM: inline Card props cleanup keeps behavior unchanged.Also applies to: 112-113
308-313: LGTM: conditional children coverage is clear and robust..vortex/docs/src/components/AsciinemaPlayer/index.js (1)
1-1: LGTM: barrel export is correct..vortex/docs/content/contributing/maintenance/release.mdx (1)
17-18: Release steps update reads well; grammar fix applied.Step 10 addition and Step 11 wording look correct and actionable.
.vortex/docs/.utils/update-installer-video.sh (4)
300-349: Optional: verify cast existence before conversions and propagate errors.You already check $output_cast; this looks good. No action needed.
351-366: CLI help looks good.Clear usage and options.
398-421: File presence verification is solid.Good defensive checks before install step.
50-69: No interactive npx calls without -y detected Allnpxinvocations across the repository include the-yflag, so this concern is resolved.
| - name: Generate video for installer (not used in the final artifact) | ||
| run: | | ||
| sudo apt install asciinema expect | ||
| npm i -g svg-term-cli sharp-cli |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Pin CLI tool versions or use npx for reproducibility.
Avoid floating globals; pin versions or run via npx with explicit versions.
- npm i -g svg-term-cli sharp-cli
+ npm i -g svg-term-cli@<version> sharp-cli@<version>
+ # or:
+ npx -y svg-term-cli@<version> --help >/dev/null
+ npx -y sharp-cli@<version> --help >/dev/null🌐 Web query:
What are the latest stable versions of svg-term-cli and sharp-cli on npm?
💡 Result:
- svg-term-cli: 2.1.1 (last published 8 years ago). [1]
- sharp-cli: 5.2.0 (last published ~2 months ago). [2]
Sources:
[1] npm: svg-term-cli.
[2] npm: sharp-cli.
Pin CLI tool versions for reproducibility.
Use explicit versions rather than floating globals:
- npm i -g svg-term-cli sharp-cli
+ npm i -g svg-term-cli@2.1.1 sharp-cli@5.2.0
+ # or via npx:
+ npx -y svg-term-cli@2.1.1 --help >/dev/null
+ npx -y sharp-cli@5.2.0 --help >/dev/null📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| npm i -g svg-term-cli sharp-cli | |
| npm i -g svg-term-cli@2.1.1 sharp-cli@5.2.0 | |
| # or via npx: | |
| npx -y svg-term-cli@2.1.1 --help >/dev/null | |
| npx -y sharp-cli@5.2.0 --help >/dev/null |
🤖 Prompt for AI Agents
.github/workflows/vortex-test-installer.yml around line 62: the workflow
installs CLI tools globally with floating versions which harms reproducibility;
update the npm install line to pin explicit versions (e.g.
svg-term-cli@<version> and sharp-cli@<version>) or reference variables/inputs
for those versions, commit the chosen version numbers, and ensure any related
cache or lockfile is updated so CI uses the exact tool versions consistently.
| update-installer-video: | ||
| usage: Update installer video in the documentation. | ||
| cmd: | | ||
| cd docs/.utils | ||
| ./update-installer-video.sh | ||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add pre-flight checks before running the updater script.
Fail fast if the script or required tools are missing; keep output actionable.
update-installer-video:
usage: Update installer video in the documentation.
cmd: |
- cd docs/.utils
- ./update-installer-video.sh
+ cd docs/.utils
+ test -x ./update-installer-video.sh || { echo "ERROR: docs/.utils/update-installer-video.sh is missing or not executable"; exit 1; }
+ bash -n ./update-installer-video.sh
+ for bin in asciinema expect php composer node; do
+ command -v "$bin" >/dev/null 2>&1 || { echo "ERROR: '$bin' not found in PATH"; exit 1; }
+ done
+ ./update-installer-video.shWant me to wire similar checks into the script itself instead?
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| update-installer-video: | |
| usage: Update installer video in the documentation. | |
| cmd: | | |
| cd docs/.utils | |
| ./update-installer-video.sh | |
| update-installer-video: | |
| usage: Update installer video in the documentation. | |
| cmd: | | |
| cd docs/.utils | |
| test -x ./update-installer-video.sh || { echo "ERROR: docs/.utils/update-installer-video.sh is missing or not executable"; exit 1; } | |
| bash -n ./update-installer-video.sh | |
| for bin in asciinema expect php composer node; do | |
| command -v "$bin" >/dev/null 2>&1 || { echo "ERROR: '$bin' not found in PATH"; exit 1; } | |
| done | |
| ./update-installer-video.sh |
🤖 Prompt for AI Agents
In .vortex/.ahoy.yml around lines 110 to 115, the updater step runs the script
directly; add pre-flight checks to fail fast if prerequisites are missing by
first verifying the docs/.utils directory exists, that
./update-installer-video.sh is present and executable, and that any required
tools (e.g., bash, git, ffmpeg — list the actual tools your script needs) are
available on PATH; update the cmd block to run these checks and emit clear,
actionable error messages and non-zero exit codes before invoking the script so
the step stops early on missing files or tools.
| # Directory for temporary workspace. | ||
| WORKSPACE_DIR="${WORKSPACE_DIR:-$HOME/www/demo}" | ||
|
|
There was a problem hiding this comment.
Critical: defaulting WORKSPACE_DIR into $HOME and unconditional rm -rf risks data loss.
Create a mktemp workspace when WORKSPACE_DIR is unset, and only delete if we created it.
-# Directory for temporary workspace.
-WORKSPACE_DIR="${WORKSPACE_DIR:-$HOME/www/demo}"
+# Directory for temporary workspace (set in main; may be overridden via env).
+# Intentionally not defaulted here to avoid deleting user directories.
+# WORKSPACE_DIR will be initialized in main().- info "Creating temporary workspace"
- WORKSPACE_DIR="${WORKSPACE_DIR:-"$(mktemp -d -t vortex-installer-recording-XXXXXX)"}"
- [ ! -d "$WORKSPACE_DIR" ] && mkdir -p "$WORKSPACE_DIR"
+ info "Creating temporary workspace"
+ local CREATED_TMPDIR=""
+ if [[ -z "${WORKSPACE_DIR:-}" ]]; then
+ CREATED_TMPDIR="$(mktemp -d -t vortex-installer-recording-XXXXXX)"
+ WORKSPACE_DIR="$CREATED_TMPDIR"
+ else
+ mkdir -p "$WORKSPACE_DIR"
+ fi
@@
- cleanup() {
- info "Cleaning up workspace directory"
- note "Removing: ${WORKSPACE_DIR}"
- rm -rf "$WORKSPACE_DIR"
- }
+ cleanup() {
+ if [[ -n "$CREATED_TMPDIR" && "$WORKSPACE_DIR" == "$CREATED_TMPDIR" ]]; then
+ info "Cleaning up workspace directory"
+ note "Removing: ${WORKSPACE_DIR}"
+ rm -rf "$WORKSPACE_DIR"
+ fi
+ }Also applies to: 377-391
| # Local installer paths | ||
| INSTALLER_DOCS="${DOCS_DIR}/static/install" | ||
| INSTALLER_BUILD="${PROJECT_ROOT}/.vortex/installer/build/installer.phar" | ||
| INSTALLER_SOURCE_DIR="${PROJECT_ROOT}/.vortex/installer" | ||
|
|
There was a problem hiding this comment.
Fix installer path mixup: cp directory to file is broken. Introduce DIR/FILE vars and guard.
Currently INSTALLER_DOCS points to a directory but later is copied as a file. Define INSTALLER_DOCS_DIR and INSTALLER_DOCS_FILE, ensure dirs exist, and copy correctly.
-# Local installer paths
-INSTALLER_DOCS="${DOCS_DIR}/static/install"
-INSTALLER_BUILD="${PROJECT_ROOT}/.vortex/installer/build/installer.phar"
+# Local installer paths
+INSTALLER_DOCS_DIR="${DOCS_DIR}/static/install"
+INSTALLER_DOCS_FILE="${INSTALLER_DOCS_DIR}/installer.php"
+INSTALLER_BUILD="${PROJECT_ROOT}/.vortex/installer/build/installer.phar"- note "Target: $INSTALLER_DOCS"
- cp "$INSTALLER_BUILD" "$INSTALLER_DOCS"
+ note "Target: $INSTALLER_DOCS_FILE"
+ mkdir -p "$(dirname "$INSTALLER_DOCS_FILE")"
+ cp "$INSTALLER_BUILD" "$INSTALLER_DOCS_FILE"- note "Target: $INSTALLER_DOCS"
- cp "$INSTALLER_BUILD" "$INSTALLER_DOCS"
+ note "Target: $INSTALLER_DOCS_FILE"
+ mkdir -p "$(dirname "$INSTALLER_DOCS_FILE")"
+ cp "$INSTALLER_BUILD" "$INSTALLER_DOCS_FILE"- note "Source: $INSTALLER_DOCS"
- note "Target: $workspace_dir/installer.php"
- cp "$INSTALLER_DOCS" "$workspace_dir/installer.php"
+ note "Source: $INSTALLER_DOCS_FILE"
+ note "Target: $workspace_dir/installer.php"
+ if [ ! -f "$INSTALLER_DOCS_FILE" ]; then
+ fail "Installer not found at: $INSTALLER_DOCS_FILE"
+ return 1
+ fi
+ cp "$INSTALLER_DOCS_FILE" "$workspace_dir/installer.php"- note " - Static: $INSTALLER_DOCS"
+ note " - Static: $INSTALLER_DOCS_FILE"Also applies to: 76-83, 111-120, 286-290, 129-131
🤖 Prompt for AI Agents
.vortex/docs/.utils/update-installer-video.sh around lines 37-41: INSTALLER_DOCS
is currently a path used inconsistently as a directory in some places and a file
in others; define explicit INSTALLER_DOCS_DIR and INSTALLER_DOCS_FILE (or
INSTALLER_BUILD_FILE) variables, ensure target directories exist with mkdir -p,
guard copies with test -f for files and test -d for directories, and use cp with
the correct source->destination semantics (file to file, directory to
directory). Apply the same pattern and guards to the other affected ranges
(lines 76-83, 111-120, 129-131, 286-290): introduce DIR/FILE vars, create any
needed dirs, check file existence before copying, and copy to the correct
destination type.
| "lint-js": "eslint src/ tests/ && prettier --check src/ tests/", | ||
| "lint-js-fix": "eslint src/ tests/ --fix && prettier --write src/ tests/", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Including tests in lint is good; ensure ESLint knows about Jest.
Without Jest/Test Library rules or env, linting tests may produce false positives.
- Consider switching non-mutating lint to use lint-docs (not lint-docs-fix).
- Add Jest and Testing Library ESLint config for tests.
Apply:
- "lint": "yarn lint-js && yarn lint-docs-fix",
+ "lint": "yarn lint-js && yarn lint-docs",
- "lint-js": "eslint src/ tests/ && prettier --check src/ tests/",
- "lint-js-fix": "eslint src/ tests/ --fix && prettier --write src/ tests/",
+ "lint-js": "eslint src/ tests/ --max-warnings=0 && prettier --check src/ tests/",
+ "lint-js-fix": "eslint src/ tests/ --fix && prettier --write src/ tests/",Add (in your ESLint config):
{
"overrides": [
{
"files": ["**/tests/**/*.{js,jsx,ts,tsx}"],
"env": { "jest": true },
"plugins": ["testing-library", "jest"],
"extends": ["plugin:testing-library/react", "plugin:jest/recommended"]
}
]
}If needed, add devDependencies:
- eslint-plugin-jest
- eslint-plugin-testing-library
Verification script:
🏁 Script executed:
#!/bin/bash
# Check ESLint config for Jest/testing-library overrides
fd -H -a ".eslintrc*" . | while read -r f; do
echo "== $f =="; sed -n '1,200p' "$f"
done
# Grep for jest env in any ESLint config
rg -nP '"jest"\s*:\s*true' -S || echo "No explicit jest env found."Length of output: 6319
🏁 Script executed:
#!/bin/bash
# Display the scripts section in .vortex/docs/package.json
sed -n '1,200p' .vortex/docs/package.jsonLength of output: 2417
Tighten lint scripts and add ESLint plugins for tests
- In .vortex/docs/package.json
• Change"lint"fromto"lint": "yarn lint-js && yarn lint-docs-fix"
• Update"lint": "yarn lint-js && yarn lint-docs"
"lint-js"to fail on warnings:(keep- "lint-js": "eslint src/ tests/ && prettier --check src/ tests/" + "lint-js": "eslint src/ tests/ --max-warnings=0 && prettier --check src/ tests/"
"lint-js-fix"unchanged) - In .vortex/docs/.eslintrc.js, add an overrides block for your tests:
overrides: [ { files: ["**/tests/**/*.{js,jsx,ts,tsx}"], plugins: ["jest", "testing-library"], extends: ["plugin:jest/recommended", "plugin:testing-library/react"] } ]
- Add these to devDependencies in .vortex/docs/package.json:
eslint-plugin-jest eslint-plugin-testing-library
| const options = { | ||
| autoPlay, | ||
| loop, | ||
| preload, | ||
| controls, | ||
| theme, | ||
| }; | ||
|
|
||
| if (poster) { | ||
| options.poster = poster; | ||
| } | ||
|
|
||
| if (startAt !== undefined) { | ||
| options.startAt = startAt; | ||
| } | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Deduplicate options building.
Build once with conditional spreads; less drift.
- const options = {
- autoPlay,
- loop,
- preload,
- controls,
- theme,
- };
- if (poster) {
- options.poster = poster;
- }
- if (startAt !== undefined) {
- options.startAt = startAt;
- }
+ const options = {
+ autoPlay, loop, preload, controls, theme,
+ ...(poster ? { poster } : {}),
+ ...(startAt !== undefined ? { startAt } : {}),
+ };(and same in the already-loaded branch)
Also applies to: 59-76
🤖 Prompt for AI Agents
.vortex/docs/src/components/AsciinemaPlayer/AsciinemaPlayer.js around lines 37
to 52: the options object is built twice with conditional assignments (poster,
startAt); consolidate into a single options construction using conditional
spreads so the base properties (autoPlay, loop, preload, controls, theme) are
set once and poster/startAt are merged only when present; apply the same
refactor to the already-loaded branch around lines 59 to 76 to remove duplicated
logic and keep a single source of truth for options.
| test('renders with undefined props', () => { | ||
| const { container } = render( | ||
| <AsciinemaPlayer | ||
| src={undefined} | ||
| poster={undefined} | ||
| startAt={undefined} | ||
| theme={undefined} | ||
| /> | ||
| ); | ||
|
|
||
| expect(container.firstChild).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| test('renders with empty string props', () => { | ||
| const { container } = render( | ||
| <AsciinemaPlayer src="" poster="" theme="" className="" /> | ||
| ); | ||
|
|
||
| expect(container.firstChild).toBeInTheDocument(); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Edge-cases: assert create is not called for falsy/empty src.
Tightens behavior for '', null, undefined inputs.
test.each([undefined, null, ''])('does not initialize player when src=%p', async (badSrc) => {
const mockCreate = jest.fn();
window.AsciinemaPlayer = { create: mockCreate };
render(<AsciinemaPlayer src={badSrc} />);
await Promise.resolve();
expect(mockCreate).not.toHaveBeenCalled();
});🤖 Prompt for AI Agents
.vortex/docs/tests/unit/AsciinemaPlayer/AsciinemaPlayer.test.js around lines 375
to 394: add a parameterized test that verifies the player is not initialized for
falsy/empty src values by creating a jest.fn() mock for
window.AsciinemaPlayer.create, assigning it to window.AsciinemaPlayer, rendering
<AsciinemaPlayer src={badSrc} /> for each of [undefined, null, ''], awaiting a
microtask (e.g. await Promise.resolve()) to let any effects run, and asserting
mockCreate was not called; ensure you restore or delete window.AsciinemaPlayer
after the test to avoid leaking globals.
| test('handles errors during library loading', async () => { | ||
| const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); | ||
|
|
||
| // Force an error by making querySelector throw | ||
| const originalQuerySelector = document.querySelector; | ||
| document.querySelector = jest.fn().mockImplementation(() => { | ||
| throw new Error('Mock error'); | ||
| }); | ||
|
|
||
| const { container } = render( | ||
| <AsciinemaPlayer src="/fixtures/test-cast.json" /> | ||
| ); | ||
|
|
||
| await new Promise(resolve => setTimeout(resolve, 50)); | ||
|
|
||
| // Should still render the container | ||
| expect(container.firstChild).toBeInTheDocument(); | ||
|
|
||
| // Should log the error | ||
| expect(consoleSpy).toHaveBeenCalledWith( | ||
| 'Failed to load Asciinema player:', | ||
| expect.any(Error) | ||
| ); | ||
|
|
||
| // Restore | ||
| document.querySelector = originalQuerySelector; | ||
| consoleSpy.mockRestore(); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Ensure document.querySelector override is restored on failure.
Wrap in try/finally to avoid leaking patched globals if an assertion fails.
- const consoleSpy = jest.spyOn(console, 'error').mockImplementation();
- const originalQuerySelector = document.querySelector;
- document.querySelector = jest.fn().mockImplementation(() => { throw new Error('Mock error'); });
- const { container } = render(<AsciinemaPlayer src="/fixtures/test-cast.json" />);
- await new Promise(resolve => setTimeout(resolve, 50));
- expect(container.firstChild).toBeInTheDocument();
- expect(consoleSpy).toHaveBeenCalledWith('Failed to load Asciinema player:', expect.any(Error));
- document.querySelector = originalQuerySelector;
- consoleSpy.mockRestore();
+ const consoleSpy = jest.spyOn(console, 'error').mockImplementation();
+ const originalQuerySelector = document.querySelector;
+ try {
+ document.querySelector = jest.fn().mockImplementation(() => { throw new Error('Mock error'); });
+ const { container } = render(<AsciinemaPlayer src="/fixtures/test-cast.json" />);
+ await waitFor(() => expect(container.firstChild).toBeInTheDocument());
+ expect(consoleSpy).toHaveBeenCalledWith('Failed to load Asciinema player:', expect.any(Error));
+ } finally {
+ document.querySelector = originalQuerySelector;
+ consoleSpy.mockRestore();
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .vortex/docs/tests/unit/AsciinemaPlayer/AsciinemaPlayer.test.js around lines
618 to 645, the test overrides document.querySelector and mocks console.error
but restores them only after assertions, so a failing assertion can leak patched
globals; wrap the body of the test in a try/finally and in the finally block
restore document.querySelector to the originalQuerySelector and call
consoleSpy.mockRestore() (and any other cleanup) to guarantee restoration even
if the test throws.
| const { container } = render(<CardGrid>{null}</CardGrid>); | ||
|
|
||
| const gridElement = container.querySelector('.cards-grid'); | ||
| expect(gridElement).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| test('handles undefined children', () => { | ||
| const { container } = render( | ||
| <CardGrid> | ||
| {undefined} | ||
| </CardGrid> | ||
| ); | ||
| const { container } = render(<CardGrid>{undefined}</CardGrid>); | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Strengthen “null/undefined children” assertions.
Also assert there are zero element children to prove no stray nodes rendered.
-const { container } = render(<CardGrid>{null}</CardGrid>);
+const { container } = render(<CardGrid>{null}</CardGrid>);
const gridElement = container.querySelector('.cards-grid');
expect(gridElement).toBeInTheDocument();
+expect(gridElement.children.length).toBe(0);And similarly for the undefined case:
-const { container } = render(<CardGrid>{undefined}</CardGrid>);
+const { container } = render(<CardGrid>{undefined}</CardGrid>);
const gridElement = container.querySelector('.cards-grid');
expect(gridElement).toBeInTheDocument();
+expect(gridElement.children.length).toBe(0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { container } = render(<CardGrid>{null}</CardGrid>); | |
| const gridElement = container.querySelector('.cards-grid'); | |
| expect(gridElement).toBeInTheDocument(); | |
| }); | |
| test('handles undefined children', () => { | |
| const { container } = render( | |
| <CardGrid> | |
| {undefined} | |
| </CardGrid> | |
| ); | |
| const { container } = render(<CardGrid>{undefined}</CardGrid>); | |
| const { container } = render(<CardGrid>{null}</CardGrid>); | |
| const gridElement = container.querySelector('.cards-grid'); | |
| expect(gridElement).toBeInTheDocument(); | |
| expect(gridElement.children.length).toBe(0); | |
| }); | |
| test('handles undefined children', () => { | |
| const { container } = render(<CardGrid>{undefined}</CardGrid>); | |
| const gridElement = container.querySelector('.cards-grid'); | |
| expect(gridElement).toBeInTheDocument(); | |
| expect(gridElement.children.length).toBe(0); | |
| }); |
🤖 Prompt for AI Agents
In .vortex/docs/tests/unit/Card/CardGrid.test.js around lines 221 to 229, the
tests for null and undefined children only assert the grid exists but do not
verify that no element children were rendered; update both tests to additionally
assert that the grid has zero element children (e.g., check
gridElement.children.length === 0 or use querySelectorAll(':scope > *').length
=== 0) to prove no stray nodes are present.
| {''} {'\n'} | ||
| <Card icon="✅" title="Valid Card" description="Real content" /> | ||
| {' '} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Optional: assert whitespace-only children don’t affect element count.
Add a children-length assertion to make the intent explicit.
render(
<CardGrid>
{''} {'\n'}
<Card icon="✅" title="Valid Card" description="Real content" />
{' '}
</CardGrid>
);
+const grid = document.querySelector('.cards-grid');
+expect(grid.children.length).toBe(1);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {''} {'\n'} | |
| <Card icon="✅" title="Valid Card" description="Real content" /> | |
| {' '} | |
| render( | |
| <CardGrid> | |
| {''} {'\n'} | |
| <Card icon="✅" title="Valid Card" description="Real content" /> | |
| {' '} | |
| </CardGrid> | |
| ); | |
| const grid = document.querySelector('.cards-grid'); | |
| expect(grid.children.length).toBe(1); |
🤖 Prompt for AI Agents
In .vortex/docs/tests/unit/Card/CardGrid.test.js around lines 451-453, the test
renders whitespace-only children around a Card but doesn't explicitly assert
that those whitespace-only children don't change the element count; add an
explicit assertion after rendering that the number of Card components (or
rendered card elements) equals the expected count (e.g., 1) to make the intent
explicit, using the same test wrapper/query method already in the file (e.g.,
wrapper.find/Card query or getAllByRole/getAllByTestId) so the test fails if
whitespace nodes are incorrectly counted.
Closes #1951
Summary by CodeRabbit
New Features
Documentation
Chores
Tests