Ensure prod-run.sh creates required log files and supports legacy log path - #48
Conversation
|
CodeAnt AI is reviewing your PR. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
codeant-ai
Bot
added
the
size:S
Sequence DiagramThe PR makes prod-run.sh create required log files (including a legacy path) early, centralizes SYSTEMD_STDOUT_LOG/LOG_DIR for systemd and logrotate templates, and falls back to the legacy log when producing human-readable diagnostics. This diagram shows the main success path of those changes. sequenceDiagram
participant Operator
participant prod-run.sh
participant Filesystem
participant ServiceManager
participant Logrotate
Operator->>prod-run.sh: start main()
prod-run.sh->>Filesystem: ensure_log_paths (create logs dir, BOT_LOG, CRASH_LOG, SYSTEMD_STDOUT_LOG, ensure legacy BOT file)
prod-run.sh->>ServiceManager: ensure_systemd_service (write/regenerate unit with StandardOutput=append:$SYSTEMD_STDOUT_LOG; daemon-reload)
prod-run.sh->>Logrotate: ensure_logrotate_config (write/replace config referencing $LOG_DIR)
prod-run.sh->>Filesystem: print_human_errors (tail BOT_LOG; if empty read LEGACY_BOT_LOG)
prod-run.sh-->>Operator: report status / diagnostics
Generated by CodeAnt AI |
Nitpicks 🔍
|
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| ensure_dir "$LOG_DIR" | ||
| ensure_file "$BOT_LOG" | ||
| ensure_file "$CRASH_LOG" | ||
| ensure_file "$SYSTEMD_STDOUT_LOG" |
There was a problem hiding this comment.
Suggestion: Pre-creating the systemd stdout log file in ensure_log_paths() can cause permission errors when the script is run as a different user than the systemd service user: if prod-run.sh is ever run manually as root, it will create $SYSTEMD_STDOUT_LOG as a root-owned file, and later when systemd starts the service as the runewager user with StandardOutput=append:$SYSTEMD_STDOUT_LOG, systemd will fail to append to this file with "permission denied", preventing the service from starting correctly. Letting systemd create and own its own stdout log file avoids this mismatch. [logic error]
Severity Level: Critical 🚨
- ❌ Systemd runewager service fails to start on production VPS.
- ❌ Telegram bot offline; /health and /metrics endpoints unavailable.
- ⚠️ Manual prod-run.sh diagnostics can silently break systemd logging.| ensure_file "$SYSTEMD_STDOUT_LOG" |
Steps of Reproduction ✅
1. Set up the production deployment under `/var/www/html/Runewager` following
`README.md:102-155`, which creates the `runewager` system user and the app directory and
installs the provided unit file `/var/www/html/Runewager/runewager.service`.
2. Inspect the unit file `/var/www/html/Runewager/runewager.service` (lines 8-12 and 18 in
that file): it runs `ExecStart=/var/www/html/Runewager/prod-run.sh` as `User=runewager`
and `Group=runewager`, with
`StandardOutput=append:/var/www/html/Runewager/logs/runewager-1.log`.
3. As `root` (or any user different from `runewager`), run the startup script manually in
the app directory: `cd /var/www/html/Runewager && ./prod-run.sh`. In
`prod-run.sh:454-466`, `main()` calls `ensure_log_paths`, which in turn calls `ensure_file
"$SYSTEMD_STDOUT_LOG"` at `prod-run.sh:41`, creating or truncating
`/var/www/html/Runewager/logs/runewager-1.log` owned by `root` (typically mode `0644
root:root`).
4. Restart the systemd service: `sudo systemctl restart runewager`. Systemd now starts
`prod-run.sh` as `User=runewager` (unit lines 8-12) and tries to append to
`/var/www/html/Runewager/logs/runewager-1.log` per
`StandardOutput=append:/var/www/html/Runewager/logs/runewager-1.log` (unit line 18), but
the file is owned by `root` and not writable by `runewager`, so systemd fails to open the
stdout log with a permission denied error and the `runewager` service fails to start.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** prod-run.sh
**Line:** 41:41
**Comment:**
*Logic Error: Pre-creating the systemd stdout log file in `ensure_log_paths()` can cause permission errors when the script is run as a different user than the systemd service user: if `prod-run.sh` is ever run manually as root, it will create `$SYSTEMD_STDOUT_LOG` as a root-owned file, and later when systemd starts the service as the `runewager` user with `StandardOutput=append:$SYSTEMD_STDOUT_LOG`, systemd will fail to append to this file with "permission denied", preventing the service from starting correctly. Letting systemd create and own its own stdout log file avoids this mismatch.
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.
|
CodeAnt AI finished reviewing your PR. |
1 participant
User description
Motivation
runewager.logfor backward compatibility.Description
LEGACY_BOT_LOG,CRASH_LOG,SYSTEMD_STDOUT_LOG) and implementedensure_log_paths()to create thelogsdirectory and all expected log files before other checks.ensure_log_paths()intomain()so log creation runs early in startup.$SYSTEMD_STDOUT_LOGvariable forStandardOutputand to regenerate stale units that do not match the detectedPROJECT_DIR/LOG_DIR.ensure_logrotate_config()emit a$LOG_DIR-based template and replaced the logrotate file when it doesn't reference the detectedLOG_DIR, and added diagnostics fallback inprint_human_errors()to read the legacy log if the primary log is empty.Testing
bash -n prod-run.sh, which passed.prod-run.shdiff to confirm the new variables,ensure_log_paths()invocation, and updated systemd/logrotate templates were applied as intended.Codex Task
CodeAnt-AI Description
Ensure prod-run creates required log files and supports legacy runewager.log
What Changed
Impact
✅ Fewer startup failures on fresh clones✅ Clearer diagnostics when logs are missing✅ Backwards-compatible log location for older deployments💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.