Skip to content

Revert "Claude/update prod runner script lq2g s (#62)" #63

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.

Sign up for GitHub

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

Closed
gamblecodezcom wants to merge 1 commit into from
Closed

Revert "Claude/update prod runner script lq2g s (#62)" #63

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 1 addition & 10 deletions deploy.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,11 @@ say "Date: $(date -u '+%Y-%m-%dT%H:%M:%SZ')"
cd "$PROJECT_DIR"

# ---------------------------------------------------------
# 0) Stop service before touching files (prevents file locks)
# ---------------------------------------------------------
if command -v systemctl >/dev/null 2>&1; then
say "Step 0/4 — Stopping ${APP_NAME} service…"
systemctl stop "${APP_NAME}.service" || true
fi

# ---------------------------------------------------------
# 1) Pull latest code from origin/main (hard reset — no merge conflicts)
# 1) Pull latest code from origin/main
# ---------------------------------------------------------
say "Step 1/4 — Fetching latest code from origin/main…"
git fetch --all
git reset --hard origin/main
git clean -fd
say "Now at: $(git rev-parse --short HEAD)"

# ---------------------------------------------------------
Expand Down
14 changes: 4 additions & 10 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2526,17 +2526,11 @@ bot.command('deploy', async (ctx) => {
.editMessageText(ctx.chat.id, status.message_id, null, text, { parse_mode: 'Markdown' })
.catch(() => {});

// ── Step 1: fetch + hard reset (avoids hang on dirty tree / merge conflicts) ──
const fetchResult = await runCmd('git', ['fetch', '--all'], PROJECT_DIR, 30000);
const gitResult = fetchResult.ok
? await runCmd('git', ['reset', '--hard', 'origin/main'], PROJECT_DIR, 15000)
: fetchResult;
if (gitResult.ok) {
await runCmd('git', ['clean', '-fd'], PROJECT_DIR, 10000).catch(() => {});
}
// ── Step 1: git pull ──────────────────────────────────────────
const gitResult = await runCmd('git', ['pull', 'origin', 'main'], PROJECT_DIR, 30000);
const gitLine = gitResult.ok
? `✅ *git update:* \`${sanitizeCmdOutput(gitResult.out.split('\n')[0])}\``
: `⚠️ *git update failed:* \`${sanitizeCmdOutput(gitResult.err || fetchResult.err)}\``;
? `✅ *git pull:* \`${sanitizeCmdOutput(gitResult.out.split('\n')[0])}\``
: `⚠️ *git pull failed:* \`${sanitizeCmdOutput(gitResult.err)}\``;

await edit(
`🚀 *Deployment in progress* — ${ts()}\n\n`
Expand Down
12 changes: 5 additions & 7 deletions prod-run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,13 @@ say "Project directory: $PROJECT_DIR"
say "Running as: $(id -un) (uid=$(id -u) gid=$(id -g))"

# ---------------------------------------------------------
# 1) Fetch + hard reset to origin/main [FIRST — before anything]
# Using fetch+reset instead of pull avoids hangs on dirty trees / conflicts.
# 1) Pull latest code from origin main [FIRST — before anything]
# ---------------------------------------------------------
say "Fetching latest code from origin main..."
if git -C "$PROJECT_DIR" fetch --all 2>&1 && git -C "$PROJECT_DIR" reset --hard origin/main 2>&1; then
git -C "$PROJECT_DIR" clean -fd 2>&1 || true
say "Git fetch + reset successful: $(git -C "$PROJECT_DIR" rev-parse --short HEAD)"
say "Pulling latest code from origin main..."
if git -C "$PROJECT_DIR" pull origin main 2>&1; then
say "Git pull successful"
else
warn "Git fetch/reset failed — continuing with local copy"
warn "Git pull failed — continuing with local copy"
Comment on lines +43 to +47

@codeant-ai codeant-ai Bot Feb 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Using git pull in this unattended script means that if the remote main branch diverges and a merge conflict occurs, the command will leave the working tree in a conflicted/partially-updated state but the script only logs a warning and continues to restart the service, so the app may run with broken files containing merge markers. A safer pattern is to use git fetch plus git reset --hard origin/main (like the deploy script) and abort on failure so the service never runs against a conflicted tree. [logic error]

Severity Level: Major ⚠️ 
- ❌ prod-run.sh may restart bot with conflicted source files.
- ❌ Telegram bot downtime if Node fails to start.
- ⚠️ /health and /metrics endpoints unavailable during failure. 
- ⚠️ Divergent prod repo state harder to recover reliably.
Suggested change
say "Pulling latest code from origin main..."
if git -C "$PROJECT_DIR" pull origin main 2>&1; then
say "Git pull successful"
else
warn "Git fetch/reset failed — continuing with local copy"
warn "Git pull failed — continuing with local copy"
say "Pulling latest code from origin/main..."
if git -C "$PROJECT_DIR" fetch origin main 2>&1 && \
git -C "$PROJECT_DIR" reset --hard origin/main 2>&1; then
say "Git update successful"
else
err "Git update failed — aborting to avoid running with a conflicted or partial checkout"
exit 1
Steps of Reproduction ✅ 
1. On the production host, ensure the bot is deployed under `/var/www/html/Runewager` as
described in `README.md:102-124`, and that `prod-run.sh` is present (verified at
`/workspace/Runewager/prod-run.sh:1-365`).

2. From the project directory, create a local change that will conflict with an upcoming
remote change (e.g., edit `index.js` and commit it locally), so the local branch diverges
from `origin/main` while still using the same repo (any normal git workflow on this
directory, since `prod-run.sh` uses `$PROJECT_DIR` as resolved at `prod-run.sh:16-23`).

3. Push a conflicting change to `origin/main` from another environment so that a
subsequent `git pull origin main` from the server will produce merge conflicts (standard
git behavior when local and remote modify overlapping sections).

4. On the server, run `npm run prod` (entrypoint defined in `package.json:10-13` as
`./prod-run.sh`) or execute `./prod-run.sh` directly; when `prod-run.sh` reaches the block
at `prod-run.sh:43-48`, `git -C "$PROJECT_DIR" pull origin main` attempts the merge, fails
with conflicts, returns non-zero, and leaves the working tree with conflict markers, but
because the command is used in an `if ... then` test (exempt from `set -e` at
`prod-run.sh:9`), the script only logs `warn "Git pull failed — continuing with local
copy"` and continues into the restart logic at `prod-run.sh:313-340`, attempting to
restart `node index.js` against a conflicted/partially-updated tree, which can cause
runtime syntax errors or a failed start.
Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** prod-run.sh
**Line:** 43:47
**Comment:**
	*Logic Error: Using `git pull` in this unattended script means that if the remote `main` branch diverges and a merge conflict occurs, the command will leave the working tree in a conflicted/partially-updated state but the script only logs a warning and continues to restart the service, so the app may run with broken files containing merge markers. A safer pattern is to use `git fetch` plus `git reset --hard origin/main` (like the deploy script) and abort on failure so the service never runs against a conflicted tree.

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.
👍 | 👎

fi

# ---------------------------------------------------------
Expand Down
Loading