feat: single-command production installer + versioned releases#25
Merged
feat: single-command production installer + versioned releases#25
Conversation
Adds a curl-able installer (install.sh) that downloads the latest GitHub release tarball, runs npm ci --omit=dev, prompts for Discord credentials, deploys slash commands, and registers a systemd user service (Linux) or launchd agent (macOS). Re-running upgrades in place and preserves ~/.config/maestro-discord/.env. Adds maestro-discord-ctl wrapper exposing start/stop/restart/status/logs/ deploy/update/uninstall/version. Adds tag-triggered release.yml workflow that gates on lint+tests, packages a release tarball, and publishes a GitHub Release with auto-generated notes. Bumps package.json to 1.0.0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a GitHub release workflow, a user-facing installer and CLI control script, systemd/launchd service templates, README install/runtime updates, and bumps package version to 1.0.0. ChangesRelease Automation
Installer & Runtime Wiring
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as "User / Admin"
participant GH as "GitHub Releases"
participant Installer as "install.sh"
participant InstallDir as "Install Dir\n(maestro-discord-<tag>)"
participant ServiceMgr as "systemd / launchd"
participant App as "Node App\n(dist/index.js)"
rect rgba(200,230,255,0.5)
User->>Installer: run install.sh (tag/latest)
Installer->>GH: query release & download tarball (+.sha256)
GH-->>Installer: tarball (and checksum)
Installer->>InstallDir: extract, npm ci, write/link .env
Installer->>ServiceMgr: install unit/plist, enable/start
ServiceMgr->>App: launch Node entrypoint with env
App-->>User: bot runs (logs available)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (6)
templates/maestro-discord.service (1)
9-9: Optional: makeEnvironmentFiletolerant of a missing file.If a user accidentally removes
~/.config/maestro-discord/.env, the unit will fail to start with a confusing error. Prefixing with-tells systemd to silently ignore a missing file (bot will then surface its own "DISCORD_BOT_TOKEN is required" error, which is more diagnosable):♻️ Proposed change
-EnvironmentFile=@CONFIG_DIR@/.env +EnvironmentFile=-@CONFIG_DIR@/.env🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/maestro-discord.service` at line 9, The EnvironmentFile directive currently requires `@CONFIG_DIR`@/.env to exist and will cause the unit to fail if it's missing; change the EnvironmentFile line in the unit template so systemd tolerates a missing file by prefixing the path with a dash (i.e., use EnvironmentFile=-@CONFIG_DIR@/.env) so the service can start and let the application surface its own missing-variable errors; update the template line that defines EnvironmentFile to include the leading '-' character.install.sh (1)
59-69: Optional: tag-name parsing via regex is fragile; jq (if available) or grep -oP is sturdier.
sed -nE 's/.*"tag_name".*"([^"]+)".*/\1/p' | head -n1will parse the right field today, but if a future release-name body contains a quotedtag_namesubstring (e.g. in release notes), or if the API returns a draft with"tag_name": null, this returns wrong/empty values. Optional hardening: preferjq -r '.tag_name'if available, else fall back to the current regex.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@install.sh` around lines 59 - 69, In resolve_release(), replace the fragile sed extraction of tag_name with a robust approach: if jq is available, use it to extract .tag_name (e.g., jq -r '.tag_name') and validate it is non-null/non-empty; otherwise fall back to a safer grep/grep -Po pattern that matches the tag_name field only (and not occurrences in release body) and treat "null" as empty; ensure the resulting tag variable is validated (non-empty and not "null") and call die with the existing api_url context if resolution fails..github/workflows/release.yml (2)
39-43: Optional:Buildstep is redundant afterTest.
npm testisnpm run build && node --test dist/...(perpackage.jsonline 14), so the standalonenpm run buildstep on line 43 just re-runstscagainst the already-builtdist/. Harmless but adds ~10s. Either drop the duplicate build, or split test from build to make the pipeline phases explicit (e.g. add a"test:only"script that doesn't rebuild).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 39 - 43, The workflow currently runs "npm test" which already performs "npm run build" (per the package.json "test" script), then runs a redundant "Build" step that calls "npm run build" again; remove the duplicate step or update scripts so phases are explicit. Either delete the "Build" job/step that runs "npm run build" in the release.yml, or change the package.json to add a separate "test:only" (or similar) script and update the release.yml to run "npm run build" then "npm run test:only" so build and test are distinct.
36-37: Step name says "Lint and type-check" but only type-checks.The step runs
npx tsc --noEmitonly — noeslint, despiteeslint(^10.2.0) andtypescript-eslint(^8.58.1) being declared as devDependencies witheslint.config.mjsconfigured. Either add the lint command or rename the step to "Type-check":Proposed change
- name: Lint and type-check - run: npx tsc --noEmit + run: | + npx eslint . + npx tsc --noEmit🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 36 - 37, The workflow step named "Lint and type-check" only runs the TypeScript type-check command `npx tsc --noEmit`; update the step so it actually performs both linting and type-checking by adding the eslint invocation (e.g., run `npx eslint .` or the project’s preferred eslint command) alongside `npx tsc --noEmit`, or change the step name to "Type-check" to accurately reflect that it only runs `npx tsc --noEmit`; adjust the step label and/or commands accordingly in the workflow where the step currently runs `npx tsc --noEmit`.README.md (1)
25-27: Optional: document a verified-download alternative tocurl | bash.The default one-liner is fine for general use, but a sizable subset of operators won't run
curl | bashand would benefit from an opt-in two-step flow (download → inspect → run) plus the SHA256 sidecar that the new release workflow already publishes (maestro-discord-vX.Y.Z.tar.gz.sha256). Consider adding a "Verify before running" snippet so the published checksum isn't only available, but discoverable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 25 - 27, Add an opt-in "Verify before running" alternative next to the existing curl|bash one-liner in README.md: describe a two-step flow that downloads the release tarball and its published SHA256 sidecar (maestro-discord-vX.Y.Z.tar.gz and maestro-discord-vX.Y.Z.tar.gz.sha256), instructs the user to inspect the archive (e.g., list or extract to a temp dir) and then verify integrity using sha256sum -c or equivalent before running the install script, and finally run the installer only after verification; keep the text short, replace X.Y.Z with the release tag pattern and show the sequence of download → inspect → verify → run.bin/maestro-discord-ctl.sh (1)
114-117: Minor:cmd_updatecan't propagateMAESTRO_DISCORD_VERSIONthrough curl | bash.
updatealways pullsinstall.shfrom themainbranch and pipes it to a fresh shell, which doesn't inherit non-exported env. Also, the new shell runs in non-interactive mode (no tty) so it skips slash-command redeploy. For anupdateuse case this is mostly desirable, but if a user wantsmaestro-discord-ctl updateto pin a version, they'd have toMAESTRO_DISCORD_VERSION=v1.2.3 maestro-discord-ctl updateand ensure the var is exported. Worth a one-lineexport+ a doc note.♻️ Suggested polish
cmd_update() { info "Re-running installer to pull the latest release" - curl -fsSL "https://raw.githubusercontent.com/${REPO}/main/install.sh" | bash + export MAESTRO_DISCORD_HOME MAESTRO_DISCORD_VERSION MAESTRO_DISCORD_BIN_DIR XDG_CONFIG_HOME MAESTRO_DISCORD_REPO + curl -fsSL "https://raw.githubusercontent.com/${REPO}/main/install.sh" | bash }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bin/maestro-discord-ctl.sh` around lines 114 - 117, In cmd_update(), ensure the MAESTRO_DISCORD_VERSION environment variable is exported so the subshell that runs the downloaded install.sh inherits it; before piping curl to bash, add an export of MAESTRO_DISCORD_VERSION when it's set (e.g., export MAESTRO_DISCORD_VERSION="${MAESTRO_DISCORD_VERSION}") so the installer can honor a pinned version, and keep the existing curl ... | bash invocation that targets install.sh.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Line 53: The release step currently uses a single cp command with "2>/dev/null
|| true" which hides failures (notably a missing LICENSE); update the workflow
so required files (package.json, package-lock.json, .env.example, README.md,
LICENSE) are copied without swallowing errors — remove the "|| true" and avoid
redirecting stderr for that cp invocation so the job fails if any required file
is missing — and if there are truly optional files keep them in a separate cp
that uses "|| true" or conditional checks; target the cp line that contains 'cp
package.json package-lock.json .env.example README.md LICENSE "$staging/"
2>/dev/null || true'.
In `@bin/maestro-discord-ctl.sh`:
- Around line 96-106: In cmd_logs (macOS branch) avoid dying when
INSTALL_DIR/logs/maestro-discord.log is missing: if the file doesn't exist,
create it (touch the log_file) or fall back to using the system log command
(e.g., log show/log stream filtering by process name) instead of calling die;
modify the macos case in cmd_logs to check [ -f "$log_file" ] and on false
either touch "$log_file" (so tail -f will work) or run a fallback like log
show/stream for SERVICE_NAME/node, and ensure any error uses die only if both
the file creation and fallback fail.
- Around line 126-130: In the linux branch of the detect_os case, disable the
user unit before removing its unit file: call systemctl --user disable
"${SERVICE_NAME}.service" (or the equivalent using SERVICE_NAME) and ignore
failures, then remove the file and run systemctl --user daemon-reload; update
the linux case in bin/maestro-discord-ctl.sh so the disable command runs prior
to rm -f and before the existing systemctl --user daemon-reload call to avoid
leaving broken symlinks.
- Around line 7-12: The script is missing the BIN_DIR variable so uninstall uses
a hardcoded $HOME/.local/bin path; add
BIN_DIR="${MAESTRO_DISCORD_BIN_DIR:-$HOME/.local/bin}" (same logic as
install.sh) near the other top-level assignments (with INSTALL_DIR, CONFIG_DIR,
REPO, SERVICE_NAME, LAUNCHD_LABEL) and replace the hardcoded uninstall target
(the path used on line 134 to remove the maestro-discord-ctl symlink) with
"$BIN_DIR/maestro-discord-ctl" (and any other places in the file that reference
the fixed $HOME/.local/bin) so custom MAESTRO_DISCORD_BIN_DIR installs are
correctly cleaned up.
In `@install.sh`:
- Around line 156-167: The heredoc writing to "$env_file" uses an unquoted
delimiter (<<EOF) so values like ${token}, ${client_id}, ${guild_id}, ${allowed}
can be corrupted by backslashes or shell expansions; change the write to safely
emit raw values (e.g., replace the heredoc with explicit safe writes using
printf '%s=%s\n' KEY "$value" for DISCORD_BOT_TOKEN, DISCORD_CLIENT_ID,
DISCORD_GUILD_ID, DISCORD_ALLOWED_USER_IDS, etc., or use a quoted heredoc like
<<'EOF' and populate the file by exporting/echoing each variable safely), and
ensure the created file is secured by writing with stricter permissions (use
install -m 0600 or redirect to a temp file then chmod 600 and mv to "$env_file")
to avoid relying solely on umask; update the code around the env_file write (the
block that currently builds the heredoc) accordingly.
- Around line 215-224: install_service_macos currently writes the plist but
doesn't ensure the log directory exists; add a step in install_service_macos to
create "$INSTALL_DIR/logs" (mkdir -p "$INSTALL_DIR/logs") before writing the
plist so the StandardOutPath/StandardErrorPath in
templates/sh.maestro.discord.plist can be opened by launchd; also either remove
the unused sed expression replacing `@CONFIG_DIR`@ in that function or replace it
by adding an EnvironmentVariables / dotenv reference in the plist if you intend
to pass CONFIG_DIR through the plist.
- Around line 88-98: The script moves the old install to a timestamped backup
(backup variable after mv "$INSTALL_DIR" "$backup") but does not restore it if
subsequent steps (cp -R "$extracted"/. "$INSTALL_DIR"/ or writing .version)
fail; add a short rollback trap immediately after the mv that on ERR moves
"$backup" back to "$INSTALL_DIR" (and preserves permissions), then clear the
trap on successful completion and optionally remove the backup on success;
ensure the trap is installed before cp and .version write and uninstalled after
both succeed so failures triggered by set -e will restore the backup
automatically.
- Around line 124-128: The prompt guard prevents interactive prompts when run as
curl ... | bash because stdin is the pipe; in prompt_var remove the surrounding
"if [ -t 0 ]" check so the function always attempts "read -r -p ... </dev/tty ||
true" (keep the existing read from /dev/tty), and in write_config and
deploy_commands replace their current stdin TTY checks ("[ ! -t 0 ]") with a
check for a real terminal device (e.g., "[ ! -c /dev/tty ]" or equivalent) so
they properly detect non-interactive environments while allowing prompts when
/dev/tty is available; update prompt_var, write_config, and deploy_commands
accordingly.
In `@templates/maestro-discord.service`:
- Around line 11-12: The service unit currently restarts on failure every 5s and
lacks start-rate limits; add start-limit guards in the [Unit] section of
templates/maestro-discord.service by defining StartLimitBurst (e.g., 5) and
StartLimitIntervalSec (e.g., 60) so systemd will stop rapid restart storms, and
optionally set StartLimitAction if a specific action is desired; update the unit
header (the [Unit] block) to include these keys alongside the existing
Restart/RestartSec settings.
In `@templates/sh.maestro.discord.plist`:
- Around line 21-24: install_service_macos() currently creates only the
LaunchAgents directory but not the log directory expected by the plist and
bin/maestro-discord-ctl.sh; modify install_service_macos() to create the logs
directory under $INSTALL_DIR (mkdir -p "$INSTALL_DIR/logs"), ensure proper
ownership/permissions, and create an empty log file (touch
"$INSTALL_DIR/logs/maestro-discord.log") so the plist paths
(StandardOutPath/StandardErrorPath) and maestro-discord-ctl.sh's log check will
succeed without errors.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 39-43: The workflow currently runs "npm test" which already
performs "npm run build" (per the package.json "test" script), then runs a
redundant "Build" step that calls "npm run build" again; remove the duplicate
step or update scripts so phases are explicit. Either delete the "Build"
job/step that runs "npm run build" in the release.yml, or change the
package.json to add a separate "test:only" (or similar) script and update the
release.yml to run "npm run build" then "npm run test:only" so build and test
are distinct.
- Around line 36-37: The workflow step named "Lint and type-check" only runs the
TypeScript type-check command `npx tsc --noEmit`; update the step so it actually
performs both linting and type-checking by adding the eslint invocation (e.g.,
run `npx eslint .` or the project’s preferred eslint command) alongside `npx tsc
--noEmit`, or change the step name to "Type-check" to accurately reflect that it
only runs `npx tsc --noEmit`; adjust the step label and/or commands accordingly
in the workflow where the step currently runs `npx tsc --noEmit`.
In `@bin/maestro-discord-ctl.sh`:
- Around line 114-117: In cmd_update(), ensure the MAESTRO_DISCORD_VERSION
environment variable is exported so the subshell that runs the downloaded
install.sh inherits it; before piping curl to bash, add an export of
MAESTRO_DISCORD_VERSION when it's set (e.g., export
MAESTRO_DISCORD_VERSION="${MAESTRO_DISCORD_VERSION}") so the installer can honor
a pinned version, and keep the existing curl ... | bash invocation that targets
install.sh.
In `@install.sh`:
- Around line 59-69: In resolve_release(), replace the fragile sed extraction of
tag_name with a robust approach: if jq is available, use it to extract .tag_name
(e.g., jq -r '.tag_name') and validate it is non-null/non-empty; otherwise fall
back to a safer grep/grep -Po pattern that matches the tag_name field only (and
not occurrences in release body) and treat "null" as empty; ensure the resulting
tag variable is validated (non-empty and not "null") and call die with the
existing api_url context if resolution fails.
In `@README.md`:
- Around line 25-27: Add an opt-in "Verify before running" alternative next to
the existing curl|bash one-liner in README.md: describe a two-step flow that
downloads the release tarball and its published SHA256 sidecar
(maestro-discord-vX.Y.Z.tar.gz and maestro-discord-vX.Y.Z.tar.gz.sha256),
instructs the user to inspect the archive (e.g., list or extract to a temp dir)
and then verify integrity using sha256sum -c or equivalent before running the
install script, and finally run the installer only after verification; keep the
text short, replace X.Y.Z with the release tag pattern and show the sequence of
download → inspect → verify → run.
In `@templates/maestro-discord.service`:
- Line 9: The EnvironmentFile directive currently requires `@CONFIG_DIR`@/.env to
exist and will cause the unit to fail if it's missing; change the
EnvironmentFile line in the unit template so systemd tolerates a missing file by
prefixing the path with a dash (i.e., use EnvironmentFile=-@CONFIG_DIR@/.env) so
the service can start and let the application surface its own missing-variable
errors; update the template line that defines EnvironmentFile to include the
leading '-' character.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9003e5b5-65c1-4c2c-a428-eccf16e2452e
📒 Files selected for processing (7)
.github/workflows/release.ymlREADME.mdbin/maestro-discord-ctl.shinstall.shpackage.jsontemplates/maestro-discord.servicetemplates/sh.maestro.discord.plist
- release.yml: split required vs optional file copies so missing README/package.json fails loudly; LICENSE stays best-effort - maestro-discord-ctl: add BIN_DIR env override, fix uninstall to use it, run systemctl disable + reset-failed before unit removal, auto-create logs/ + touch log on macOS for first-run UX - install.sh: add ERR trap in install_release to restore backup if extraction fails post-mv; switch [ -t 0 ] guards to [ -r /dev/tty ] so curl | bash prompts and slash command deploy actually run; write .env via mktemp+printf+chmod 600+mv (no shell expansion of values, explicit perms); mkdir logs/ in install_service_macos; drop no-op @CONFIG_DIR@ sed in plist install - maestro-discord.service: add StartLimitIntervalSec/StartLimitBurst to cap restart storms on persistent failures Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@coderabbitai full review please |
|
🧠 Learnings used✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bin/maestro-discord-ctl.sh`:
- Around line 116-119: cmd_update currently pipes a mutable installer from main
into a child bash which won't see non-exported INSTALL_DIR/BIN_DIR/REPO and also
pulls from main instead of the pinned release; change cmd_update to invoke the
installer with the necessary env exported or passed inline (preserve
INSTALL_DIR, BIN_DIR, REPO by exporting them or prefixing the curl|bash
invocation with them) and fetch the installer from the pinned release/tag rather
than main (use the repository release URL or a VERSION/RELEASE_TAG variable) so
the update uses the same versioned release model; reference cmd_update,
INSTALL_DIR, BIN_DIR, REPO and the install.sh URL when making the edit.
In `@install.sh`:
- Around line 71-76: The installer currently untars the downloaded artifact in
download_release without verifying integrity; update download_release to also
fetch the corresponding .sha256 asset (e.g., the same URL with .sha256
appended), compute the SHA-256 of the downloaded tarball (using sha256sum or
shasum -a 256) and compare it to the published checksum, and if they differ, die
with a clear error instead of proceeding to extraction; ensure you reference the
local variables tag and dest in download_release so the checksum URL and
verification operate on the exact file you downloaded.
- Around line 78-113: The rollback trap currently set in install_release is
cleared before install_deps runs, leaving no automatic rollback if npm ci fails;
keep the ERR trap active until after production dependencies are installed by
either moving the npm ci step into install_release (run npm ci in INSTALL_DIR
before calling trap - ERR) or by deferring the trap - ERR call until after
install_deps completes (ensure install_deps runs while backup and trap variables
from install_release are still in scope so mv "$backup" "$INSTALL_DIR" on ERR
can run); reference install_release, install_deps, INSTALL_DIR, backup and the
trap ERR usage when making the change.
- Around line 220-229: The macOS installer (install_service_macos) writes the
launchd plist without loading the app .env, so processes started from the
generated plist lack DISCORD_BOT_TOKEN / DISCORD_CLIENT_ID / DISCORD_GUILD_ID
and fail in src/config.ts; modify install_service_macos to ensure the plist
loads $CONFIG_DIR/.env by either (a) injecting EnvironmentVariables entries into
the plist from the key/value pairs in $CONFIG_DIR/.env when generating
sh.maestro.discord.plist, or (b) adjust the template substitution to set
ProgramArguments to run a shell wrapper that sources $CONFIG_DIR/.env before
exec'ing the node binary; update the install_service_macos logic that writes
"$INSTALL_DIR/templates/sh.maestro.discord.plist" ->
"$HOME/Library/LaunchAgents/sh.maestro.discord.plist" so the launched process
has the same env keys used by src/config.ts (DISCORD_BOT_TOKEN,
DISCORD_CLIENT_ID, DISCORD_GUILD_ID, etc.).
- Around line 142-145: The non-interactive branch copies .env.example to
$env_file and symlinks to $INSTALL_DIR/.env without restricting permissions,
exposing secrets under common umasks; after the cp and ln -sf steps (references:
env_file, INSTALL_DIR, .env.example, .env) add a chmod 0600 "$env_file" (and
optionally chmod 0600 "$INSTALL_DIR/.env" or ensure the symlink target is set)
so the created .env is locked down the same way as the interactive path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6f6392a5-4558-425b-b243-da48ee443537
📒 Files selected for processing (4)
.github/workflows/release.ymlbin/maestro-discord-ctl.shinstall.shtemplates/maestro-discord.service
✅ Files skipped from review due to trivial changes (1)
- templates/maestro-discord.service
- install.sh: verify downloaded tarball against published .sha256 using sha256sum or shasum; fail on mismatch, warn if neither tool is available - install.sh: extend ERR rollback through install_deps so a failing npm ci also restores the previous install (script-scope RELEASE_BACKUP, set -E for trap inheritance into functions) - install.sh: chmod 600 the .env on the non-interactive template path to match the interactive write - launchd plist: load $CONFIG_DIR/.env via `bash -c set -a; . ; set +a` so the macOS agent gets DISCORD_BOT_TOKEN and friends; restore the @CONFIG_DIR@ sed substitution - maestro-discord-ctl: pin `update` to the latest release tag's install.sh and forward MAESTRO_DISCORD_HOME / BIN_DIR / REPO and XDG_CONFIG_HOME so custom-location installs update in place Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@coderabbitai full review please |
|
✅ Actions performedFull review triggered. |
Migrate a regular-file .env from the previous install dir into $CONFIG_DIR/.env during upgrade so users coming from manual git-clone installs do not lose their working config. Refactor write_config to honor DISCORD_BOT_TOKEN / DISCORD_CLIENT_ID / DISCORD_GUILD_ID supplied via environment in non-interactive shells, and gate deploy_commands on config completeness (via a config_complete helper that rejects empty and your_* placeholder values) instead of /dev/tty availability — headless installs with env vars now register slash commands automatically. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@coderabbitai re-review please |
|
Tip For best results, initiate chat on the files or code changes.
[full_review] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
curl -fsSL .../install.sh | bash— downloads latest GitHub release tarball, runsnpm ci --omit=dev, prompts for Discord credentials, deploys slash commands, registers a systemd user / launchd agent service. Idempotent: re-running upgrades in place and preserves~/.config/maestro-discord/.env.maestro-discord-ctlwrapper symlinked to~/.local/binexposesstart | stop | restart | status | logs | deploy | update | uninstall | version.release.ymlworkflow: validatespackage.jsonversion against the tag, runs lint + tests, builds, packagesmaestro-discord-vX.Y.Z.tar.gz(containingdist/,bin/,templates/, manifests, README,.env.example) with SHA256 checksum, publishes a GitHub Release with auto-generated notes.package.jsonto1.0.0. README rewritten with separate Production install / Development install sections.Cutting the first release
After this merges, push tag
v1.0.0to trigger the release workflow:The
install.shone-liner becomes live the moment that release publishes.Files
install.sh— installer (root, curl-able)bin/maestro-discord-ctl.sh— service wrappertemplates/maestro-discord.service— systemd unit (Linux)templates/sh.maestro.discord.plist— launchd plist (macOS).github/workflows/release.yml— tag-triggered build + publishpackage.json— version bumpREADME.md— Production / Development install sectionsTest plan
bash -n install.sh && bash -n bin/maestro-discord-ctl.shsyntax clean (verified)shellcheck install.sh bin/maestro-discord-ctl.sh— no warnings (verified via dockerized shellcheck:stable)npm run build— clean tsc (verified)npm test— 110/110 pass (verified)v1.0.0, confirm release workflow uploads tarball + checksum, confirm GH Release notes auto-generatecurl -fsSL .../install.sh | bashend-to-end on a clean Linux VM and a clean macOS hostmaestro-discord-ctl updatere-installs and preserves.envjournalctl --user -u maestro-discord -fshows logsNotes
node:sqlite+bun build --compile) was scoped but not implemented in this PR — better-sqlite3 prebuilt binaries viaprebuild-installkeep the install zero-compile on common platforms today.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores