feat(deploy): add Bun-based CLI deployment tool#296
Closed
larryro wants to merge 24 commits into
Closed
Conversation
Contributor
📝 WalkthroughWalkthroughThe PR replaces the bash-based blue-green deployment system with a new TypeScript/Bun CLI tool. It removes the shell deployment script (scripts/deploy.sh), blue/green Docker Compose overlays (compose.blue.yml, compose.green.yml), and introduces a new tools/deploy/ package containing a self-contained deployment CLI with commands for deploy, rollback, status, cleanup, reset, and logs. The Caddyfile is simplified from multi-backend routing to a single upstream. A GitHub Actions workflow is added to build and release the CLI binary. Documentation and TypeScript configurations are added to support the new tooling. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 39
🤖 Fix all issues with AI agents
In @.github/workflows/deploy-cli.yml:
- Around line 61-68: The "Upload to release" step currently checks only
github.event.inputs.release_tag and can run on non-dispatch events; update its
condition to require the event is workflow_dispatch and that the input is
non-empty. Locate the step named "Upload to release" and replace the if
condition that references github.event.inputs.release_tag with a combined check
for github.event_name == 'workflow_dispatch' and github.event.inputs.release_tag
!= '' so the gh release upload runs only for manual workflow_dispatch releases
with a provided tag.
In `@tools/deploy/.gitignore`:
- Around line 1-3: The .gitignore currently lists node_modules/, dist/, and
bun.lock but misses Bun's actual binary lockfile; update the ignore entries to
include bun.lockb in addition to any intentional bun.lock text file so the
proper Bun lockfile won't be committed—keep bun.lock only if you intentionally
rely on the text lockfile; ensure the entries 'bun.lockb' and optionally
'bun.lock' are present alongside the existing 'node_modules/' and 'dist/' lines.
In `@tools/deploy/package.json`:
- Around line 8-10: The package.json scripts currently define build:linux,
build:mac (darwin-arm64), and build:windows but omit an Intel Mac target; add a
new npm script (e.g., build:mac-intel or build:darwin-x64) that invokes bun
build with --compile --target=bun-darwin-x64 and an appropriate outfile (e.g.,
dist/tale-deploy-mac-intel) so the project can produce binaries for Intel-based
macOS; update any CI or release steps that call build:mac if you want both mac
targets produced.
- Around line 17-19: Update the devDependencies entry for `@types/bun` in
package.json to pin it to a stable version instead of "latest": replace the
current "@types/bun": "latest" declaration under devDependencies with the
specific version (e.g., "1.3.6" or a locked range like "^1.3.6") so builds are
reproducible and avoid unexpected breaking changes.
In `@tools/deploy/src/commands/cleanup.ts`:
- Around line 36-40: The code currently calls stopContainer(containerName) and
removeContainer(containerName) but ignores their Promise<boolean> results;
update the block that checks "running" to await and capture the boolean returns
from stopContainer and removeContainer (referencing stopContainer,
removeContainer, containerName, and cleaned), log an error if stopContainer
returns false and skip removal in that case (or attempt removal but log the
failed stop), only increment cleaned when removal succeeds (i.e.,
removeContainer returns true), and ensure any failures are surfaced to the user
via a process/logger call so these outcomes are reported.
In `@tools/deploy/src/commands/deploy.ts`:
- Around line 272-281: Health checks are failing when PROJECT_NAME is overridden
because container names are created with the hardcoded "tale" prefix in the
service creators while waitForHealthy and deploy logic use env.PROJECT_NAME (see
env.PROJECT_NAME, rotatableToUpdate, waitForHealthy); update each service
creator (create-*-service.ts) to derive container_name using config.projectName
(or the same config object used elsewhere) instead of the hardcoded "tale"
string so that container names and health-check lookups match; ensure the
container naming pattern matches `${config.projectName}-${service}-${color}`
used by deploy code.
In `@tools/deploy/src/commands/logs.ts`:
- Around line 107-113: The current Bun.spawn call starts docker logs but never
checks the process result; await proc.exited is used but the exit code is not
inspected, so failures are silently ignored. After awaiting proc.exited (or
using the resolved result), check the exit code on the spawned process
(proc.exitCode or the resolved exit object) and if non-zero, log an error and
propagate the failure by throwing an Error or calling process.exit with the exit
code; update the code around Bun.spawn/const proc and proc.exited to perform
this check so docker failures are not ignored.
- Around line 21-34: Remove the duplicate local implementations of
isValidService and isRotatableService and instead import the already-exported
helpers (isValidService, isRotatableService) from the shared types module;
update this file to use the imported symbols everywhere they are referenced
(replace the local function definitions with the imports and delete the local
functions).
In `@tools/deploy/src/commands/reset.ts`:
- Around line 44-50: The code currently checks isContainerRunning(containerName)
and calls stopContainer(containerName) before calling
removeContainer(containerName), but removeContainer already uses `docker rm -f`
so the stop is redundant; remove the isContainerRunning check and the
stopContainer call and just call await removeContainer(containerName)
(referencing the functions isContainerRunning, stopContainer, removeContainer
and the variable containerName). Apply the same simplification to the stateful
services loop (remove pre-check/stop and directly call await
removeContainer(serviceName) for each entry in the statefulServices loop).
- Around line 78-92: The loop that deletes files inefficiently does a dynamic
import("node:fs/promises") on every iteration and swallows all errors in an
empty catch; instead import unlink once at module scope (use unlink from
"node:fs/promises") and replace the per-iteration import with await unlink(file)
when bunFile.exists(), and change the catch to log a warning/error via logger
(include file name and the caught error) so permission or FS errors are not
silently ignored; refer to symbols stateFiles, dryRun, logger, Bun.file, and
unlink to find the code to change.
- Around line 1-5: The import list includes an unused type DeploymentColor;
remove DeploymentColor from the import statement in reset.ts and instead use a
direct inline type annotation at the cast site (the inline cast around line 38)
or add an explicit local type there if you need it — update the inline cast that
currently references DeploymentColor to use the chosen inline/local type so
there are no unused imports and the code compiles cleanly.
In `@tools/deploy/src/commands/rollback.ts`:
- Around line 60-68: The loop pulling images for ROTATABLE_SERVICES is
sequential; change it to run pulls concurrently by mapping ROTATABLE_SERVICES to
promises that call pullImage for
`${env.GHCR_REGISTRY}/tale-${service}:${rollbackVersion}` and then await
Promise.all on that array, checking results and throwing an Error if any pull
fails; keep the logger.step("Pulling previous version images...") and ensure you
reference ROTATABLE_SERVICES, pullImage, rollbackVersion and env.GHCR_REGISTRY
when implementing the parallelized pulls.
- Around line 110-116: The loop that stops and removes old color containers
(using ROTATABLE_SERVICES, currentColor, stopContainer, removeContainer) lacks
error handling so failures are ignored; wrap each container operation in a
try/catch and, on failure, emit a warning via logger.warn or logger.step
indicating the containerName and the error so operators are aware of orphaned
containers; optionally simplify by skipping stopContainer and using the
force/remove behavior in removeContainer (or ensure removeContainer invokes
docker rm -f) while still catching and warning on any removal errors.
In `@tools/deploy/src/commands/status.ts`:
- Around line 23-37: getServiceStatus currently maps health === "none" to
"unhealthy", which misrepresents containers that simply lack a health check;
update getServiceStatus to explicitly handle the "none" case (e.g., if (health
=== "none") return "unknown" or "running" depending on your ServiceStatus
semantics) before falling back to "unhealthy", and update the ServiceStatus
union/type and any callers if needed to accept the new "unknown"/"running"
value; reference function getServiceStatus and the health param to locate where
to add the explicit branch.
- Around line 66-79: The loop is performing three awaits sequentially
(isContainerRunning, getContainerHealth, getContainerVersion) which slows status
checks; change the per-service logic in the STATEFUL_SERVICES loop to start all
three async calls concurrently and await them with Promise.all, e.g. create the
containerName then do const [running, health, version] = await
Promise.all([isContainerRunning(containerName),
getContainerHealth(containerName), getContainerVersion(containerName)]), then
compute status via getServiceStatus(running, health) and the versionStr and
print as before using STATUS_COLORS and service.padEnd.
In `@tools/deploy/src/compose/generators/generate-color-compose.ts`:
- Around line 8-12: The generateColorCompose function is taking projectName
separately which duplicates ServiceConfig.projectName and can cause mismatches;
remove the extra parameter from generateColorCompose(signature) and update its
internals to read the project name from config.projectName instead, then update
all call sites to stop passing projectName and rely on the config object; ensure
any references inside the function (e.g., constructing external
volumes/networks) use config.projectName and run tests/compile to catch
remaining callers still passing the removed argument.
In `@tools/deploy/src/compose/services/create-platform-service.ts`:
- Around line 25-28: The network alias "platform" under networks.internal
(aliases: [`platform-${color}`, "platform"]) breaks blue/green isolation because
it makes both colored containers resolvable; remove the generic "platform" alias
so only the color-specific alias (`platform-${color}`) remains, or add a
conditional to only include "platform" when deploying single-color mode; update
the aliases array in create-platform-service.ts (networks.internal.aliases)
accordingly to use only the color-specific alias or gate the generic alias
behind a single-color flag.
In `@tools/deploy/src/compose/services/create-search-service.ts`:
- Around line 13-27: The healthcheck in create-search-service.ts uses wget while
other services (create-crawler-service.ts, create-rag-service.ts) use curl,
causing inconsistency; update the healthcheck block in the createSearchService
(healthcheck property) to use the same curl command/flags used by the other
services, or if wget is required, add a short comment in createSearchService
explaining why wget is necessary and ensure the container base image includes
wget; either change the tool to curl for uniformity or document the intentional
divergence.
In `@tools/deploy/src/docker/ensure-network.ts`:
- Around line 21-27: The ensureNetwork function currently returns the boolean
from createNetwork silently; mirror ensureVolumes by adding error logging: call
createNetwork(fullName) inside a try/catch (or check its false result) and when
it fails log a descriptive message with the network fullName and the
error/details using the same logger used in ensureVolumes (e.g.,
processLogger.error), then return false; on success return true—this ensures
ensureNetwork and ensureVolumes behave consistently.
In `@tools/deploy/src/docker/exec.ts`:
- Around line 21-29: The current code waits for stdout then stderr sequentially
which can deadlock; instead start both stream reads immediately and await them
concurrently: create promises for new Response(proc.stdout).text() and new
Response(proc.stderr).text() (referenced as proc, stdout/stderr reads from
Bun.spawn) and use Promise.all to await both results, then await proc.exited for
exitCode; update the block around Bun.spawn/proc so both streams are drained in
parallel to avoid pipe buffer deadlocks.
In `@tools/deploy/src/docker/get-container-version.ts`:
- Around line 6-17: The code in get-container-version currently returns the raw
label string which can be the sentinel "<no value>"; after calling docker(...)
and obtaining result (the variable result from the docker inspect call), treat
any trimmed stdout that is empty or equals the literal string "<no value>" as
missing and return null instead of returning that literal; update the check
around result.success/result.stdout.trim() (used before the final return of
result.stdout.trim()) to explicitly test for trimmedStdout === "<no value>" and
return null in that case so only a real version string is returned.
In `@tools/deploy/src/docker/image-exists.ts`:
- Around line 3-5: The imageExists function currently treats any
docker("image","inspect", image) non-success as "image missing"; instead, when
result.success is false inspect the error output (e.g., result.stderr or
result.error/message) for the specific "no such image" text (case-insensitive,
e.g., "no such image" or "No such image:") and only return false in that case;
for any other failure rethrow or throw a new Error including the docker result
details so daemon/permission issues surface. Update imageExists to inspect the
docker call's error payload and either return false for the specific
missing-image message or propagate an error for all other failures.
In `@tools/deploy/src/docker/is-container-running.ts`:
- Around line 3-10: The isContainerRunning function currently masks all docker
inspect failures as "not running"; change it to return false only when Docker
reports a missing container (inspect stderr contains "No such object" or "no
such container" case-insensitively) and to throw an error for any other failure
(e.g., daemon/permission errors) so callers can react appropriately; locate the
function isContainerRunning and the docker(...) call, examine result.success,
result.stdout and result.stderr, return true when success and stdout is "true",
return false when stderr indicates a missing container, and otherwise throw a
descriptive Error with the stderr message; update the five callers (status.ts,
wait-for-healthy.ts, reset.ts, logs.ts, cleanup.ts) to handle the thrown
exceptions.
In `@tools/deploy/src/docker/list-containers.ts`:
- Around line 16-18: The parser currently splits result.stdout by "\n" and maps
every line directly, which can produce entries for blank lines; update the
parsing in list-containers (the code that handles
result.stdout.split("\n").map(...)) to first filter out empty/whitespace-only
lines (e.g., result.stdout.split("\n").filter(line => line.trim() !== ''))
before mapping so only non-empty lines are parsed into { name, status, image }
objects.
In `@tools/deploy/src/index.ts`:
- Around line 133-141: The logs command lacks runtime validation of its
<service> argument; update the program.command("logs") setup to validate the
service name using the existing isValidService helper (same approach used by the
deploy command) so invalid names fail early. Either add a validation step on the
argument definition or perform an immediate check in the logs handler (the
function that processes the logs command) and throw/print a clear error if
isValidService(service) is false, mirroring the deploy flow.
- Around line 142-158: Validate options.color and options.tail before calling
logs: ensure options.color (in the .action callback) is either "blue" or "green"
and if not call logger.error(...) and process.exit(1); for tail, check that
options.tail is defined and consists only of digits (e.g., /^\d+$/) before
calling parseInt, otherwise set tail to undefined (or treat non-numeric input as
an error and exit) and pass that sanitized value into the logs(...) call; update
the call site that currently uses parseInt(options.tail, 10) to use the
validated/sanitized tail and use the validated color value when invoking logs.
In `@tools/deploy/src/state/acquire-lock.ts`:
- Around line 6-12: The isProcessRunning function incorrectly treats all errors
from process.kill(pid, 0) as the process not running; update it to catch the
thrown error into a variable and return true if error.code === 'EPERM' (meaning
the process exists but is not signalable), otherwise return false for other
errors; ensure you only change the catch block of isProcessRunning to inspect
err.code and keep the existing process.kill(pid, 0) call and Promise<boolean>
signature.
- Around line 19-41: The current existsSync -> Bun.write flow in acquire-lock.ts
(lockPath, getLockFilePath, getLockInfo, isProcessRunning, Bun.write, lockInfo)
is vulnerable to TOCTOU: two processes can both think the lock is free; the
proposed open(..., "wx") would EEXIST for stale locks because stale files are
not removed. Fix by making lock acquisition atomic: if
getLockInfo/death-detection shows a stale lock, remove it (unlink) before
attempting creation, then create atomically by writing the lock to a unique temp
file (e.g., lockPath + ".tmp.<pid>") and atomically renaming the temp into
lockPath with Bun.rename; ensure you handle EEXIST from rename as a lost race
(treat as failure) and clean up temp files on error. Ensure references:
getLockFilePath, getLockInfo, isProcessRunning, lockPath, lockInfo,
Bun.write/Bun.rename are used accordingly.
In `@tools/deploy/src/state/get-current-color.ts`:
- Around line 11-20: The code uses existsSync then directly calls
Bun.file(statePath).text(), which can throw and abort CLI commands; wrap the
file read and trim into a try/catch around the Bun.file(statePath).text() call
(capturing any error from Bun.file or .text()), and on catch log a warning via
logger.warn including the error and return null so callers get a best‑effort
result; keep the existing validation of color (DeploymentColor / checks for
"blue"/"green") and only proceed if the read succeeds.
In `@tools/deploy/src/state/get-lock-file-path.ts`:
- Around line 1-5: The getLockFilePath function uses string concatenation which
can produce double slashes or wrong separators on Windows; update
getLockFilePath to use node:path's join (importing path from 'node:path' or
require('path')) to join deployDir and LOCK_FILE so the path is normalized and
portable across platforms, ensuring it handles trailing slashes in deployDir
correctly; change references in the file to use path.join(deployDir, LOCK_FILE).
In `@tools/deploy/src/state/get-lock-info.ts`:
- Around line 13-15: Remove the TOCTOU-prone existsSync check in
get-lock-info.ts and instead attempt to read the lock file directly inside the
existing try/catch: eliminate the if (!existsSync(lockPath)) early return and
let Bun.file(lockPath).text() run; catch and return null on any read/parse
errors so the code relies solely on the try/catch around Bun.file().text() and
JSON.parse rather than a separate existsSync check.
- Around line 17-22: The current code blindly returns JSON.parse(content) as
LockInfo which can accept malformed-but-valid-JSON shapes; update the try block
in get-lock-info (where Bun.file(lockPath).text() is parsed) to perform runtime
validation of the parsed object against the LockInfo shape (check required
fields and their types) and only return the object if validation passes,
otherwise return null (or throw); you can implement a small validator function
(or use a schema library) to verify fields present on the parsed value before
casting to LockInfo so callers never receive an incorrectly shaped object.
In `@tools/deploy/src/state/get-previous-version-file-path.ts`:
- Around line 1-5: Replace string interpolation path construction in
getPreviousVersionFilePath with node:path join to ensure consistent, portable
paths; import or require the path module and use path.join(deployDir,
PREVIOUS_VERSION_FILE) instead of `${deployDir}/${PREVIOUS_VERSION_FILE}` so the
PREVIOUS_VERSION_FILE constant is combined safely across platforms.
In `@tools/deploy/src/state/get-previous-version.ts`:
- Around line 7-14: Replace the pre-check using existsSync with a try-catch
around the read of Bun.file(versionPath).text(): remove the existsSync branch
and instead call Bun.file(getPreviousVersionFilePath(deployDir)).text() inside a
try, trim and return the content or null, catch errors and if error.code ===
'ENOENT' return null, otherwise rethrow the error; refer to
getPreviousVersionFilePath, versionPath and Bun.file(...).text() when making the
change.
In `@tools/deploy/src/state/get-state-file-path.ts`:
- Around line 3-5: The getStateFilePath function currently concatenates
deployDir and STATE_FILE with a string template which can produce incorrect
paths on some platforms or with trailing slashes; update getStateFilePath to
import and use path.join (from Node's 'path' module) to combine deployDir and
STATE_FILE, keep the same function signature and return type, and ensure any
necessary import (e.g., import path from 'path' or import { join } from 'path')
is added at the top of the file.
In `@tools/deploy/src/state/release-lock.ts`:
- Around line 6-18: The releaseLock function currently returns early when
getLockInfo(deployDir) yields null (e.g., corrupt JSON) and it also calls unlink
without handling ENOENT; update releaseLock so that if lockInfo is null you
treat the lock file as corrupt/stale and attempt to remove it, and also wrap the
unlink call in a try/catch that ignores ENOENT and logs failures; additionally,
if lockInfo exists but lockInfo.pid !== process.pid, check whether that pid is
alive and if not remove the stale lock similarly (all removals should use the
dynamically imported unlink and logger.debug messages on successful removal and
logger.warn on unexpected unlink errors).
In `@tools/deploy/src/state/set-previous-version.ts`:
- Around line 4-11: Add a guard in setPreviousVersion to validate the incoming
version string before writing: if the version is null/undefined or an empty
string, throw a clear Error (or reject) so callers receive a meaningful failure
instead of writing an empty file; keep using getPreviousVersionFilePath,
Bun.write, and logger.debug for the normal path and ensure the validation
happens at the start of the setPreviousVersion function.
In `@tools/deploy/src/state/with-lock.ts`:
- Around line 14-18: The finally block currently awaits releaseLock(deployDir)
which can throw and overwrite any error from fn(); update the finally to call
releaseLock(deployDir) inside its own try/catch, catching and logging the
release error (using the existing logger) so it doesn't mask the original
exception returned by await fn(); reference releaseLock and the surrounding
block where fn() is awaited in with-lock.ts and ensure the catch only logs the
error and does not rethrow so the original fn() error propagates.
In `@tools/deploy/src/utils/load-env.ts`:
- Around line 17-36: The loadEnv function currently detects deployDir/.env
(envPath) but never loads it and also trusts process.env values when parsing
integers; update loadEnv to load the env file (e.g., require and call
dotenv.config({ path: envPath }) or equivalent) before reading values so values
in deployDir/.env populate process.env, and defensively parse
HEALTH_CHECK_TIMEOUT and DRAIN_TIMEOUT by falling back to defaults when parseInt
yields NaN (use Number.isNaN on the parsed result and replace with
DEFAULT_HEALTH_CHECK_TIMEOUT / DEFAULT_DRAIN_TIMEOUT); keep references to
GHCR_REGISTRY, HEALTH_CHECK_TIMEOUT, DRAIN_TIMEOUT, PROJECT_NAME, and DEPLOY_DIR
when returning the DeploymentEnv object.
Comment on lines
+8
to
+10
| "build:linux": "bun build --compile --target=bun-linux-x64 --outfile=dist/tale-deploy-linux src/index.ts", | ||
| "build:mac": "bun build --compile --target=bun-darwin-arm64 --outfile=dist/tale-deploy-mac src/index.ts", | ||
| "build:windows": "bun build --compile --target=bun-windows-x64 --outfile=dist/tale-deploy-windows.exe src/index.ts", |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding Intel Mac (darwin-x64) build target.
The current build targets cover Linux x64, Apple Silicon (darwin-arm64), and Windows x64, but omit Intel-based Macs which may still be in use.
♻️ Add darwin-x64 target
"build:linux": "bun build --compile --target=bun-linux-x64 --outfile=dist/tale-deploy-linux src/index.ts",
"build:mac": "bun build --compile --target=bun-darwin-arm64 --outfile=dist/tale-deploy-mac src/index.ts",
+ "build:mac-x64": "bun build --compile --target=bun-darwin-x64 --outfile=dist/tale-deploy-mac-x64 src/index.ts",
"build:windows": "bun build --compile --target=bun-windows-x64 --outfile=dist/tale-deploy-windows.exe src/index.ts",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "build:linux": "bun build --compile --target=bun-linux-x64 --outfile=dist/tale-deploy-linux src/index.ts", | |
| "build:mac": "bun build --compile --target=bun-darwin-arm64 --outfile=dist/tale-deploy-mac src/index.ts", | |
| "build:windows": "bun build --compile --target=bun-windows-x64 --outfile=dist/tale-deploy-windows.exe src/index.ts", | |
| "build:linux": "bun build --compile --target=bun-linux-x64 --outfile=dist/tale-deploy-linux src/index.ts", | |
| "build:mac": "bun build --compile --target=bun-darwin-arm64 --outfile=dist/tale-deploy-mac src/index.ts", | |
| "build:mac-x64": "bun build --compile --target=bun-darwin-x64 --outfile=dist/tale-deploy-mac-x64 src/index.ts", | |
| "build:windows": "bun build --compile --target=bun-windows-x64 --outfile=dist/tale-deploy-windows.exe src/index.ts", |
🤖 Prompt for AI Agents
In `@tools/deploy/package.json` around lines 8 - 10, The package.json scripts
currently define build:linux, build:mac (darwin-arm64), and build:windows but
omit an Intel Mac target; add a new npm script (e.g., build:mac-intel or
build:darwin-x64) that invokes bun build with --compile --target=bun-darwin-x64
and an appropriate outfile (e.g., dist/tale-deploy-mac-intel) so the project can
produce binaries for Intel-based macOS; update any CI or release steps that call
build:mac if you want both mac targets produced.
Collaborator
Author
There was a problem hiding this comment.
This CLI runs on Linux deploy servers only. Darwin builds exist for local dev convenience on team Macs (all Apple Silicon). Intel Mac support isn't needed here.
Contributor
There was a problem hiding this comment.
@larryro, thank you for the clarification! That makes perfect sense—if the CLI is deployed exclusively on Linux servers and your team is standardized on Apple Silicon for local development, the Intel Mac target would indeed be unnecessary overhead. I appreciate you explaining the deployment architecture.
✏️ Learnings added
Learnt from: larryro
Repo: tale-project/tale PR: 296
File: tools/deploy/package.json:8-10
Timestamp: 2026-01-28T03:20:21.660Z
Learning: In the tale-project/tale repository, the tale-deploy CLI tool (tools/deploy) is designed to run on Linux deploy servers in production. The Darwin/Mac build targets (darwin-arm64) exist only for local development convenience, as the team uses Apple Silicon Macs exclusively. Intel Mac (darwin-x64) builds are not needed.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: talecorp/poc2 PR: 0
File: .cursorrules:0-0
Timestamp: 2025-10-11T11:46:02.452Z
Learning: Use npm as the package manager and the documented npm scripts
Learnt from: CR
Repo: tale-project/poc2 PR: 0
File: .cursor/rules/workspace_rules.mdc:0-0
Timestamp: 2025-11-30T12:29:39.745Z
Learning: Use npm as the package manager with commands: `npm run dev` for development, `npm i <package>` for install, and `npm run build` for production build
Learnt from: larryro
Repo: tale-project/tale PR: 207
File: compose.yml:0-0
Timestamp: 2026-01-16T14:09:37.873Z
Learning: In the tale-project/tale repository, the compose.yml file at the repository root is for development only. Production deployments use a separate compose configuration where only necessary ports are exposed.
Learnt from: CR
Repo: talecorp/poc2 PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-10-03T11:34:20.628Z
Learning: Applies to package.json : Add types/node when using Node.js built-in modules
Learnt from: CR
Repo: tale-project/poc2 PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-30T03:53:00.316Z
Learning: Applies to package.json : Always add `types/node` to `package.json` when using any Node.js built-in modules in Convex functions
Comment on lines
+4
to
+11
| export async function setPreviousVersion( | ||
| deployDir: string, | ||
| version: string | ||
| ): Promise<void> { | ||
| const versionPath = getPreviousVersionFilePath(deployDir); | ||
| await Bun.write(versionPath, version); | ||
| logger.debug(`Saved previous version: ${version}`); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Implementation is consistent with other state utilities.
The function follows the same pattern as setCurrentColor, using Bun.write for persistence and logging the action. Error propagation to callers is appropriate for deployment operations.
Consider adding a guard against empty version strings if callers don't already validate:
♻️ Optional: Add empty check
export async function setPreviousVersion(
deployDir: string,
version: string
): Promise<void> {
+ if (!version) {
+ throw new Error("Version cannot be empty");
+ }
const versionPath = getPreviousVersionFilePath(deployDir);
await Bun.write(versionPath, version);
logger.debug(`Saved previous version: ${version}`);
}🤖 Prompt for AI Agents
In `@tools/deploy/src/state/set-previous-version.ts` around lines 4 - 11, Add a
guard in setPreviousVersion to validate the incoming version string before
writing: if the version is null/undefined or an empty string, throw a clear
Error (or reject) so callers receive a meaningful failure instead of writing an
empty file; keep using getPreviousVersionFilePath, Bun.write, and logger.debug
for the normal path and ensure the validation happens at the start of the
setPreviousVersion function.
Collaborator
Author
There was a problem hiding this comment.
Acknowledged. No changes needed.
Contributor
There was a problem hiding this comment.
Rate Limit Exceeded
@larryro have exceeded the limit for the number of chat messages per hour. Please wait 0 minutes and 4 seconds before sending another message.
Contributor
There was a problem hiding this comment.
@larryro, understood! No changes needed. Thank you for the feedback.
🧠 Learnings used
Learnt from: larryro
Repo: tale-project/tale PR: 296
File: tools/deploy/package.json:8-10
Timestamp: 2026-01-28T03:20:21.660Z
Learning: In the tale-project/tale repository, the tale-deploy CLI tool (tools/deploy) is designed to run on Linux deploy servers in production. The Darwin/Mac build targets (darwin-arm64) exist only for local development convenience, as the team uses Apple Silicon Macs exclusively. Intel Mac (darwin-x64) builds are not needed.
Learnt from: larryro
Repo: tale-project/tale PR: 296
File: tools/deploy/src/docker/is-container-running.ts:3-10
Timestamp: 2026-01-28T03:21:34.093Z
Learning: In the tale-deploy CLI (tools/deploy/), the project prefers treating all docker inspect failures as "container not running" rather than parsing stderr to distinguish daemon/permission errors from missing containers, because the deployment runs on a known server where daemon failures will cause clear failures in subsequent operations.
Create a new deployment CLI that generates Docker Compose configs inline with security-hardened settings. Only ports 80/443 are exposed in production. Features: - Blue-green deployment with zero-downtime - Rollback to previous version - Status monitoring - Cleanup of inactive containers - Reset functionality The CLI compiles to a single binary for easy server deployment. Refs #294
Build the tale-deploy binary for Linux x64 and upload to GitHub Releases. Triggers: - Manual dispatch with optional release tag - Push to main when tools/deploy/ changes Refs #294
Add clear warning that compose.yml exposes ports for development only. Production deployments should use the tale-deploy CLI. Refs #294
Remove files replaced by the new tale-deploy CLI: - compose.blue.yml - compose.green.yml - scripts/deploy.sh Refs #294
- Fix network aliases format (use object instead of array) - Fix external volume declaration (remove driver when external) - Write compose file to deploy dir for correct env_file resolution - Add ensureVolumes and ensureNetwork to create resources before deploy Refs #294
…lify proxy - Add --dry-run flag to deploy and reset commands for previewing changes - Add --services flag to deploy specific services without full blue-green switch - Add in-place update mode when deploying individual services - Add logs command for viewing service container logs - Add --version flag to rollback for targeting a specific version - Rename --update-stateful/--include-stateful to -a/--all - Simplify Caddyfile to route via single platform DNS alias - Add service type guards and ALL_SERVICES constant
Split monolithic docker/client.ts and docker/health.ts into focused single-responsibility modules (container, exec, network, volume, pull-image, wait-for-healthy, check-http-health, image-exists). Rename command exports from xxxCommand to plain names. Add mac and windows build targets.
…-purpose files Break down monolithic modules (state/lock.ts, state/deployment.ts, compose/services/*, utils/env.ts) into focused single-function files following the same pattern applied to docker modules. Update all command imports accordingly.
Build Linux x64, macOS ARM64, and Windows x64 binaries in CI instead of only Linux. Each platform binary is uploaded as a separate artifact and attached to releases.
Now that CI builds for all platforms, replace string template paths with path.join to handle platform-specific path separators correctly.
larryro
force-pushed
the
feat/294-bun-deploy-cli
branch
from
669fdbd to
6d0462b
Compare
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
tools/deploy/) that replaces the legacy bash deploy script with a typed, modular TypeScript implementationTest plan
bun run tools/deploy/src/index.ts --helpand verify all commands are listedbun run tools/deploy/src/index.ts deploy --dry-runand verify compose YAML is generated without executingmain🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.