Repository navigation
feat: omnidash lifecycle script + container error scanner [OMN-5141] - #845
Conversation
…5141] - omnidash-lifecycle.sh: start/stop/restart/status for local omnidash dev server with health check polling and bus mode selection (OMN-5142) - container-error-to-linear.py: scan Docker containers for persistent errors (restart loops, OOM kills, crash loops, log errors) with dry-run output suitable for Linear ticket creation (OMN-5150)
📝 WalkthroughWalkthroughTwo new utility scripts are introduced: a Python script that discovers omnibase-infra containers, analyzes logs for error patterns, and generates structured issue data with Linear ticket suggestions; and a Bash script that provides lifecycle management (start, stop, restart, status) for a local omnidash development server with health-check verification. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
scripts/omnidash-lifecycle.sh (2)
24-24: Remove unused variableSCRIPT_DIR.
SCRIPT_DIRis defined but never referenced. Either remove it or use it (e.g., for sourcing configs relative to the script).Proposed fix
-SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/omnidash-lifecycle.sh` at line 24, The variable SCRIPT_DIR is defined but never used; either remove the assignment line for SCRIPT_DIR or use it when resolving relative paths (e.g., use $SCRIPT_DIR to source configs or reference files relative to the script) so the definition is meaningful; update any subsequent sourcing or file references to use SCRIPT_DIR (or simply delete the SCRIPT_DIR line if you choose not to use it).
43-48: Hardcoded developer-specific fallback path reduces portability.The path
/Volumes/PRO-G40/Code/omni_home/omnidashis specific to one machine. Other contributors will hit the error message. Consider removing this fallback or documenting it as a local-only override.Proposed fix — remove hardcoded fallback
- # Hardcoded fallback - local fallback="/Volumes/PRO-G40/Code/omni_home/omnidash" - if [[ -f "${fallback}/package.json" ]]; then - echo "${fallback}" - return 0 - fi - echo "[omnidash] ERROR: Cannot find omnidash directory. Set OMNIDASH_DIR." >&2 return 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/omnidash-lifecycle.sh` around lines 43 - 48, The hardcoded developer-specific fallback using the local variable fallback and the check [[ -f "${fallback}/package.json" ]] should be removed to avoid machine-specific behavior; replace it with a configurable option (e.g., check an environment variable like OMNIDASH_HOME or OMNIDASH_FALLBACK first and only use it when set) or simply remove the fallback block altogether. Locate the block that declares local fallback="/Volumes/PRO-G40/Code/omni_home/omnidash" and the subsequent if [[ -f "${fallback}/package.json" ]]; then ... return 0; fi and delete that hardcoded assignment and conditional, or change it to read from an env var and validate that env var before echoing and returning. Ensure any new env var is documented in the script header or README.scripts/container-error-to-linear.py (3)
273-284: Consider raising an error or exiting non-zero when--executeis used but unimplemented.Currently,
--executesilently proceeds with a warning. Users expecting ticket creation may not notice it didn't happen. Considersys.exit(2)to signal the feature is unavailable.Proposed fix
if args.execute and issues: print( "[container-error-to-linear] --execute: Linear ticket creation not yet implemented.", file=sys.stderr, ) print( "[container-error-to-linear] Ticket data is available in --json output for manual creation.", file=sys.stderr, ) + sys.exit(2) # Signal that requested action was not performed # Future: integrate with Linear MCP tools or API # For now, dry-run outputs the ticket data so it can be piped to the Linear MCP🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/container-error-to-linear.py` around lines 273 - 284, When args.execute is True and issues exist, the script currently only prints warnings; change this to fail fast by exiting with a non-zero status: after printing the two stderr messages in the block that checks "if args.execute and issues:" call sys.exit(2) (or raise SystemExit(2)) so callers can detect the unimplemented execute path; ensure you still print the explanatory messages before exiting and reference the symbols args.execute and the if block handling issues to locate where to add the exit.
88-101: Minor optimization: batchdocker inspectcalls.Each container triggers a separate
docker inspectsubprocess. For many containers, this could be batched:inspect_json = run_cmd(["docker", "inspect"] + names)This is optional since the current approach works correctly and the container count is typically small.
57-69: Consider logging stderr on failure for debugging.
run_cmdswallows errors silently. This is fine for the current use case, but loggingresult.stderrto stderr on non-zero exit codes could aid debugging when Docker commands fail unexpectedly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/container-error-to-linear.py` around lines 57 - 69, The run_cmd function currently returns an empty string on failure without showing error output, which can hinder debugging. Modify run_cmd so that if subprocess.run completes with a non-zero exit code, it logs result.stderr to standard error before returning an empty string while keeping the existing exception handling. This will help surface Docker command error messages for easier troubleshooting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/omnidash-lifecycle.sh`:
- Line 37: The if-statement checking for package.json uses an incorrect
directory test ([-d]) which is always false; update the condition that
references "${candidate}/package.json" in the if (the line containing if [[ -d
"${candidate}/package.json" ]] || [[ -f "${candidate}/package.json" ]]) to
remove the -d check and only test for a regular file (use [[ -f
"${candidate}/package.json" ]] ) or a generic existence test ([[ -e ... ]]) as
appropriate so the branch correctly triggers for package.json files.
- Around line 58-63: Argument parsing crashes when --bus is provided without a
following value; update the case for --bus to validate that a next argument
exists and is not another option before assigning bus_mode and shifting.
Specifically, in the while/case block check if "${2-}" is empty or starts with
'-' (e.g. if [[ -z "${2-}" || "${2#-}" != "$2" ]]) and if so print an error and
return 1; otherwise set bus_mode="${2}" and shift 2. This keeps the existing
while/case structure, references the --bus branch, the bus_mode variable, and
the shift 2 usage, and prevents failures under set -u.
- Around line 17-18: The header comment incorrectly claims the script sources
~/.omnibase/.env but it doesn't; either add the actual sourcing or remove/update
the comment. Fix by inserting a sourcing step near the top of
omnidash-lifecycle.sh (before any database/Kafka/Infisical operations) such as
sourcing ~/.omnibase/.env (e.g., source "$HOME/.omnibase/.env" with a safe
fallback or error handling) so environment vars are available to the script, or
alternatively remove the misleading comment if you decide not to source the
file.
---
Nitpick comments:
In `@scripts/container-error-to-linear.py`:
- Around line 273-284: When args.execute is True and issues exist, the script
currently only prints warnings; change this to fail fast by exiting with a
non-zero status: after printing the two stderr messages in the block that checks
"if args.execute and issues:" call sys.exit(2) (or raise SystemExit(2)) so
callers can detect the unimplemented execute path; ensure you still print the
explanatory messages before exiting and reference the symbols args.execute and
the if block handling issues to locate where to add the exit.
- Around line 57-69: The run_cmd function currently returns an empty string on
failure without showing error output, which can hinder debugging. Modify run_cmd
so that if subprocess.run completes with a non-zero exit code, it logs
result.stderr to standard error before returning an empty string while keeping
the existing exception handling. This will help surface Docker command error
messages for easier troubleshooting.
In `@scripts/omnidash-lifecycle.sh`:
- Line 24: The variable SCRIPT_DIR is defined but never used; either remove the
assignment line for SCRIPT_DIR or use it when resolving relative paths (e.g.,
use $SCRIPT_DIR to source configs or reference files relative to the script) so
the definition is meaningful; update any subsequent sourcing or file references
to use SCRIPT_DIR (or simply delete the SCRIPT_DIR line if you choose not to use
it).
- Around line 43-48: The hardcoded developer-specific fallback using the local
variable fallback and the check [[ -f "${fallback}/package.json" ]] should be
removed to avoid machine-specific behavior; replace it with a configurable
option (e.g., check an environment variable like OMNIDASH_HOME or
OMNIDASH_FALLBACK first and only use it when set) or simply remove the fallback
block altogether. Locate the block that declares local
fallback="/Volumes/PRO-G40/Code/omni_home/omnidash" and the subsequent if [[ -f
"${fallback}/package.json" ]]; then ... return 0; fi and delete that hardcoded
assignment and conditional, or change it to read from an env var and validate
that env var before echoing and returning. Ensure any new env var is documented
in the script header or README.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e20b093e-0abc-4bf8-b31a-1e9819e12b01
📒 Files selected for processing (2)
scripts/container-error-to-linear.pyscripts/omnidash-lifecycle.sh
Summary
omnidash-start,omnidash-stop,omnidash-restart,omnidash-status) added to~/.zshrc.Test plan
omnidash-lifecycle.sh statusto verify status checkomnidash-lifecycle.sh startto verify server startupcontainer-error-to-linear.py --jsonto verify container scanningsource ~/.zshrc && omnidash-statusSummary by CodeRabbit