Conversation
📝 WalkthroughWalkthroughThe change adds an eight-step Docker Compose Wizard with template generation, service configuration, secret handling, validation, previews, and downloads. It also adds dynamic repository statistics and updates site navigation, statistics, styling, dependencies, and cache exclusions. ChangesDocker Compose Wizard
Site statistics and presentation
Estimated code review effort: 5 (Critical) | ~120 minutes Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (2)
src/pages/index.astro (1)
87-87: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse the common image component for the hero image.
Replace the direct
astro:assetsimage with~/components/common/Image.astro. Setloading="eager"andfetchpriority="high"because this image is above the fold.As per coding guidelines, use
src/components/common/Image.astrofor all image handling and use eager, high-priority loading for hero images.Source: Coding guidelines
src/pages/wizard.astro (1)
1147-1153: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winBuild the warning list with
textContentinstead ofinnerHTML.All current warning strings are internal constants, so there is no injection today. The pattern is still fragile. If a future warning embeds an answer value, such as the application URL or a Traefik network name, that value reaches
innerHTMLunescaped.
renderInstructionsalready appliesescapeHtml. Apply the same discipline here.🛡️ Proposed fix
if (result.warnings.length > 0) { warningsBox.classList.remove('hidden'); - warningsBox.innerHTML = result.warnings.map((w) => `<p>${w}</p>`).join(''); + warningsBox.replaceChildren( + ...result.warnings.map((w) => { + const p = document.createElement('p'); + p.textContent = w; + return p; + }) + ); } else { warningsBox.classList.add('hidden'); - warningsBox.innerHTML = ''; + warningsBox.replaceChildren(); }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 488c0c29-edd6-4998-aa31-73f39685ad3d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (27)
.gitignorepackage.jsonsrc/assets/styles/tailwind.csssrc/components/widgets/Stats.astrosrc/data/wizard/docker-compose.yamlsrc/data/wizard/env.examplesrc/navigation.jssrc/pages/index.astrosrc/pages/support.astrosrc/pages/wizard.astrosrc/types.d.tssrc/utils/repoStats.tssrc/utils/wizard/answers.tssrc/utils/wizard/composeEdit.tssrc/utils/wizard/dbCompose.tssrc/utils/wizard/dockerSecrets.tssrc/utils/wizard/envFileCompose.tssrc/utils/wizard/generator.tssrc/utils/wizard/nsfwService.tssrc/utils/wizard/oauthProviders.tssrc/utils/wizard/phpMyAdminCompose.tssrc/utils/wizard/secrets.tssrc/utils/wizard/templates.tssrc/utils/wizard/traefikCompose.tssrc/utils/wizard/validate.tssrc/utils/wizard/workerCompose.tstailwind.config.cjs
💤 Files with no reviewable changes (1)
- tailwind.config.cjs
| <ol id="wizard-stepper" class="flex flex-wrap gap-2 lg:shrink-0" role="tablist"> | ||
| <li><button type="button" class="wizard-step-btn" data-step-btn="0" aria-selected="true">1. Welcome</button></li> | ||
| <li><button type="button" class="wizard-step-btn" data-step-btn="1">2. General</button></li> | ||
| <li><button type="button" class="wizard-step-btn" data-step-btn="2">3. Database</button></li> | ||
| <li><button type="button" class="wizard-step-btn" data-step-btn="3">4. Secrets</button></li> | ||
| <li><button type="button" class="wizard-step-btn" data-step-btn="4">5. Services</button></li> | ||
| <li><button type="button" class="wizard-step-btn" data-step-btn="5">6. Workers & Proxy</button></li> | ||
| <li><button type="button" class="wizard-step-btn" data-step-btn="6">7. System</button></li> | ||
| <li><button type="button" class="wizard-step-btn" data-step-btn="7">8. Deploy</button></li> | ||
| </ol> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add role="tab" to the stepper buttons, or drop the tablist ARIA.
Line 40 sets role="tablist" on the <ol>. The tablist role requires tab children. Two problems break that contract:
- Each button is wrapped in an
<li>, so notabis an owned child of thetablist. - The buttons carry
aria-selectedbut norole="tab".aria-selectedis not valid on a plainbutton.
The stepper is not a tab interface. It is a linear step indicator. Remove the ARIA tab roles and expose the current step instead.
♿ Proposed fix for the stepper semantics
- <ol id="wizard-stepper" class="flex flex-wrap gap-2 lg:shrink-0" role="tablist">
- <li><button type="button" class="wizard-step-btn" data-step-btn="0" aria-selected="true">1. Welcome</button></li>
- <li><button type="button" class="wizard-step-btn" data-step-btn="1">2. General</button></li>
+ <ol id="wizard-stepper" class="flex flex-wrap gap-2 lg:shrink-0">
+ <li><button type="button" class="wizard-step-btn" data-step-btn="0" aria-current="step">1. Welcome</button></li>
+ <li><button type="button" class="wizard-step-btn" data-step-btn="1">2. General</button></li>Update showStep() and the .wizard-step-btn selector accordingly:
- stepButtons.forEach((btn, idx) => btn.setAttribute('aria-selected', String(idx === currentStep)));
+ stepButtons.forEach((btn, idx) => {
+ if (idx === currentStep) btn.setAttribute('aria-current', 'step');
+ else btn.removeAttribute('aria-current');
+ });- .wizard-step-btn[aria-selected='true'] {
+ .wizard-step-btn[aria-current='step'] {📝 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.
| <ol id="wizard-stepper" class="flex flex-wrap gap-2 lg:shrink-0" role="tablist"> | |
| <li><button type="button" class="wizard-step-btn" data-step-btn="0" aria-selected="true">1. Welcome</button></li> | |
| <li><button type="button" class="wizard-step-btn" data-step-btn="1">2. General</button></li> | |
| <li><button type="button" class="wizard-step-btn" data-step-btn="2">3. Database</button></li> | |
| <li><button type="button" class="wizard-step-btn" data-step-btn="3">4. Secrets</button></li> | |
| <li><button type="button" class="wizard-step-btn" data-step-btn="4">5. Services</button></li> | |
| <li><button type="button" class="wizard-step-btn" data-step-btn="5">6. Workers & Proxy</button></li> | |
| <li><button type="button" class="wizard-step-btn" data-step-btn="6">7. System</button></li> | |
| <li><button type="button" class="wizard-step-btn" data-step-btn="7">8. Deploy</button></li> | |
| </ol> | |
| <ol id="wizard-stepper" class="flex flex-wrap gap-2 lg:shrink-0"> | |
| <li><button type="button" class="wizard-step-btn" data-step-btn="0" aria-current="step">1. Welcome</button></li> | |
| <li><button type="button" class="wizard-step-btn" data-step-btn="1">2. General</button></li> | |
| <li><button type="button" class="wizard-step-btn" data-step-btn="2">3. Database</button></li> | |
| <li><button type="button" class="wizard-step-btn" data-step-btn="3">4. Secrets</button></li> | |
| <li><button type="button" class="wizard-step-btn" data-step-btn="4">5. Services</button></li> | |
| <li><button type="button" class="wizard-step-btn" data-step-btn="5">6. Workers & Proxy</button></li> | |
| <li><button type="button" class="wizard-step-btn" data-step-btn="6">7. System</button></li> | |
| <li><button type="button" class="wizard-step-btn" data-step-btn="7">8. Deploy</button></li> | |
| </ol> |
| const portError = validatePort(a.appPort); | ||
| const dbPortError = validateOptionalPort(a.dbPort); | ||
| const puidError = validateUint(a.puid); | ||
| const pgidError = validateUint(a.pgid); | ||
| const workerCountError = a.useWorker ? validatePositiveInt(a.workerCount) : null; | ||
| setError('appPort', portError); | ||
| setError('dbPort', dbPortError); | ||
| setError('puid', puidError); | ||
| setError('pgid', pgidError); | ||
| setError('workerCount', workerCountError); | ||
| if (portError || dbPortError || puidError || pgidError || workerCountError) return; | ||
|
|
||
| const result = generate(templates.envExample, templates.compose, a, secrets); | ||
| lastResult = result; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clear lastResult when validation fails.
Line 1133 returns early on any validation error. lastResult keeps the previously generated output. The output panels, the Copy buttons, and the Download buttons then still serve the earlier configuration while the form shows an error.
A user who fixes nothing and presses Download receives a file that does not match the form. Disable the output actions while the form is invalid.
🐛 Proposed fix to block stale downloads
+ function setOutputActionsEnabled(enabled: boolean) {
+ document
+ .querySelectorAll<HTMLButtonElement>('[data-copy], [data-download]')
+ .forEach((b) => (b.disabled = !enabled));
+ }
+
function render() {
const a = readAnswers();
updateVisibility(a);
@@
setError('workerCount', workerCountError);
- if (portError || dbPortError || puidError || pgidError || workerCountError) return;
+ if (portError || dbPortError || puidError || pgidError || workerCountError) {
+ setOutputActionsEnabled(false);
+ return;
+ }
+ setOutputActionsEnabled(true);📝 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 portError = validatePort(a.appPort); | |
| const dbPortError = validateOptionalPort(a.dbPort); | |
| const puidError = validateUint(a.puid); | |
| const pgidError = validateUint(a.pgid); | |
| const workerCountError = a.useWorker ? validatePositiveInt(a.workerCount) : null; | |
| setError('appPort', portError); | |
| setError('dbPort', dbPortError); | |
| setError('puid', puidError); | |
| setError('pgid', pgidError); | |
| setError('workerCount', workerCountError); | |
| if (portError || dbPortError || puidError || pgidError || workerCountError) return; | |
| const result = generate(templates.envExample, templates.compose, a, secrets); | |
| lastResult = result; | |
| function setOutputActionsEnabled(enabled: boolean) { | |
| document | |
| .querySelectorAll<HTMLButtonElement>('[data-copy], [data-download]') | |
| .forEach((b) => (b.disabled = !enabled)); | |
| } | |
| function render() { | |
| const a = readAnswers(); | |
| updateVisibility(a); | |
| const portError = validatePort(a.appPort); | |
| const dbPortError = validateOptionalPort(a.dbPort); | |
| const puidError = validateUint(a.puid); | |
| const pgidError = validateUint(a.pgid); | |
| const workerCountError = a.useWorker ? validatePositiveInt(a.workerCount) : null; | |
| setError('appPort', portError); | |
| setError('dbPort', dbPortError); | |
| setError('puid', puidError); | |
| setError('pgid', pgidError); | |
| setError('workerCount', workerCountError); | |
| if (portError || dbPortError || puidError || pgidError || workerCountError) { | |
| setOutputActionsEnabled(false); | |
| return; | |
| } | |
| setOutputActionsEnabled(true); | |
| const result = generate(templates.envExample, templates.compose, a, secrets); | |
| lastResult = result; |
| customAiVisionKey: (a) => envTarget('AI_VISION_API_KEY', a), | ||
| aiVisionApiKey: (a) => envTarget('AI_VISION_API_KEY', a), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the AI Vision needle to AI_VISION_FACE_API_KEY.
src/utils/wizard/generator.ts line 270 emits the key AI_VISION_FACE_API_KEY. These two entries search for AI_VISION_API_KEY. scrollToNeedle uses trimStart().startsWith(needle) for exact targets, so the search never matches.
Editing aiVisionApiKey or toggling customAiVisionKey therefore never scrolls the preview. The NSFW entries on lines 1320-1321 already use the full key name.
🐛 Proposed fix
- customAiVisionKey: (a) => envTarget('AI_VISION_API_KEY', a),
- aiVisionApiKey: (a) => envTarget('AI_VISION_API_KEY', a),
+ customAiVisionKey: (a) => envTarget('AI_VISION_FACE_API_KEY', a),
+ aiVisionApiKey: (a) => envTarget('AI_VISION_FACE_API_KEY', a),📝 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.
| customAiVisionKey: (a) => envTarget('AI_VISION_API_KEY', a), | |
| aiVisionApiKey: (a) => envTarget('AI_VISION_API_KEY', a), | |
| customAiVisionKey: (a) => envTarget('AI_VISION_FACE_API_KEY', a), | |
| aiVisionApiKey: (a) => envTarget('AI_VISION_FACE_API_KEY', a), |
| } | ||
|
|
||
| async function fetchGitHubRepo(): Promise<{ stars: number; forks: number }> { | ||
| const res = await fetch(GITHUB_REPO_URL, { headers: { Accept: 'application/vnd.github+json' } }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for file in package.json .nvmrc .node-version .tool-versions; do
if [ -f "$file" ]; then
echo "== $file =="
sed -n '1,160p' "$file"
fi
done
ast-grep outline src/utils/repoStats.ts --items all
rg -n -C 2 'fetch\(|AbortSignal\.timeout|signal:' src/utils/repoStats.tsRepository: LycheeOrg/LycheeOrg.github.io
Length of output: 4269
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== src/utils/repoStats.ts =="
cat -n src/utils/repoStats.ts
echo
echo "== Node AbortSignal.timeout availability =="
node - <<'JS'
const node = process.version;
console.log({ node });
try {
console.log(typeof AbortSignal.timeout);
} catch (e) {
console.log("AbortSignal.timeout unavailable");
}
JSRepository: LycheeOrg/LycheeOrg.github.io
Length of output: 5747
Set explicit deadlines for build-time statistics requests.
A stalled upstream service can delay the static build before the local fallback handlers run. Use a shared AbortSignal.timeout(...) signal for the GitHub repository, GitHub releases pages, and Docker Hub requests, while keeping the per-source fallback behavior.
📍 Affects 1 file
src/utils/repoStats.ts#L46-L46(this comment)src/utils/repoStats.ts#L57-L57src/utils/repoStats.ts#L77-L77
| // removeIndentedBlock removes the line matching startLineRegex and every | ||
| // following line that's indented deeper than it, i.e. its whole nested | ||
| // block. A single blank line immediately before the block is swallowed too, | ||
| // so removal doesn't leave a double blank line behind. | ||
| export function removeIndentedBlock(lines: string[], startLineRegex: RegExp): string[] { | ||
| const start = lines.findIndex((l) => startLineRegex.test(l)); | ||
| if (start === -1) return lines; | ||
|
|
||
| const indent = (/^ */.exec(lines[start]) ?? [''])[0].length; | ||
| let end = lines.length; | ||
| for (let i = start + 1; i < lines.length; i++) { | ||
| const m = /^( *)\S/.exec(lines[i]); | ||
| if (m && m[1].length <= indent) { | ||
| end = i; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| let removeStart = start; | ||
| if (start > 0 && lines[start - 1].trim() === '') removeStart = start - 1; | ||
| return [...lines.slice(0, removeStart), ...lines.slice(end)]; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect workerCompose.ts's removal regex and its relation to the header comment.
fd workerCompose.ts --exec cat -n {}Repository: LycheeOrg/LycheeOrg.github.io
Length of output: 971
Don’t remove lychee_worker: without its header comment.
removeWorkerService() matches only ^ {2}lychee_worker:\\s*$, so removeIndentedBlock() removes the service block but leaves the preceding 15-line comment header behind in the generated compose file. Update the helper to also remove an immediately preceding comment block, or start the removal from that comment.
🧰 Tools
🪛 OpenGrep (1.26.0)
[ERROR] 14-14: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
[ERROR] 17-17: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
| export function removeEnvFileReferences(compose: string): RemoveEnvFileReferencesResult { | ||
| let lines = compose.split('\n'); | ||
| let removedAny = false; | ||
|
|
||
| // There are exactly two occurrences; removeIndentedBlock only strips the | ||
| // first match per call, so run it once per occurrence. | ||
| for (let i = 0; i < 2; i++) { | ||
| const before = lines.length; | ||
| lines = removeIndentedBlock(lines, /^\s*env_file:\s*$/); | ||
| if (lines.length !== before) removedAny = true; | ||
| } | ||
|
|
||
| return { compose: lines.join('\n'), removed: removedAny }; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fixed "exactly two occurrences" assumption is fragile against live-fetched templates.
removeEnvFileReferences loops exactly twice, assuming the compose content always has exactly two env_file: blocks. This project fetches the live upstream docker-compose.yaml by default (see templates.ts), not just the bundled snapshot. If upstream ever changes the number of env_file: occurrences, this function either performs a no-op extra pass or silently leaves a dangling env_file: reference to a .env file that this code path intentionally does not create.
removed: removedAny only reports whether at least one occurrence was removed, not whether every occurrence found in the actual content was removed. This can produce a broken generated docker-compose.yaml without any warning to the user.
Loop until no more matches are found instead of a fixed count.
🛠️ Proposed fix
export function removeEnvFileReferences(compose: string): RemoveEnvFileReferencesResult {
let lines = compose.split('\n');
let removedAny = false;
- // There are exactly two occurrences; removeIndentedBlock only strips the
- // first match per call, so run it once per occurrence.
- for (let i = 0; i < 2; i++) {
- const before = lines.length;
- lines = removeIndentedBlock(lines, /^\s*env_file:\s*$/);
- if (lines.length !== before) removedAny = true;
- }
+ // removeIndentedBlock only strips the first match per call; keep calling
+ // it until no more env_file: blocks are found, so this stays correct even
+ // if the (possibly live-fetched) template's occurrence count changes.
+ while (true) {
+ const before = lines.length;
+ lines = removeIndentedBlock(lines, /^\s*env_file:\s*$/);
+ if (lines.length === before) break;
+ removedAny = true;
+ }
return { compose: lines.join('\n'), removed: removedAny };
}📝 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.
| export function removeEnvFileReferences(compose: string): RemoveEnvFileReferencesResult { | |
| let lines = compose.split('\n'); | |
| let removedAny = false; | |
| // There are exactly two occurrences; removeIndentedBlock only strips the | |
| // first match per call, so run it once per occurrence. | |
| for (let i = 0; i < 2; i++) { | |
| const before = lines.length; | |
| lines = removeIndentedBlock(lines, /^\s*env_file:\s*$/); | |
| if (lines.length !== before) removedAny = true; | |
| } | |
| return { compose: lines.join('\n'), removed: removedAny }; | |
| } | |
| export function removeEnvFileReferences(compose: string): RemoveEnvFileReferencesResult { | |
| let lines = compose.split('\n'); | |
| let removedAny = false; | |
| // removeIndentedBlock only strips the first match per call; keep calling | |
| // it until no more env_file: blocks are found, so this stays correct even | |
| // if the (possibly live-fetched) template's occurrence count changes. | |
| while (true) { | |
| const before = lines.length; | |
| lines = removeIndentedBlock(lines, /^\s*env_file:\s*$/); | |
| if (lines.length === before) break; | |
| removedAny = true; | |
| } | |
| return { compose: lines.join('\n'), removed: removedAny }; | |
| } |
| const networksIdx = lines.findIndex((l) => /^networks:\s*$/.test(l)); | ||
| if (networksIdx === -1) { | ||
| return { compose, inserted: false }; | ||
| } | ||
| lines = [...lines.slice(0, networksIdx), ...NSFW_SERVICE_LINES, '', ...lines.slice(networksIdx)]; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check the order of top-level keys in the bundled compose template.
fd -t f 'docker-compose.yaml' src/data | while IFS= read -r f; do
echo "== $f"
rg -n '^[A-Za-z_-]+:' "$f"
doneRepository: LycheeOrg/LycheeOrg.github.io
Length of output: 288
🏁 Script executed:
#!/bin/bash
set -e
# Show the relevant implementation and the surrounding template sections for context.
sed -n '1,130p' src/utils/wizard/nsfwService.ts
echo "== docker-compose template top-level/surrounding keys =="
sed -n '600,665p' src/data/wizard/docker-compose.yaml
echo "== behavioral check: where service block is inserted for the bundled compose =="
python3 - <<'PY'
from pathlib import Path
compose = Path('src/data/wizard/docker-compose.yaml').read_text().splitlines()
service_block = """ nsfw:
container_name: nsfw
image: ghcr.io/lynkey/netsize-file-analyzer:latest
expose:
- 24088
restart: unless-stopped
depends_on:
db:
condition: service_healthy
networks:
- lychee-network
"""
lines = compose[:]
networks_idx = next((i for i,l in enumerate(lines) if l.rstrip() == 'networks:'), -1)
if networks_idx != -1:
inserted = lines[:networks_idx] + service_block.rstrip().splitlines() + [''] + lines[networks_idx:]
service_starts = [i for i,l in enumerate(inserted) if l.strip() == 'services:']
networks = [i for i,l in enumerate(inserted) if l.rstrip() == 'networks:']
idx = service_starts[0] if service_starts else None
print(f"services line: {idx+1 if idx is not None else None}")
print(f"original services: {next((i for i,l in enumerate(lines) if l.strip() == 'services:')) + 1}")
print(f"insert idx: {networks_idx + 1}")
print(f"indent of inserted line after services: {repr(inserted[idx+1])}")
print(f"content of inserted line after services: {lines[idx].rstrip()}")
PYRepository: LycheeOrg/LycheeOrg.github.io
Length of output: 7121
Insert the NSFW service before volumes: only if it is a top-level key.
The bundled compose file has networks: before volumes:, so the current insertion happens inside services:. If volumes: appears before networks:, this also drops the block under services:. Locate networks: as the first top-level key, then insert NSFW_SERVICE_LINES at that top-level boundary.
| const volumesIdx = lines.findIndex((l) => /^volumes:\s*$/.test(l)); | ||
| if (volumesIdx !== -1) { | ||
| lines = [...lines.slice(0, volumesIdx + 1), ...NSFW_VOLUME_LINES, ...lines.slice(volumesIdx + 1)]; | ||
| } | ||
|
|
||
| return { compose: lines.join('\n'), inserted: true }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Compose helpers report partial edits as complete results. Both helpers apply a multi-anchor edit but collapse the outcome into a single boolean. When one anchor is missing, the helper still returns the modified Compose text. src/utils/wizard/generator.ts then keeps that text and either emits no warning or emits a warning that contradicts what was applied. In both cases the generated docker-compose.yaml references an undeclared resource and docker compose up fails.
src/utils/wizard/nsfwService.ts#L93-L98: return a separatevolumeDeclaredflag when the top-levelvolumes:anchor is missing, and warn ingenerate()that thensfw_classification_queuevolume needs a manual declaration.src/utils/wizard/traefikCompose.ts#L66-L71: return the originalcomposeargument when the top-levelnetworks:anchor is missing, so the labels and the service-levelnetworks:list are not left behind withadded: false.
📍 Affects 2 files
src/utils/wizard/nsfwService.ts#L93-L98(this comment)src/utils/wizard/traefikCompose.ts#L66-L71
| export async function loadTemplates(fallbackEnvExample: string, fallbackCompose: string): Promise<Templates> { | ||
| try { | ||
| const [envRes, composeRes] = await Promise.all([ | ||
| fetch(ENV_EXAMPLE_URL, { cache: 'no-store' }), | ||
| fetch(COMPOSE_URL, { cache: 'no-store' }), | ||
| ]); | ||
| if (!envRes.ok || !composeRes.ok) { | ||
| throw new Error('non-200 response fetching upstream templates'); | ||
| } | ||
| const [envExample, compose] = await Promise.all([envRes.text(), composeRes.text()]); | ||
| if (!envExample.trim() || !compose.trim()) { | ||
| throw new Error('empty response fetching upstream templates'); | ||
| } | ||
| return { envExample, compose, fromLive: true }; | ||
| } catch { | ||
| return { envExample: fallbackEnvExample, compose: fallbackCompose, fromLive: false }; | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
What browsers support AbortSignal.timeout()?
💡 Result:
The AbortSignal.timeout static method is widely supported across all major modern web browsers [1][2]. It is available in Web Workers and the main thread [3]. Browser support details: Chrome: Supported since version 124 [1][4]. Edge: Supported since version 124 [1][4]. Firefox: Supported since version 100 [1][4]. Safari: Supported since version 16.0 [1][2]. Chrome for Android: Supported since version 124 [1]. Firefox for Android: Supported since version 100 [1]. Safari on iOS: Supported since version 16.0 [1]. This method returns an AbortSignal that automatically aborts after a specified duration, triggering a TimeoutError DOMException [3][5]. If you need to support older environments, ensure you implement feature detection or use a polyfill [3].
Citations:
- 1: https://caniuse.com/mdn-api_abortsignal_timeout_static
- 2: https://web-platform-dx.github.io/web-features-explorer/features/abortsignal-timeout/
- 3: https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/timeout_static
- 4: https://webstatus.dev/features/abortsignal-timeout?sort=stable_chrome_asc
- 5: https://github.com/mdn/content/blob/main/files/en-us/web/api/abortsignal/timeout_static/index.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files matching templates.ts:\n'
fd -a 'templates\.ts$' . || true
printf '\nRelevant source snippets:\n'
for f in $(fd 'templates\.ts$' .); do
echo "== $f =="
nl -ba "$f" | sed -n '1,120p'
done
printf '\nSearch for loadTemplates usage:\n'
rg -n "loadTemplates|ENV_EXAMPLE_URL|COMPOSE_URL|fromLive" -S .
printf '\nRepo package/browser config candidates:\n'
fd -a 'package(\.json)?$|browserslist|\.browserslistrc|astro.config\.(js|ts|mjs|cjs)$|svelte.config\.js|vite.config\.(js|ts)$|tsconfig.*\.json$' . | sed -n '1,200p'
printf '\nConfig files snippets (if present):\n'
for f in package.json astro.config.* svelte.config.js vite.config.* tsconfig.json astro.config.ts; do
[ -e "$f" ] && printf '\n== %s ==\n' "$f" && nl -ba "$f" | sed -n '1,220p'
doneRepository: LycheeOrg/LycheeOrg.github.io
Length of output: 351
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Relevant source:\n'
awk '{printf "%6d\t%s\n", NR, $0}' src/utils/wizard/templates.ts | sed -n '1,140p'
printf '\nSearch for loadTemplates usage and related symbols:\n'
grep -RInE "loadTemplates|ENV_EXAMPLE_URL|COMPOSE_URL|fromLive|raw\.githubusercontent\.com|browsers|caniuse|AbortSignal\.timeout|timeout(" . || true
printf '\nConfig/package files:\n'
printf '--- package.json ---\n'
awk '{printf "%6d\t%s\n", NR, $0}' package.json | sed -n '1,220p'
printf '\n--- astro config candidates ---\n'
for f in astro.config.js astro.config.ts astro.config.mjs astro.config.cjs; do
if [ -f "$f" ]; then
echo "--- $f ---"
awk '{printf "%6d\t%s\n", NR, $0}' "$f" | sed -n '1,220p'
fi
done
printf '\n--- browser/engine targets candidates ---\n'
for f in package.json browserslistrc .browserslistrc; do
if [ -f "$f" ]; then
echo "--- $f ---"
awk '{printf "%6d\t%s\n", NR, $0}' "$f" | sed -n '1,220p'
fi
doneRepository: LycheeOrg/LycheeOrg.github.io
Length of output: 2019
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Search for loadTemplates usage and related symbols:\n'
grep -RInE 'loadTemplates|ENV_EXAMPLE_URL|COMPOSE_URL|fromLive|raw\.githubusercontent\.com|browsers|caniuse|AbortSignal\.timeout|timeout\(' . || true
printf '\nConfig/package files:\n'
printf '%s\n' '--- package.json ---'
awk '{printf "%6d\t%s\n", NR, $0}' package.json | sed -n '1,220p'
printf '%s\n' '--- astro config candidates ---'
for f in astro.config.js astro.config.ts astro.config.mjs astro.config.cjs; do
if [ -f "$f" ]; then
printf '%s\n' "--- $f ---"
awk '{printf "%6d\t%s\n", NR, $0}' "$f" | sed -n '1,220p'
fi
done
printf '%s\n' '--- browser/engine targets candidates ---'
for f in package.json .browserslistrc browserslistrc; do
if [ -f "$f" ]; then
printf '%s\n' "--- $f ---"
awk '{printf "%6d\t%s\n", NR, $0}' "$f" | sed -n '1,220p'
fi
done
printf '%s\n' '--- tsconfig candidates ---'
for f in tsconfig.json tsconfig.node.json tsconfig.app.json; do
if [ -f "$f" ]; then
printf '%s\n' "--- $f ---"
awk '{printf "%6d\t%s\n", NR, $0}' "$f" | sed -n '1,220p'
fi
doneRepository: LycheeOrg/LycheeOrg.github.io
Length of output: 50386
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'loadTemplates references in tracked source:\n'
grep -nE 'loadTemplates|ENV_EXAMPLE_URL|COMPOSE_URL|fromLive' src 2>/dev/null || true
printf '\nBrowser-target config (tracked root config files only):\n'
for f in package.json .browserslistrc browserslistrc astro.config.js astro.config.ts astro.config.mjs astro.config.cjs tsconfig.json; do
[ -f "$f" ] || continue
echo "--- $f ---"
grep -nE 'browserslist|targets|support|engines|abort|AbortSignal|fetch|astro' "$f" || true
case "$f" in
package.json) sed -n '1,220p' "$f" ;;
.browserslistrc|browserslistrc) sed -n '1,220p' "$f" ;;
astro.config.*) sed -n '1,220p' "$f" ;;
tsconfig.json) sed -n '1,220p' "$f" ;;
esac
doneRepository: LycheeOrg/LycheeOrg.github.io
Length of output: 8834
Add a timeout to the template fetches.
loadTemplates falls back on fetch failure, non-2xx status, or empty body, but a slow or hanging upstream response does not trigger the fallback. Add an AbortSignal.timeout() deadline to both fetch calls so the wizard can use bundled templates within a bounded time.
| export function validatePort(s: string): string | null { | ||
| const n = Number(s); | ||
| if (!Number.isInteger(n) || n <= 0 || n > 65535) { | ||
| return 'Must be a valid port number (1-65535).'; | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| // validateOptionalPort is validatePort, except a blank value is valid — used | ||
| // for the external-database port field, which falls back to the engine's | ||
| // default port when left empty. | ||
| export function validateOptionalPort(s: string): string | null { | ||
| if (s.trim() === '') return null; | ||
| return validatePort(s); | ||
| } | ||
|
|
||
| export function validateUint(s: string): string | null { | ||
| const n = Number(s); | ||
| if (!Number.isInteger(n) || n < 0) { | ||
| return 'Must be a non-negative integer.'; | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| export function validatePositiveInt(s: string): string | null { | ||
| const n = Number(s); | ||
| if (!Number.isInteger(n) || n < 1) { | ||
| return 'Must be a positive integer (1 or more).'; | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject non-decimal numeric strings before parsing.
All four validators call Number(s) directly. Number() also accepts scientific notation ("1e2"), hexadecimal ("0x50"), and leading +/whitespace, and these pass Number.isInteger(). If the raw typed string (not the parsed number) is what later gets embedded into the generated .env/docker-compose.yaml output, a value like "0x50" would pass validation as port 80 but leave the literal string "0x50" in the generated file.
Add a plain-digit check before parsing.
🛠️ Proposed fix
export function validatePort(s: string): string | null {
+ if (!/^\d+$/.test(s.trim())) {
+ return 'Must be a valid port number (1-65535).';
+ }
const n = Number(s);
if (!Number.isInteger(n) || n <= 0 || n > 65535) {
return 'Must be a valid port number (1-65535).';
}
return null;
}
@@
export function validateUint(s: string): string | null {
+ if (!/^\d+$/.test(s.trim())) {
+ return 'Must be a non-negative integer.';
+ }
const n = Number(s);
if (!Number.isInteger(n) || n < 0) {
return 'Must be a non-negative integer.';
}
return null;
}
@@
export function validatePositiveInt(s: string): string | null {
+ if (!/^\d+$/.test(s.trim())) {
+ return 'Must be a positive integer (1 or more).';
+ }
const n = Number(s);
if (!Number.isInteger(n) || n < 1) {
return 'Must be a positive integer (1 or more).';
}
return 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.
| export function validatePort(s: string): string | null { | |
| const n = Number(s); | |
| if (!Number.isInteger(n) || n <= 0 || n > 65535) { | |
| return 'Must be a valid port number (1-65535).'; | |
| } | |
| return null; | |
| } | |
| // validateOptionalPort is validatePort, except a blank value is valid — used | |
| // for the external-database port field, which falls back to the engine's | |
| // default port when left empty. | |
| export function validateOptionalPort(s: string): string | null { | |
| if (s.trim() === '') return null; | |
| return validatePort(s); | |
| } | |
| export function validateUint(s: string): string | null { | |
| const n = Number(s); | |
| if (!Number.isInteger(n) || n < 0) { | |
| return 'Must be a non-negative integer.'; | |
| } | |
| return null; | |
| } | |
| export function validatePositiveInt(s: string): string | null { | |
| const n = Number(s); | |
| if (!Number.isInteger(n) || n < 1) { | |
| return 'Must be a positive integer (1 or more).'; | |
| } | |
| return null; | |
| } | |
| export function validatePort(s: string): string | null { | |
| if (!/^\d+$/.test(s.trim())) { | |
| return 'Must be a valid port number (1-65535).'; | |
| } | |
| const n = Number(s); | |
| if (!Number.isInteger(n) || n <= 0 || n > 65535) { | |
| return 'Must be a valid port number (1-65535).'; | |
| } | |
| return null; | |
| } | |
| // validateOptionalPort is validatePort, except a blank value is valid — used | |
| // for the external-database port field, which falls back to the engine's | |
| // default port when left empty. | |
| export function validateOptionalPort(s: string): string | null { | |
| if (s.trim() === '') return null; | |
| return validatePort(s); | |
| } | |
| export function validateUint(s: string): string | null { | |
| if (!/^\d+$/.test(s.trim())) { | |
| return 'Must be a non-negative integer.'; | |
| } | |
| const n = Number(s); | |
| if (!Number.isInteger(n) || n < 0) { | |
| return 'Must be a non-negative integer.'; | |
| } | |
| return null; | |
| } | |
| export function validatePositiveInt(s: string): string | null { | |
| if (!/^\d+$/.test(s.trim())) { | |
| return 'Must be a positive integer (1 or more).'; | |
| } | |
| const n = Number(s); | |
| if (!Number.isInteger(n) || n < 1) { | |
| return 'Must be a positive integer (1 or more).'; | |
| } | |
| return null; | |
| } |
1 participant
Summary by CodeRabbit