[codex] Hide sys log follower unit from argv#2
Conversation
There was a problem hiding this comment.
Pull request overview
This PR changes sys log follow mode to avoid leaving the target unit name in long-lived process argv, addressing false process matches by services that inspect command lines.
Changes:
- Adds a safe follow implementation using a short
journalctl -uprelude and a long-lived JSON stream filtered by Python. - Rejects follow-mode options that would conflict with the internal JSON follower or expose unit names in argv.
- Updates help/docs and adds an install-time
python3dependency warning.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
sys |
Adds the safe log follower and routes default sys log follow mode through it. |
README.md |
Documents argv-hiding behavior, follow-mode restrictions, and python3 dependency. |
install.sh |
Warns when required python3 is missing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| -f|--follow) | ||
| echo "sys: log 명령은 기본적으로 follow 모드라서 $arg 옵션을 따로 받을 수 없어요." >&2 | ||
| exit 1 | ||
| ;; | ||
| --follow=*) | ||
| echo "sys: log 명령은 기본적으로 follow 모드라서 $arg 옵션을 따로 받을 수 없어요." >&2 | ||
| exit 1 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a82ea8df3b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| else | ||
| journal_cmd+=(-n 0) | ||
| fi | ||
| journal_cmd+=("$@") |
There was a problem hiding this comment.
When a user runs sys log <unit> --no-tail, journalctl defines --no-tail as showing all lines even in follow mode. The prelude already receives that option and prints the unit's history, but passing the same option through here makes the unscoped follower replay the entire journal again before Python filters it, so the target unit's historical logs are emitted a second time and the command scans all journals unnecessarily. This regresses the documented journalctl passthrough behavior for --no-tail; either consume it in the prelude path or reject it like the other unsupported follow options.
Useful? React with 👍 / 👎.
| arg="${filtered_args[$i]}" | ||
| case "$arg" in | ||
| -n|--lines) | ||
| if [[ $((i + 1)) -ge ${#filtered_args[@]} || ! "${filtered_args[$((i + 1))]}" =~ ^([0-9]+|all)$ ]]; then |
There was a problem hiding this comment.
This new validation rejects sys log <unit> -n +100 and the --lines=+100/-n+100 variants even though these were previously passed directly to journalctl and are valid; I checked journalctl --help, which advertises -n --lines[=[+]INTEGER]. In follow mode this now exits before journalctl runs, so users relying on journalctl's leading-plus line-count semantics lose a supported passthrough option.
Useful? React with 👍 / 👎.
| if [[ -n "${SYS_LOG_AFTER_CURSOR:-}" ]]; then | ||
| journal_cmd+=(-n all --after-cursor "$SYS_LOG_AFTER_CURSOR") | ||
| else | ||
| journal_cmd+=(-n 0) |
There was a problem hiding this comment.
If the prelude finds no matching entry, SYS_LOG_AFTER_CURSOR is empty and the long-lived follower starts with -n 0; journalctl --help describes -n as the number of entries to show, so this only follows entries that arrive after that second journalctl process is attached. Because the new implementation splits the old single journalctl -f -u invocation into a prelude and a later unscoped follower, any first log line written by the unit in the gap between those two commands is silently missed.
Useful? React with 👍 / 👎.
Summary
sys logfollow mode so the long-livedjournalctlprocess no longer receives-u <unit>in argv.python3dependency warning.Root Cause
sys log hl-visorkept a persistent process likejournalctl --user -o cat -f -u hl-visor.hl-visorappears to scan process command lines broadly, so it interpreted the log follower as a second matchinghl-visorprocess during restart and panicked.Validation
bash -n sys install.sh uninstall.shgit diff --checksystemctl/journalctlharness confirming defaultsys log hl-visoroutput still follows matching records while persistentjournalctl/filter process argv does not containhl-visor.-f,-o,--unit, and unit-name-containing passthrough args.Closes #1