-
Notifications
You must be signed in to change notification settings - Fork 0
Claude/plan v3 deployment vp mpw #111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
c3ce323
1c1d19c
92e93e6
3d4befe
41c2083
4a2f475
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,63 @@ | ||||||||||||||||||||||||||||||||
| #!/usr/bin/env bash | ||||||||||||||||||||||||||||||||
| # add_tooltip.sh — Append a new placeholder tooltip entry to tooltips.json. | ||||||||||||||||||||||||||||||||
| # Called by the bot admin panel "Add Tooltip (Script)" button, or manually. | ||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||
| # Usage: | ||||||||||||||||||||||||||||||||
| # ./add_tooltip.sh [--text "Custom tooltip text"] | ||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||
| # Outputs: new tooltip ID on stdout. | ||||||||||||||||||||||||||||||||
| # On success exits 0; on failure exits non-zero. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| set -euo pipefail | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||||||||||||||||||||||||||||||||
| APP_DIR="${RUNEWAGER_DIR:-$SCRIPT_DIR}" | ||||||||||||||||||||||||||||||||
| DATA_DIR="$APP_DIR/data" | ||||||||||||||||||||||||||||||||
| TOOLTIPS_FILE="$DATA_DIR/tooltips.json" | ||||||||||||||||||||||||||||||||
| TMP_FILE="$TOOLTIPS_FILE.tmp.$$" | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| CUSTOM_TEXT="" | ||||||||||||||||||||||||||||||||
| while [[ $# -gt 0 ]]; do | ||||||||||||||||||||||||||||||||
| case "$1" in | ||||||||||||||||||||||||||||||||
| --text) CUSTOM_TEXT="$2"; shift 2 ;; | ||||||||||||||||||||||||||||||||
| *) shift ;; | ||||||||||||||||||||||||||||||||
| esac | ||||||||||||||||||||||||||||||||
| done | ||||||||||||||||||||||||||||||||
|
Comment on lines
+20
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Argument parsing missing bounds check for If 🛡️ Add bounds check while [[ $# -gt 0 ]]; do
case "$1" in
- --text) CUSTOM_TEXT="$2"; shift 2 ;;
+ --text)
+ [[ $# -lt 2 ]] && { echo "ERROR: --text requires a value" >&2; exit 1; }
+ CUSTOM_TEXT="$2"; shift 2 ;;
*) shift ;;
esac
done📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| info() { echo "[add_tooltip] INFO: $*"; } | ||||||||||||||||||||||||||||||||
| error() { echo "[add_tooltip] ERROR: $*" >&2; exit 1; } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| mkdir -p "$DATA_DIR" || error "Cannot create data dir" | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| # Ensure tooltips.json exists | ||||||||||||||||||||||||||||||||
| if [[ ! -f "$TOOLTIPS_FILE" ]]; then | ||||||||||||||||||||||||||||||||
| info "tooltips.json not found — initialising empty list" | ||||||||||||||||||||||||||||||||
| echo '[]' > "$TOOLTIPS_FILE" | ||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| # Validate existing file | ||||||||||||||||||||||||||||||||
| node -e "JSON.parse(require('fs').readFileSync('$TOOLTIPS_FILE','utf8'))" 2>/dev/null \ | ||||||||||||||||||||||||||||||||
| || error "Existing $TOOLTIPS_FILE is not valid JSON" | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| TOOLTIP_TEXT="${CUSTOM_TEXT:-New tooltip — edit in admin panel via /tips.}" | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| # Append new entry and get new ID using Node.js | ||||||||||||||||||||||||||||||||
| NEW_ID=$(node - "$TOOLTIPS_FILE" <<EOF | ||||||||||||||||||||||||||||||||
| const fs = require('fs'); | ||||||||||||||||||||||||||||||||
| const file = process.argv[1]; | ||||||||||||||||||||||||||||||||
| const list = JSON.parse(fs.readFileSync(file, 'utf8')); | ||||||||||||||||||||||||||||||||
| const maxId = list.reduce((m, t) => Math.max(m, Number(t.id) || 0), 0); | ||||||||||||||||||||||||||||||||
| const newId = maxId + 1; | ||||||||||||||||||||||||||||||||
| list.push({ id: newId, text: $(node -e "process.stdout.write(JSON.stringify('$TOOLTIP_TEXT'))"), enabled: true }); | ||||||||||||||||||||||||||||||||
|
Comment on lines
+45
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (bug_risk): Shell interpolation inside the Node.js heredoc is producing invalid JavaScript when adding the tooltip text. Inside the heredoc, the shell expands Instead, JSON-encode the tooltip text once in the shell and pass it as an argument, then parse it inside the Node script: TOOLTIP_TEXT_JSON=$(node -e 'console.log(JSON.stringify(process.argv[1] || ""))' "$TOOLTIP_TEXT")
NEW_ID=$(node - "$TOOLTIPS_FILE" "$TOOLTIP_TEXT_JSON" <<'EOF'
const fs = require('fs');
const file = process.argv[2];
const text = JSON.parse(process.argv[3]);
const list = JSON.parse(fs.readFileSync(file, 'utf8'));
const maxId = list.reduce((m, t) => Math.max(m, Number(t.id) || 0), 0);
const newId = maxId + 1;
list.push({ id: newId, text, enabled: true });
fs.writeFileSync('${TMP_FILE}', JSON.stringify(list, null, 2));
console.log(newId);
EOF
)This keeps quoting inside Node and avoids shell-generated JS fragments.
Comment on lines
+45
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: The Node.js heredoc currently reads from the wrong CLI argument ( Severity Level: Critical 🚨- ❌ Admin "Add Tooltip (Script)" cannot append any tooltips.
- ❌ Manual `./add_tooltip.sh` usage always fails with ENOENT.
- ⚠️ Custom texts containing quotes corrupt or abort JSON generation.
Suggested change
Steps of Reproduction ✅1. From the repository root `/workspace/Runewager`, run `./add_tooltip.sh` (script defined
in `add_tooltip.sh:1-63`).
2. Script creates `data/tooltips.json` if missing and validates it via `node -e` at
`add_tooltip.sh:32-40`.
3. The Node heredoc at `add_tooltip.sh:44-54` runs `node - "$TOOLTIPS_FILE"`, so inside
Node `process.argv[1]` is `"-"` and `process.argv[2]` is the actual `tooltips.json` path.
4. Line `47` (`const file = process.argv[1];`) causes `fs.readFileSync('-','utf8')` to
execute; since no file named `-` exists, Node throws `ENOENT`, the `node` command exits
non‑zero, `set -e` aborts the script, and no tooltip is appended.
5. Additionally, invoke `./add_tooltip.sh --text "It's broken"`; at `add_tooltip.sh:51`
the nested `node -e "JSON.stringify('$TOOLTIP_TEXT')"` expansion injects an unescaped
single quote into JavaScript, causing a parse error in the inner Node process and again
aborting the script before updating `tooltips.json`.
6. In both cases, the admin-facing "Add Tooltip (Script)" flow described in
`add_tooltip.sh:3` will fail whenever it calls this script, preventing new tooltips from
being added.Prompt for AI Agent 🤖This is a comment left during a code review.
**Path:** add_tooltip.sh
**Line:** 45:51
**Comment:**
*Logic Error: The Node.js heredoc currently reads from the wrong CLI argument (`process.argv[1]` instead of the tooltips file argument) and inlines the tooltip text via a nested `node -e` command substitution, which both causes it to read a non-existent `-` file and makes any tooltip text containing quotes or special characters break the script or corrupt the JSON; passing the text via an environment variable and using the correct argv index fixes both issues.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. |
||||||||||||||||||||||||||||||||
| fs.writeFileSync('${TMP_FILE}', JSON.stringify(list, null, 2)); | ||||||||||||||||||||||||||||||||
| console.log(newId); | ||||||||||||||||||||||||||||||||
| EOF | ||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||
|
Comment on lines
+45
to
+55
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shell variable interpolation in Node code is vulnerable to injection. Line 51 embeds 🔒 Safer approach using environment variables # Append new entry and get new ID using Node.js
-NEW_ID=$(node - "$TOOLTIPS_FILE" <<EOF
+NEW_ID=$(TOOLTIP_TEXT="$TOOLTIP_TEXT" TMP_FILE="$TMP_FILE" node - "$TOOLTIPS_FILE" <<'EOF'
const fs = require('fs');
const file = process.argv[1];
const list = JSON.parse(fs.readFileSync(file, 'utf8'));
const maxId = list.reduce((m, t) => Math.max(m, Number(t.id) || 0), 0);
const newId = maxId + 1;
-list.push({ id: newId, text: $(node -e "process.stdout.write(JSON.stringify('$TOOLTIP_TEXT'))"), enabled: true });
-fs.writeFileSync('${TMP_FILE}', JSON.stringify(list, null, 2));
+list.push({ id: newId, text: process.env.TOOLTIP_TEXT, enabled: true });
+fs.writeFileSync(process.env.TMP_FILE, JSON.stringify(list, null, 2));
console.log(newId);
EOF
)Note: Changed 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| # Validate and move | ||||||||||||||||||||||||||||||||
| node -e "JSON.parse(require('fs').readFileSync('$TMP_FILE','utf8'))" 2>/dev/null \ | ||||||||||||||||||||||||||||||||
| || { rm -f "$TMP_FILE"; error "Generated JSON failed validation"; } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| mv "$TMP_FILE" "$TOOLTIPS_FILE" | ||||||||||||||||||||||||||||||||
| info "Added tooltip #${NEW_ID}: ${TOOLTIP_TEXT:0:60}" | ||||||||||||||||||||||||||||||||
| echo "$NEW_ID" | ||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,75 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #!/usr/bin/env bash | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # generate_tooltips.sh — Refresh the Helpful Tooltips data file. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Idempotent: safe to run on every deploy. Creates/overwrites tooltips.json | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # from the source-of-truth DEFAULT_TIPS_LIST embedded in index.js. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Called automatically by deploy.sh and prod-run.sh before bot restart. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Usage: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # ./generate_tooltips.sh [--dry-run] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # --dry-run Print what would be written without making changes. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| set -euo pipefail | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| APP_DIR="${RUNEWAGER_DIR:-$SCRIPT_DIR}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DATA_DIR="$APP_DIR/data" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| TOOLTIPS_FILE="$DATA_DIR/tooltips.json" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| TMP_FILE="$TOOLTIPS_FILE.tmp.$$" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DRY_RUN=false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for arg in "$@"; do | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [[ "$arg" == "--dry-run" ]] && DRY_RUN=true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| done | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| info() { echo "[generate_tooltips] INFO: $*"; } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| warn() { echo "[generate_tooltips] WARN: $*" >&2; } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| error() { echo "[generate_tooltips] ERROR: $*" >&2; exit 1; } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Ensure data directory exists | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mkdir -p "$DATA_DIR" || error "Cannot create data dir: $DATA_DIR" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Extract DEFAULT_TIPS_LIST from index.js using Node.js | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if [[ ! -f "$APP_DIR/index.js" ]]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| error "index.js not found at $APP_DIR/index.js" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| info "Extracting DEFAULT_TIPS_LIST from index.js..." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| TOOLTIP_JSON=$(node - <<'EOF' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const fs = require('fs'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const src = fs.readFileSync(process.argv[1] || 'index.js', 'utf8'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Execute just the DEFAULT_TIPS_LIST block and print it as JSON | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const m = src.match(/const DEFAULT_TIPS_LIST\s*=\s*(\[[\s\S]+?\]);/); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!m) { process.stderr.write('DEFAULT_TIPS_LIST not found\n'); process.exit(1); } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Use Function constructor for safe eval of the array literal | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const list = (new Function('return ' + m[1]))(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.log(JSON.stringify(list, null, 2)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+37
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (bug_risk): The heredoc / command substitution block is syntactically and semantically broken, likely causing generate_tooltips.sh to fail or misbehave. This combines the heredoc and a separate Inside the Node snippet, You likely want something along the lines of: TOOLTIP_JSON=$(node "$APP_DIR/index.js" <<'EOF'
const fs = require('fs');
const appIndex = process.argv[2] || 'index.js';
const src = fs.readFileSync(appIndex, 'utf8');
...
EOF
"$APP_DIR/index.js") || { ... }or otherwise restructure so the heredoc and any additional |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (e) { process.stderr.write('Parse error: ' + e.message + '\n'); process.exit(1); } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| EOF | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| node "$APP_DIR/index.js" --version 2>/dev/null || true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+37
to
+49
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: The tooltip generation script never actually reads the intended Severity Level: Major
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| TOOLTIP_JSON=$(node - <<'EOF' | |
| const fs = require('fs'); | |
| const src = fs.readFileSync(process.argv[1] || 'index.js', 'utf8'); | |
| // Execute just the DEFAULT_TIPS_LIST block and print it as JSON | |
| const m = src.match(/const DEFAULT_TIPS_LIST\s*=\s*(\[[\s\S]+?\]);/); | |
| if (!m) { process.stderr.write('DEFAULT_TIPS_LIST not found\n'); process.exit(1); } | |
| try { | |
| // Use Function constructor for safe eval of the array literal | |
| const list = (new Function('return ' + m[1]))(); | |
| console.log(JSON.stringify(list, null, 2)); | |
| } catch (e) { process.stderr.write('Parse error: ' + e.message + '\n'); process.exit(1); } | |
| EOF | |
| node "$APP_DIR/index.js" --version 2>/dev/null || true | |
| TOOLTIP_JSON=$(node - "$APP_DIR/index.js" <<'EOF' | |
| const fs = require('fs'); | |
| const srcPath = process.argv[2] || 'index.js'; | |
| const src = fs.readFileSync(srcPath, 'utf8'); | |
| // Execute just the DEFAULT_TIPS_LIST block and print it as JSON | |
| const m = src.match(/const DEFAULT_TIPS_LIST\s*=\s*(\[[\s\S]+?\]);/); | |
| if (!m) { process.stderr.write('DEFAULT_TIPS_LIST not found\n'); process.exit(1); } | |
| try { | |
| // Use Function constructor for safe eval of the array literal | |
| const list = (new Function('return ' + m[1]))(); | |
| console.log(JSON.stringify(list, null, 2)); | |
| } catch (e) { process.stderr.write('Parse error: ' + e.message + '\n'); process.exit(1); } | |
| EOF |
Steps of Reproduction ✅
1. Trigger a deployment that runs the tooltip generator, e.g. execute the VPS deployment
script `deploy.sh` at `/workspace/Runewager/deploy.sh`, which at lines 217–221 sets
`TOOLTIP_SCRIPT="$PROJECT_DIR/generate_tooltips.sh"` and invokes
`RUNEWAGER_DIR="$PROJECT_DIR" bash "$TOOLTIP_SCRIPT"` before restarting the bot.
2. Inside `generate_tooltips.sh` at lines 36–47, the script runs `TOOLTIP_JSON=$(node -
<<'EOF' ...)` and the embedded Node snippet calls `fs.readFileSync(process.argv[1] ||
'index.js', 'utf8')`; because the `node` command is invoked as `node -`, `process.argv[1]`
is `'-'`, so Node attempts to read a non-existent file named `-` and exits with an error
without emitting JSON.
3. Still inside the same command substitution (lines 36–50), the next command `node
"$APP_DIR/index.js" --version 2>/dev/null || true` runs successfully and prints a version
string (for example `v3.0.0`) to stdout; because this is the last command in the subshell
its zero exit status causes the overall `$(...)` to succeed, and `TOOLTIP_JSON` is set to
the non‑JSON version string instead of a JSON array.
4. Later in `generate_tooltips.sh` at lines 62–71, the script writes `"$TOOLTIP_JSON"`
(the version string) to the temp file and validates it with `node -e "JSON.parse(...)"`;
JSON parsing fails, so the `|| { ... }` block at lines 66–71 replaces the temp file
contents with a hard-coded placeholder JSON array, and finally `mv "$TMP_FILE"
"$TOOLTIPS_FILE"` at line 73 writes this placeholder to
`/var/www/html/Runewager/data/tooltips.json`, which is exactly the path used by
`loadHelpfulMessages()` in `index.js` (lines 14–23 and the `systemTooltipsFile` constant
at line 337) to populate the in-bot Helpful Tooltips list.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** generate_tooltips.sh
**Line:** 37:49
**Comment:**
*Logic Error: The tooltip generation script never actually reads the intended `index.js` file and may also append unrelated `node index.js --version` output into the JSON, because the Node snippet is invoked as `node - <<'EOF'` and then uses `process.argv[1]` (which is `'-'` in this mode) as the path, and also runs a second `node "$APP_DIR/index.js" --version` inside the same command substitution; this causes the extraction to fail and always fall back to the placeholder data instead of the real `DEFAULT_TIPS_LIST`.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Orphaned command inside command substitution will cause issues.
Line 49 (node "$APP_DIR/index.js" --version ...) appears after the heredoc ends but is still within the $(...) command substitution. This line:
- Executes the entire
index.jsfile (not just parsing), which may start the bot or have side effects - Its output gets appended to
TOOLTIP_JSON, corrupting the JSON
🐛 Proposed fix — remove orphaned line
TOOLTIP_JSON=$(node - <<'EOF'
const fs = require('fs');
const src = fs.readFileSync(process.argv[1] || 'index.js', 'utf8');
// Execute just the DEFAULT_TIPS_LIST block and print it as JSON
const m = src.match(/const DEFAULT_TIPS_LIST\s*=\s*(\[[\s\S]+?\]);/);
if (!m) { process.stderr.write('DEFAULT_TIPS_LIST not found\n'); process.exit(1); }
try {
// Use Function constructor for safe eval of the array literal
const list = (new Function('return ' + m[1]))();
console.log(JSON.stringify(list, null, 2));
} catch (e) { process.stderr.write('Parse error: ' + e.message + '\n'); process.exit(1); }
EOF
-node "$APP_DIR/index.js" --version 2>/dev/null || 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.
| TOOLTIP_JSON=$(node - <<'EOF' | |
| const fs = require('fs'); | |
| const src = fs.readFileSync(process.argv[1] || 'index.js', 'utf8'); | |
| // Execute just the DEFAULT_TIPS_LIST block and print it as JSON | |
| const m = src.match(/const DEFAULT_TIPS_LIST\s*=\s*(\[[\s\S]+?\]);/); | |
| if (!m) { process.stderr.write('DEFAULT_TIPS_LIST not found\n'); process.exit(1); } | |
| try { | |
| // Use Function constructor for safe eval of the array literal | |
| const list = (new Function('return ' + m[1]))(); | |
| console.log(JSON.stringify(list, null, 2)); | |
| } catch (e) { process.stderr.write('Parse error: ' + e.message + '\n'); process.exit(1); } | |
| EOF | |
| node "$APP_DIR/index.js" --version 2>/dev/null || true | |
| ) || { | |
| # Fallback: emit a minimal valid tooltips.json with a placeholder | |
| warn "Could not extract tooltips from index.js — writing placeholder." | |
| TOOLTIP_JSON='[{"id":1,"text":"Helpful tooltip placeholder — configure via /tips in the bot admin.","enabled":true}]' | |
| } | |
| TOOLTIP_JSON=$(node - <<'EOF' | |
| const fs = require('fs'); | |
| const src = fs.readFileSync(process.argv[1] || 'index.js', 'utf8'); | |
| // Execute just the DEFAULT_TIPS_LIST block and print it as JSON | |
| const m = src.match(/const DEFAULT_TIPS_LIST\s*=\s*(\[[\s\S]+?\]);/); | |
| if (!m) { process.stderr.write('DEFAULT_TIPS_LIST not found\n'); process.exit(1); } | |
| try { | |
| // Use Function constructor for safe eval of the array literal | |
| const list = (new Function('return ' + m[1]))(); | |
| console.log(JSON.stringify(list, null, 2)); | |
| } catch (e) { process.stderr.write('Parse error: ' + e.message + '\n'); process.exit(1); } | |
| EOF | |
| ) || { | |
| # Fallback: emit a minimal valid tooltips.json with a placeholder | |
| warn "Could not extract tooltips from index.js — writing placeholder." | |
| TOOLTIP_JSON='[{"id":1,"text":"Helpful tooltip placeholder — configure via /tips in the bot admin.","enabled":true}]' | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@generate_tooltips.sh` around lines 37 - 54, The command substitution that
populates TOOLTIP_JSON accidentally contains an orphaned line that runs node
"$APP_DIR/index.js" --version inside the $(...) block, causing the full app to
execute and corrupt the JSON; remove that orphaned line from the heredoc/block
(or move it outside the command substitution and run it separately if you need
the version check), ensuring the only commands inside the $(...) are the inline
node script that extracts DEFAULT_TIPS_LIST and the subsequent JSON-producing
logic so TOOLTIP_JSON contains only valid JSON.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Terminology migration is incomplete in the authoritative map.
Line 14 switches to “Helpful Tooltips,” but other sections in this map still use “Content drops” (for example Line 111 and Line 123), which makes the source-of-truth internally inconsistent.
Based on learnings: "Applies to RUNEWAGER_FUNCTIONALITY_MAP.md : Always treat RUNEWAGER_FUNCTIONALITY_MAP.md as the authoritative source of truth before and after code changes."
🤖 Prompt for AI Agents