init - #1
Open
chasegiunta wants to merge 2 commits into
Open
Conversation
WalkthroughAdds a new Bun + TypeScript CLI/TUI project ("tug") for Craft CMS workflows: configuration, type definitions, runtime/process runner, task implementations (folder, database, composer, backups, shell), CLI commands, TUI, tests, and supporting tooling/config files. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Runtime as LoadedRuntime
participant Config as Config/Env
participant TaskSystem as Task System
participant Runner as Process Runner
participant Remote as SSH/Remote
User->>CLI: invoke tug (subcommand or TUI)
CLI->>Runtime: createLoadedRuntime(options)
Runtime->>Config: loadRuntimeConfig() / readLocalEnv()
Config-->>Runtime: TugConfig + credentials
CLI->>TaskSystem: plan(runtime, action)
TaskSystem->>Config: resolveLocal/RemoteCredentials()
Config-->>TaskSystem: DatabaseCredentials
TaskSystem-->>CLI: TaskPlan (commands, artifacts)
CLI->>TaskSystem: run(runtime, action)
TaskSystem->>Runner: runner.run(commandSpec)
Runner->>Remote: SSH / rsync / local binaries
Remote-->>Runner: command results
Runner-->>TaskSystem: stdout/stderr + exit codes
TaskSystem-->>CLI: TaskResult
CLI->>User: print summary / artifacts
sequenceDiagram
participant User as TTY User
participant TUI as TUI App
participant Doctor as Doctor Inspector
participant Runner as Process Runner
participant Logger as MemoryLogger
participant TaskSystem as Task Execution
User->>TUI: startTui(options)
TUI->>Runtime: createLoadedRuntime(interactive)
TUI->>Doctor: inspectProject(async)
Doctor->>Runner: run diagnostic commands (which, ssh checks)
Runner-->>Doctor: tool availability results
Doctor-->>TUI: DoctorReport
User->>TUI: select task + confirm
TUI->>Logger: clear() and subscribe()
TUI->>TaskSystem: run(runtime, action)
TaskSystem->>Runner: execute plan commands
Runner->>Logger: stream stdout/stderr via hooks
Logger-->>TUI: push log updates
TUI->>User: render task-runner / status / lastSummary
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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. 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: 16
🧹 Nitpick comments (11)
src/core/shell.ts (1)
5-6: UseshellQuoteinjoinCommandfor unambiguous rendering.
Current output can be misleading for args containing spaces or shell-sensitive characters.🛠️ Proposed refactor
export function joinCommand(parts: string[]): string { - return parts.join(" "); + return parts.map(shellQuote).join(" "); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/shell.ts` around lines 5 - 6, The joinCommand function currently concatenates parts with spaces which can misrepresent args containing spaces or shell-sensitive characters; update joinCommand to map each element through shellQuote (use the existing shellQuote function) and then join the quoted parts with a single space so every argument is safely and unambiguously rendered (also ensure the function still accepts string[] and handles empty strings by quoting them via shellQuote).package.json (1)
26-26: Use an explicit semver range or pinned version instead oflatestfor@types/bunto keep installs reproducible.Floating
"latest"versions can resolve differently across CI runs, causing non-determinism and potential lockfile conflicts. Use explicit versions like"^1.3.11"or runbun add -d@types/bun`` to generate a consistent range.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 26, The package.json entry for the devDependency "@types/bun" uses the floating tag "latest", which makes installs non-reproducible; replace it with an explicit semver range or pinned version (e.g., "^1.3.11" or "1.3.11") by updating the "@types/bun" value in package.json or run the command `bun add -d `@types/bun`` to have Bun write a consistent range into package.json and the lockfile; ensure you commit the updated package.json and lockfile so CI and local installs are deterministic.tests/config.test.ts (1)
21-26: Add an assertion for.env.samplecopy behavior.This test sets up
.env.examplebut never verifies thatwriteInitialConfigcopied it to.env.sample, so regressions in that path can slip through.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/config.test.ts` around lines 21 - 26, The test currently writes ".env.example" and calls writeInitialConfig but doesn't assert that writeInitialConfig copied it to ".env.sample"; update the test (around where writeProjectFile, writeInitialConfig, and readConfig are used) to assert that a ".env.sample" file exists in the test cwd and that its contents equal the expected "DB_DATABASE=test\nDB_USER=root\n" (or match the contents of the written .env.example) so regressions in the copy behavior are caught.src/tasks/index.ts (1)
3-54: Consider consolidating task metadata with execution routing.
src/tui/app.tsdispatches bydescriptor.id, while this file separately carriesaction. Keeping these in separate places can drift silently. A shared registry (descriptor + execute/confirm binding together) would reduce that risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tasks/index.ts` around lines 3 - 54, The taskDescriptors array is only metadata while dispatch logic in src/tui/app.ts branches on descriptor.id, risking drift; modify the TaskDescriptor shape to include execution routing (e.g., add an executor and optional confirmHandler fields) and update taskDescriptors entries (folderPull, folderPush, databasePull, databasePush, composerPull, composerPush, backups, terminal) to reference the appropriate handler names; then change the dispatcher in src/tui/app.ts to call descriptor.executor(descriptor) (and descriptor.confirmHandler when needed) so descriptor.id and action stay colocated with their execution functions.src/tasks/backups.ts (1)
16-24: Backups “open” action is currently macOS-specific.Using
open(and Finder wording) limits this command to macOS. Consider platform-aware openers (open/xdg-open/start) for a smoother cross-platform CLI experience.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tasks/backups.ts` around lines 16 - 24, The "open" action is macOS-specific (uses "open" and "Finder"); make it platform-aware by detecting process.platform and selecting the correct opener for runtimePaths.backupRoot (e.g., "open" for darwin, "xdg-open" for linux, and use "cmd" with ['/c','start',''] or an equivalent for win32) and update the action description (replace "Finder" with a neutral phrase or per-platform wording). Modify the commands branch that builds commands for action === "open" to use this platform mapping (or call a small helper like getPlatformOpener()) so the commands array and args are correct for each OS, and ensure destructive remains false.src/core/doctor.ts (3)
152-162: SSH failure should befail, notwarn, when diagnostics cannot proceed.If SSH connectivity fails, subsequent remote tool checks are skipped entirely, yet the check status is
warn. This could mask a critical connectivity issue. Consider usingfailstatus when SSH is unreachable, since the user cannot deploy without it.Proposed change
} catch (error) { addCheck({ id: "ssh-reachability", label: "SSH reachability", - status: "warn", + status: "fail", message: error instanceof Error ? error.message : `Unable to verify SSH connectivity to ${config.remote.host}.`, }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/doctor.ts` around lines 152 - 162, The SSH connectivity catch block in the doctor flow is marking the "ssh-reachability" check as "warn" which hides a blocking error; update the addCheck invocation in the catch of the try that verifies SSH (the block adding id "ssh-reachability") to set status to "fail" instead of "warn" and keep the existing error message logic (error instanceof Error ? error.message : `Unable to verify SSH connectivity to ${config.remote.host}.`) so the failure is clearly surfaced when remote tool checks cannot proceed.
139-151: Minor duplication: remote tools list duplicates logic fromrequiredTools.The database-specific tools (
mysqldump,mysql,pg_dump,psql) are listed both inrequiredTools()(lines 20-25) and again here. Consider extracting a shared helper to avoid drift.Possible extraction
function databaseTools(engine: "mysql" | "postgres"): string[] { return engine === "mysql" ? ["mysql", "mysqldump"] : ["psql", "pg_dump"]; }Then use in both
requiredToolsand the remote tool check.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/doctor.ts` around lines 139 - 151, Extract the duplicated DB-specific tool list into a shared helper (e.g., databaseTools(engine: "mysql" | "postgres"): string[]) and use it both in requiredTools() and in the remote-tool check where remoteTools is currently computed; replace the inline ternary that builds remoteTools with a call to databaseTools(config.database.engine), and update requiredTools() to call the same helper instead of hardcoding ["mysqldump","mysql"] or ["pg_dump","psql"]; keep existing calls to remoteToolExists(runner, config, tool) and addCheck({ id:`remote-tool-${tool}`, ... }) unchanged.
29-59: Consider adding a note about potential hangs if remote command stalls.The SSH
ConnectTimeout=2handles connection timeouts, but if the connection succeeds and the remotecommand -vhangs (e.g., due to shell initialization issues), there's no read timeout. This is low-risk sincecommand -vis fast, but worth documenting or handling in a follow-up for robustness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/doctor.ts` around lines 29 - 59, The remoteToolExists function may hang if SSH connects but the remote command stalls; update remoteToolExists to guard against read stalls by adding a per-run timeout when calling runner.run (or wrapping runner.run in a timeout Promise) and handle timeout by returning false and optionally logging; reference the remoteToolExists function, the runner: ProcessRunner instance and the runner.run invocation to add a timeout option or a Promise.race-based cancel, and also add a short comment above remoteToolExists documenting the potential stall and the chosen timeout behavior.src/tasks/composer.ts (1)
75-75: Redundantmkdircall —createBackupSessionalready creates the directory.Per the context snippet,
createBackupSessionalready callsawait mkdir(backupDir, { recursive: true })before returning. This line is unnecessary.Remove redundant mkdir
const session = await createBackupSession(runtime.options.cwd, runtime.config); - await mkdir(session.backupDir, { recursive: true });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tasks/composer.ts` at line 75, The call to mkdir is redundant because createBackupSession already creates the directory; remove the unnecessary await mkdir(session.backupDir, { recursive: true }) from the compose/backup flow (the code that runs after createBackupSession returns) so you rely on createBackupSession's directory creation—locate references to session.backupDir and the createBackupSession function to remove this duplicate mkdir invocation.src/tasks/database.ts (2)
308-316: Complex string concatenation for remote clear command is fragile.This constructs the clear command by concatenating
.commandand manually quoting.args. This bypasses the structuredCommandSpecdesign and could break ifclearSpecformat changes. Consider a helper to serialize aCommandSpecto a shell string.Extract a helper function
function specToShellCommand(spec: CommandSpec): string { if (spec.shell) return spec.command; return [spec.command, ...(spec.args ?? []).map(shellQuote)].join(" "); }Then use:
sshCommand( runtime, specToShellCommand(clearSpec(remoteCreds.engine, remoteCreds, "clear remote database")), remoteCreds.engine === "mysql" ? mysqlEnv(remoteCreds.password) : postgresEnv(remoteCreds.password), ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tasks/database.ts` around lines 308 - 316, The current call builds the remote clear command by manually concatenating clearSpec(...).command and quoted args, which is fragile; add a helper function (e.g., specToShellCommand(spec: CommandSpec)) that returns spec.command directly when spec.shell is true otherwise joins spec.command with (spec.args ?? []).map(shellQuote), then replace the inline concat in the sshCommand invocation with specToShellCommand(clearSpec(remoteCreds.engine, remoteCreds, "clear remote database")); keep the same env selection (mysqlEnv/postgresEnv) and reuse existing symbols sshCommand, clearSpec, shellQuote, mysqlEnv, postgresEnv to minimize changes.
133-150: Missingdisplayproperty on SSH command spec.Unlike other command builders (e.g.,
rsyncCopyFromRemote),sshCommanddoesn't setdisplay. This may result in less informative progress output or error messages showing raw SSH command strings instead of human-readable labels.Add display parameter
-function sshCommand(runtime: LoadedRuntime, remoteScript: string, env?: Record<string, string>): CommandSpec { +function sshCommand(runtime: LoadedRuntime, remoteScript: string, env?: Record<string, string>, display?: string): CommandSpec { const prefix = env ? `${Object.entries(env) .map(([key, value]) => `${key}=${shellQuote(value ?? "")}`) .join(" ")} ` : ""; return { command: "ssh", args: [ "-p", String(runtime.config.remote.port), `${runtime.config.remote.user}@${runtime.config.remote.host}`, `${prefix}${remoteScript}`, ], stdout: "pipe", stderr: "pipe", + display, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tasks/database.ts` around lines 133 - 150, The sshCommand function returns a CommandSpec without a human-readable display label; update sshCommand (the function returning CommandSpec) to include a display property similar to other builders (e.g., rsyncCopyFromRemote) so progress/errors show a friendly label instead of the raw SSH string—add a display array or string that includes "ssh", runtime.config.remote.host (or user@host) and a short remoteScript identifier so callers/logging display meaningful information.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitignore:
- Around line 15-16: The gitignore entries "_.log" and
"report.[0-9]_.[0-9]_.[0-9]_.[0-9]_.json" are using underscores instead of the
wildcard "*"; replace "_.log" with "*.log" and change the report pattern to a
proper glob (for example "report.[0-9]*_[0-9]*_[0-9]*_[0-9]*.json" if the parts
are underscore-separated numbers, or "report.*.*.*.*.json" /
"report.[0-9]*.[0-9]*.[0-9]*.[0-9]*.json" depending on the actual filename
format) so the intended files are matched.
In `@src/cli/commands/init.ts`:
- Around line 12-14: The init command currently ignores the parsed --force flag
and may overwrite an existing tug.toml; update the logic around optionsFromArgs
and writeInitialConfig so the force behavior is respected: either pass
options.force into writeInitialConfig (modify its signature to accept a force
boolean) or, before calling writeInitialConfig, resolve the target path from
options.cwd/options.configPath and if options.force is false check for file
existence and abort with a clear message instead of overwriting; reference the
functions optionsFromArgs, writeInitialConfig and the options properties
options.cwd, options.configPath, options.force when implementing this guard or
signature change.
In `@src/cli/main.ts`:
- Around line 31-33: Replace the current rawArgs scanning logic that sets
hasSubcommand (using context.rawArgs.some(...)) with a check against
context.args.command: determine if context.args.command exists and is not "tug"
(or is one of the known subcommands: init, doctor, folder, db, composer, shell,
backups); if so treat it as a subcommand and return early. Update the code paths
that reference hasSubcommand to use this new check (look for symbols
context.rawArgs, context.args.command and the existing early-return) so option
values like "--config tug.toml" no longer trigger a false positive.
In `@src/core/config.ts`:
- Around line 84-91: The current try/catch around accessing and copying
exampleEnvSource (.env.example) to exampleEnvTarget (.env.sample) swallows all
errors and uses an unusual target name; modify the catch to only ignore a
missing-source error by checking the error.code === 'ENOENT' (rethrow or surface
any other errors) and rename exampleEnvTarget to a conventional '.env' (or
confirm intent if '.env.sample' was deliberate) so update the variable
exampleEnvTarget and the copyFile call accordingly; keep the access/copyFile
calls and only suppress the specific ENOENT case.
In `@src/core/env.ts`:
- Around line 26-42: The parseDatabaseUrl function currently calls new
URL(urlString) without handling malformed input; wrap the URL construction in a
try-catch inside parseDatabaseUrl, catch the TypeError (or any error), and
rethrow a new Error with a clear, user-friendly message (e.g., "Invalid
DATABASE_URL: <original message>") while preserving original error details; keep
the rest of parseDatabaseUrl logic (normalizeEngine,
host/port/database/user/password/url) unchanged and ensure the thrown error is
descriptive for callers expecting DatabaseCredentials.
- Around line 107-124: fetchRemoteEnv currently ignores the SSH exit status and
injects raw path text into the remote command; update fetchRemoteEnv to validate
the runner.run result (check result.exitCode or equivalent) and throw a
descriptive error including result.stderr when SSH fails, and defensively
quote/escape the remote env path (config.database.remote_env_path ||
config.remote.env_path) before embedding it in the SSH command string so shell
metacharacters / spaces can't break the remote cat invocation; reference
fetchRemoteEnv, runner.run, result, dotenv.parse and the remote_env_path symbol
when making the changes.
In `@src/core/process.ts`:
- Around line 38-40: The code constructs the shell command array using a
hardcoded "/bin/zsh" when spec.shell is true; replace that with a
configurable/fallback shell by using process.env.SHELL (or another config) with
a fallback like "/bin/sh" so the command array becomes [chosenShell, "-lc",
spec.command]; update the logic that builds command (the const command variable
where spec.shell is checked) to use this selectedShell value (e.g., const shell
= process.env.SHELL ?? "/bin/sh") to ensure portability for functions that rely
on shell pipes (spec.shell true).
In `@src/tasks/composer.ts`:
- Around line 72-92: plan() currently calls createBackupSession and mkdir before
ensureForceForDestructive, causing duplicate sessions and orphan dirs; remove
the createBackupSession and mkdir calls from plan(), change plan() to accept
either a backupDir argument or build commands without creating a session (e.g.,
call buildRemoteBackupCommands(runtime, backupDir) only when backupDir is
provided), and move a single createBackupSession + mkdir into run() before
ensureForceForDestructive so run() creates the session once and passes its
backupDir into plan()/buildRemoteBackupCommands; update run() to call
plan(runtime, session.backupDir) or otherwise build commands after session
creation to ensure only one session/dir is created.
In `@src/tasks/database.ts`:
- Around line 214-230: The toolsFor function returns incorrect tool lists for
pull actions: update the logic so that for action === "pull" you return only the
importer on the local side and only the exporter on the remote side (for engine
"mysql" local pull should return ["mysql"], remote pull ["mysqldump"]; for
Postgres local pull ["psql"], remote pull ["pg_dump"]). Keep the existing
behavior for non-pull actions (push/export) unchanged; modify the branches that
check engine, side, and action in toolsFor to reflect these mappings.
- Around line 95-113: The args in function postgresClearSpec inject credential
values directly (host, port, user, database) which can break with special
characters; update postgresClearSpec to wrap each interpolated credential value
using the same shellQuote helper used elsewhere (e.g.,
`shellQuote(credentials.host)`, `shellQuote(credentials.port)`,
`shellQuote(credentials.user)`, `shellQuote(credentials.database)`) so the
generated args are safely quoted while preserving the existing flags, env via
postgresEnv, and display label.
- Around line 255-269: The backup session is created too early in
buildDatabaseCommands (createBackupSession) which can leave orphan directories
if assertDatabasePrereqs fails; reorder the steps so you first resolve
credentials (resolveLocalDatabaseCredentials, resolveRemoteDatabaseCredentials)
and call assertDatabasePrereqs(runtime, localCreds.engine, remoteCreds.engine,
action) before calling createBackupSession, then assign session and continue;
ensure any subsequent use of session remains valid after this reordering and
adjust variable declarations if needed.
In `@src/tasks/folder.ts`:
- Around line 39-46: The buildFolderPullCommands function emits real mkdir -p
commands even during a dry run; update buildFolderPullCommands to check
runtime.options.dryRun (or equivalent flag on runtime.options) and when true do
not produce real mkdirCommands for runtime.config.files.pull — either omit those
entries entirely or replace them with non-mutating no-op commands (e.g., an
explicit dry-run echo or a skipped marker) so folder pull --dry-run does not
modify the local filesystem; adjust references to mkdirCommands in
buildFolderPullCommands accordingly.
- Around line 28-31: The rsync/ssh argument strings interpolate
runtime.config.remote.app_path and folder directly, which can break or allow
shell injection on the remote; import the existing shellQuote utility (from
"../core/shell") and wrap the posix-joined remote path and the target folder
values used in the rsync/rsync-path and destination arguments (the expressions
using path.posix.join(runtime.config.remote.app_path, folder) and folder) with
shellQuote so they are single-quoted and properly escaped before interpolating
into the rsync/ssh command strings.
In `@src/tasks/shell.ts`:
- Around line 12-13: The template string that builds the remote command (`cd
${runtime.config.remote.app_path} && exec $SHELL -l`) injects raw
runtime.config.remote.app_path into a shell expression; change this to properly
quote and escape the path before embedding (e.g., use a shell-escape helper or
wrap the path in single quotes and escape any internal single quotes using the
standard '\'' pattern) so paths with spaces or metacharacters are safe, and
replace the inline interpolation in the command array with the escaped/quoted
value.
In `@src/tui/app.ts`:
- Around line 277-283: The terminal-task branch tears down the UI with
renderer.destroy() then calls await task.execute(runtime) outside the
surrounding try/catch, so SSH/session failures become unhandled; wrap the
execution and result handling for the "terminal" task in the same error handling
as the rest of the function (or add a local try/catch around await
task.execute(runtime)) to catch exceptions, log a clear error (including
error.message or stack), and set process.exitCode to a non-zero value on
failure; ensure you still call renderer.destroy() before running the task and
that successful completion sets process.exitCode = 0 and returns as before.
In `@tests/process.test.ts`:
- Around line 5-19: The test "merges custom env with PATH for spawned commands"
currently never verifies the child process received TUG_TEST_ENV; update the
test in tests/process.test.ts that uses BunProcessRunner.run so the spawned
command prints its environment (e.g., run a shell/command that echoes
$TUG_TEST_ENV or runs env and outputs that variable) and then add an assertion
that result.stdout contains the expected TUG_TEST_ENV value (e.g., "1"); keep
the intent and test name aligned by asserting the environment variable is
present in the child process output.
---
Nitpick comments:
In `@package.json`:
- Line 26: The package.json entry for the devDependency "@types/bun" uses the
floating tag "latest", which makes installs non-reproducible; replace it with an
explicit semver range or pinned version (e.g., "^1.3.11" or "1.3.11") by
updating the "@types/bun" value in package.json or run the command `bun add -d
`@types/bun`` to have Bun write a consistent range into package.json and the
lockfile; ensure you commit the updated package.json and lockfile so CI and
local installs are deterministic.
In `@src/core/doctor.ts`:
- Around line 152-162: The SSH connectivity catch block in the doctor flow is
marking the "ssh-reachability" check as "warn" which hides a blocking error;
update the addCheck invocation in the catch of the try that verifies SSH (the
block adding id "ssh-reachability") to set status to "fail" instead of "warn"
and keep the existing error message logic (error instanceof Error ?
error.message : `Unable to verify SSH connectivity to ${config.remote.host}.`)
so the failure is clearly surfaced when remote tool checks cannot proceed.
- Around line 139-151: Extract the duplicated DB-specific tool list into a
shared helper (e.g., databaseTools(engine: "mysql" | "postgres"): string[]) and
use it both in requiredTools() and in the remote-tool check where remoteTools is
currently computed; replace the inline ternary that builds remoteTools with a
call to databaseTools(config.database.engine), and update requiredTools() to
call the same helper instead of hardcoding ["mysqldump","mysql"] or
["pg_dump","psql"]; keep existing calls to remoteToolExists(runner, config,
tool) and addCheck({ id:`remote-tool-${tool}`, ... }) unchanged.
- Around line 29-59: The remoteToolExists function may hang if SSH connects but
the remote command stalls; update remoteToolExists to guard against read stalls
by adding a per-run timeout when calling runner.run (or wrapping runner.run in a
timeout Promise) and handle timeout by returning false and optionally logging;
reference the remoteToolExists function, the runner: ProcessRunner instance and
the runner.run invocation to add a timeout option or a Promise.race-based
cancel, and also add a short comment above remoteToolExists documenting the
potential stall and the chosen timeout behavior.
In `@src/core/shell.ts`:
- Around line 5-6: The joinCommand function currently concatenates parts with
spaces which can misrepresent args containing spaces or shell-sensitive
characters; update joinCommand to map each element through shellQuote (use the
existing shellQuote function) and then join the quoted parts with a single space
so every argument is safely and unambiguously rendered (also ensure the function
still accepts string[] and handles empty strings by quoting them via
shellQuote).
In `@src/tasks/backups.ts`:
- Around line 16-24: The "open" action is macOS-specific (uses "open" and
"Finder"); make it platform-aware by detecting process.platform and selecting
the correct opener for runtimePaths.backupRoot (e.g., "open" for darwin,
"xdg-open" for linux, and use "cmd" with ['/c','start',''] or an equivalent for
win32) and update the action description (replace "Finder" with a neutral phrase
or per-platform wording). Modify the commands branch that builds commands for
action === "open" to use this platform mapping (or call a small helper like
getPlatformOpener()) so the commands array and args are correct for each OS, and
ensure destructive remains false.
In `@src/tasks/composer.ts`:
- Line 75: The call to mkdir is redundant because createBackupSession already
creates the directory; remove the unnecessary await mkdir(session.backupDir, {
recursive: true }) from the compose/backup flow (the code that runs after
createBackupSession returns) so you rely on createBackupSession's directory
creation—locate references to session.backupDir and the createBackupSession
function to remove this duplicate mkdir invocation.
In `@src/tasks/database.ts`:
- Around line 308-316: The current call builds the remote clear command by
manually concatenating clearSpec(...).command and quoted args, which is fragile;
add a helper function (e.g., specToShellCommand(spec: CommandSpec)) that returns
spec.command directly when spec.shell is true otherwise joins spec.command with
(spec.args ?? []).map(shellQuote), then replace the inline concat in the
sshCommand invocation with specToShellCommand(clearSpec(remoteCreds.engine,
remoteCreds, "clear remote database")); keep the same env selection
(mysqlEnv/postgresEnv) and reuse existing symbols sshCommand, clearSpec,
shellQuote, mysqlEnv, postgresEnv to minimize changes.
- Around line 133-150: The sshCommand function returns a CommandSpec without a
human-readable display label; update sshCommand (the function returning
CommandSpec) to include a display property similar to other builders (e.g.,
rsyncCopyFromRemote) so progress/errors show a friendly label instead of the raw
SSH string—add a display array or string that includes "ssh",
runtime.config.remote.host (or user@host) and a short remoteScript identifier so
callers/logging display meaningful information.
In `@src/tasks/index.ts`:
- Around line 3-54: The taskDescriptors array is only metadata while dispatch
logic in src/tui/app.ts branches on descriptor.id, risking drift; modify the
TaskDescriptor shape to include execution routing (e.g., add an executor and
optional confirmHandler fields) and update taskDescriptors entries (folderPull,
folderPush, databasePull, databasePush, composerPull, composerPush, backups,
terminal) to reference the appropriate handler names; then change the dispatcher
in src/tui/app.ts to call descriptor.executor(descriptor) (and
descriptor.confirmHandler when needed) so descriptor.id and action stay
colocated with their execution functions.
In `@tests/config.test.ts`:
- Around line 21-26: The test currently writes ".env.example" and calls
writeInitialConfig but doesn't assert that writeInitialConfig copied it to
".env.sample"; update the test (around where writeProjectFile,
writeInitialConfig, and readConfig are used) to assert that a ".env.sample" file
exists in the test cwd and that its contents equal the expected
"DB_DATABASE=test\nDB_USER=root\n" (or match the contents of the written
.env.example) so regressions in the copy behavior are caught.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 803f8847-a9cb-49d1-8239-54a7f49e28b7
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (43)
.env.example.gitignoreREADME.mdoxlint.jsonpackage.jsonsrc/cli/commands/backups.tssrc/cli/commands/composer.tssrc/cli/commands/db.tssrc/cli/commands/doctor.tssrc/cli/commands/folder.tssrc/cli/commands/init.tssrc/cli/commands/shell.tssrc/cli/common.tssrc/cli/main.tssrc/core/config.tssrc/core/doctor.tssrc/core/env.tssrc/core/logging.tssrc/core/paths.tssrc/core/process.tssrc/core/runtime.tssrc/core/shell.tssrc/tasks/backups.tssrc/tasks/composer.tssrc/tasks/database.tssrc/tasks/folder.tssrc/tasks/index.tssrc/tasks/shared.tssrc/tasks/shell.tssrc/tui/app.tssrc/tui/components/confirmation-dialog.tssrc/tui/screens/dashboard.tssrc/tui/screens/task-runner.tssrc/types/index.tstests/config.test.tstests/database.test.tstests/doctor.test.tstests/env.test.tstests/folder.test.tstests/helpers.tstests/process.test.tstsconfig.jsontug.example.toml
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/tasks/composer.ts (1)
106-122:⚠️ Potential issue | 🟡 MinorMove
createBackupSession()behind the remaining preflight checks.This still creates a backup directory before
plan()verifies composer is enabled and before the push path checks forcomposer.json. If either check fails, the command leaves an orphan session directory behind.Suggested change
export async function run(runtime: LoadedRuntime, action: ComposerAction): Promise<TaskResult> { if (!runtime.options.force) { throw new Error("Composer sync is destructive. Re-run with --force or use the interactive TUI."); } - const session = await createBackupSession(runtime.options.cwd, runtime.config); - const taskPlan = await plan(runtime, action, session.backupDir); - ensureForceForDestructive(runtime, taskPlan); - if (action === "pull") { - await backupLocalComposerFiles(runtime, session.backupDir); - } else { + await plan(runtime, action); + if (action === "push") { const localComposer = path.resolve(runtime.options.cwd, "composer.json"); if (!(await fileExists(localComposer))) { throw new Error("composer.json is required for composer push."); } } + const session = await createBackupSession(runtime.options.cwd, runtime.config); + const taskPlan = await plan(runtime, action, session.backupDir); + ensureForceForDestructive(runtime, taskPlan); + if (action === "pull") { + await backupLocalComposerFiles(runtime, session.backupDir); + } await executePlan(runtime, taskPlan); return ok(`${taskPlan.title} finished successfully.`, [session.backupDir]); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tasks/composer.ts` around lines 106 - 122, The code calls createBackupSession(...) too early in run(): move the createBackupSession call to after the preflight checks (after plan(...) and after verifying push-path composer.json existence) so the backup directory is only created when all validations pass; specifically, keep the existing calls to plan(runtime, action, session.backupDir) and ensureForceForDestructive(runtime, taskPlan) but first invoke plan(runtime, action) or adjust plan usage so preflight checks (composer enabled and fileExists(localComposer) for push) run before creating the session; then call createBackupSession(...) and pass session.backupDir into backupLocalComposerFiles(...) and executePlan(...).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/core/config.ts`:
- Around line 65-71: The configExists function incorrectly treats all access()
errors as “missing” config; update configExists(cwd: string, explicitPath?:
string) to call access(resolveConfigPath(...)) inside try/catch but only return
false when the caught error.code === 'ENOENT' and re-throw any other errors so
permission or transient I/O errors surface; reference the existing
resolveConfigPath and access calls and ensure non-ENOENT errors are not
swallowed.
In `@src/core/doctor.ts`:
- Around line 38-63: The timeout created inside the Promise.race for the SSH
probe is never cleared, leaving a live timer after runner.run completes; modify
the race so the timeout handle is stored (e.g., let timer = setTimeout(...))
and, when the ssh promise (runner.run) resolves or rejects, call
clearTimeout(timer) before returning; update the anonymous timeout Promise and
the logic that handles the runner.run result (the Promise.race usage around
runner.run, the setTimeout resolver, and the resolved object with exitCode 124)
to ensure the timer is cleared in all code paths that finish the probe.
In `@src/tasks/database.ts`:
- Around line 194-220: remoteCommandExists can hang after SSH connects because
ConnectTimeout only limits connection setup; add a hard overall timeout to the
probe (same bounded-probe pattern used by the doctor code) so
assertDatabasePrereqs()/database.doctor() can't block indefinitely. Modify the
runtime.runner.run call inside remoteCommandExists to include the runner's
timeout option (or wrap the call in a Promise.race that rejects after a fixed
timeout, e.g. 3–5s) and ensure the runner process is aborted/cancelled on
timeout; keep the probe behavior and return semantics (resolve true when
exitCode === 0, false otherwise) unchanged.
- Around line 160-183: The rsync helpers rsyncCopyFromRemote and
rsyncCopyToRemote embed remote operands unquoted, which breaks when
runtime.config.remote.* paths contain spaces or metacharacters; fix by quoting
the remote path portion in the args so rsync receives user@host:"path" (e.g.
construct
`${runtime.config.remote.user}@${runtime.config.remote.host}:"${from}"` for the
remote source and
`${runtime.config.remote.user}@${runtime.config.remote.host}:"${to}"` for the
remote destination), ensuring both functions wrap the remote path in quotes (or
use a proper shell-escape utility) so remote.app_path is safe.
---
Duplicate comments:
In `@src/tasks/composer.ts`:
- Around line 106-122: The code calls createBackupSession(...) too early in
run(): move the createBackupSession call to after the preflight checks (after
plan(...) and after verifying push-path composer.json existence) so the backup
directory is only created when all validations pass; specifically, keep the
existing calls to plan(runtime, action, session.backupDir) and
ensureForceForDestructive(runtime, taskPlan) but first invoke plan(runtime,
action) or adjust plan usage so preflight checks (composer enabled and
fileExists(localComposer) for push) run before creating the session; then call
createBackupSession(...) and pass session.backupDir into
backupLocalComposerFiles(...) and executePlan(...).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 334574a5-09f9-4206-8493-45f900a9567c
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
.gitignorepackage.jsonsrc/cli/commands/init.tssrc/cli/main.tssrc/core/config.tssrc/core/doctor.tssrc/core/env.tssrc/core/process.tssrc/core/shell.tssrc/tasks/backups.tssrc/tasks/composer.tssrc/tasks/database.tssrc/tasks/folder.tssrc/tasks/shell.tssrc/tui/app.tstests/config.test.tstests/database.test.tstests/folder.test.tstests/process.test.ts
✅ Files skipped from review due to trivial changes (5)
- tests/process.test.ts
- src/core/shell.ts
- .gitignore
- tests/database.test.ts
- tests/folder.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- package.json
- src/cli/commands/init.ts
- src/tasks/backups.ts
- src/cli/main.ts
- src/core/env.ts
- src/tasks/folder.ts
- src/core/process.ts
Comment on lines
+65
to
+71
| export async function configExists(cwd: string, explicitPath?: string): Promise<boolean> { | ||
| try { | ||
| await access(resolveConfigPath(cwd, explicitPath)); | ||
| return true; | ||
| } catch { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Only treat ENOENT as “config missing.”
configExists() currently returns false for any access() failure, so permission errors and transient I/O errors get misreported as “Missing tug.toml.” Re-throw non-ENOENT failures here so callers surface the real problem instead of a false missing-file result.
Suggested change
export async function configExists(cwd: string, explicitPath?: string): Promise<boolean> {
try {
await access(resolveConfigPath(cwd, explicitPath));
return true;
- } catch {
- return false;
+ } catch (error) {
+ if ((error as NodeJS.ErrnoException | undefined)?.code === "ENOENT") {
+ return false;
+ }
+ throw error;
}
}📝 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
| export async function configExists(cwd: string, explicitPath?: string): Promise<boolean> { | |
| try { | |
| await access(resolveConfigPath(cwd, explicitPath)); | |
| return true; | |
| } catch { | |
| return false; | |
| } | |
| export async function configExists(cwd: string, explicitPath?: string): Promise<boolean> { | |
| try { | |
| await access(resolveConfigPath(cwd, explicitPath)); | |
| return true; | |
| } catch (error) { | |
| if ((error as NodeJS.ErrnoException | undefined)?.code === "ENOENT") { | |
| return false; | |
| } | |
| throw error; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/config.ts` around lines 65 - 71, The configExists function
incorrectly treats all access() errors as “missing” config; update
configExists(cwd: string, explicitPath?: string) to call
access(resolveConfigPath(...)) inside try/catch but only return false when the
caught error.code === 'ENOENT' and re-throw any other errors so permission or
transient I/O errors surface; reference the existing resolveConfigPath and
access calls and ensure non-ENOENT errors are not swallowed.
Comment on lines
+38
to
+63
| const result = await Promise.race([ | ||
| runner.run({ | ||
| command: "ssh", | ||
| args: [ | ||
| "-o", | ||
| "BatchMode=yes", | ||
| "-o", | ||
| "ConnectTimeout=2", | ||
| "-o", | ||
| "NumberOfPasswordPrompts=0", | ||
| "-o", | ||
| "PreferredAuthentications=publickey", | ||
| "-p", | ||
| String(config.remote.port), | ||
| `${config.remote.user}@${config.remote.host}`, | ||
| `command -v ${tool} >/dev/null 2>&1`, | ||
| ], | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| allowFailure: true, | ||
| }), | ||
| new Promise<{ exitCode: number; stdout: string; stderr: string }>((resolve) => { | ||
| setTimeout(() => { | ||
| resolve({ exitCode: 124, stdout: "", stderr: "Timed out checking remote tool." }); | ||
| }, 3000); | ||
| }), |
There was a problem hiding this comment.
Clear the timeout when the SSH probe finishes.
The timer created in this Promise.race() is never cancelled, so even a fast ssh result leaves a live 3-second handle behind. In CLI mode that can delay process exit and make tug doctor look hung after the checks have already completed.
Suggested change
async function remoteToolExists(
runner: ProcessRunner,
config: TugConfig,
tool: string,
): Promise<boolean> {
+ let timeoutId: ReturnType<typeof setTimeout> | undefined;
try {
+ const timeout = new Promise<{ exitCode: number; stdout: string; stderr: string }>((resolve) => {
+ timeoutId = setTimeout(() => {
+ resolve({ exitCode: 124, stdout: "", stderr: "Timed out checking remote tool." });
+ }, 3000);
+ });
const result = await Promise.race([
runner.run({
command: "ssh",
args: [
"-o",
@@
stderr: "pipe",
allowFailure: true,
}),
- new Promise<{ exitCode: number; stdout: string; stderr: string }>((resolve) => {
- setTimeout(() => {
- resolve({ exitCode: 124, stdout: "", stderr: "Timed out checking remote tool." });
- }, 3000);
- }),
+ timeout,
]);
return result.exitCode === 0;
} catch {
return false;
+ } finally {
+ if (timeoutId) {
+ clearTimeout(timeoutId);
+ }
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/doctor.ts` around lines 38 - 63, The timeout created inside the
Promise.race for the SSH probe is never cleared, leaving a live timer after
runner.run completes; modify the race so the timeout handle is stored (e.g., let
timer = setTimeout(...)) and, when the ssh promise (runner.run) resolves or
rejects, call clearTimeout(timer) before returning; update the anonymous timeout
Promise and the logic that handles the runner.run result (the Promise.race usage
around runner.run, the setTimeout resolver, and the resolved object with
exitCode 124) to ensure the timer is cleared in all code paths that finish the
probe.
Comment on lines
+160
to
+183
| function rsyncCopyFromRemote(runtime: LoadedRuntime, from: string, to: string, label: string): CommandSpec { | ||
| return { | ||
| command: "rsync", | ||
| args: [ | ||
| "--archive", | ||
| `--rsh=ssh -p ${runtime.config.remote.port}`, | ||
| `${runtime.config.remote.user}@${runtime.config.remote.host}:${from}`, | ||
| to, | ||
| ], | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| display: label, | ||
| }; | ||
| } | ||
|
|
||
| function rsyncCopyToRemote(runtime: LoadedRuntime, from: string, to: string, label: string): CommandSpec { | ||
| return { | ||
| command: "rsync", | ||
| args: [ | ||
| "--archive", | ||
| `--rsh=ssh -p ${runtime.config.remote.port}`, | ||
| from, | ||
| `${runtime.config.remote.user}@${runtime.config.remote.host}:${to}`, | ||
| ], |
There was a problem hiding this comment.
Quote the remote rsync path operands.
These helpers embed remote paths directly into the user@host:path rsync argument. Unlike src/tasks/composer.ts, the path is not shell-escaped here, so any remote.app_path with spaces or shell metacharacters will break pull/push database transfers.
Suggested change
function rsyncCopyFromRemote(runtime: LoadedRuntime, from: string, to: string, label: string): CommandSpec {
return {
command: "rsync",
args: [
"--archive",
`--rsh=ssh -p ${runtime.config.remote.port}`,
- `${runtime.config.remote.user}@${runtime.config.remote.host}:${from}`,
+ `${runtime.config.remote.user}@${runtime.config.remote.host}:${shellQuote(from)}`,
to,
],
@@
function rsyncCopyToRemote(runtime: LoadedRuntime, from: string, to: string, label: string): CommandSpec {
return {
command: "rsync",
args: [
"--archive",
`--rsh=ssh -p ${runtime.config.remote.port}`,
from,
- `${runtime.config.remote.user}@${runtime.config.remote.host}:${to}`,
+ `${runtime.config.remote.user}@${runtime.config.remote.host}:${shellQuote(to)}`,
],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tasks/database.ts` around lines 160 - 183, The rsync helpers
rsyncCopyFromRemote and rsyncCopyToRemote embed remote operands unquoted, which
breaks when runtime.config.remote.* paths contain spaces or metacharacters; fix
by quoting the remote path portion in the args so rsync receives
user@host:"path" (e.g. construct
`${runtime.config.remote.user}@${runtime.config.remote.host}:"${from}"` for the
remote source and
`${runtime.config.remote.user}@${runtime.config.remote.host}:"${to}"` for the
remote destination), ensuring both functions wrap the remote path in quotes (or
use a proper shell-escape utility) so remote.app_path is safe.
Comment on lines
+194
to
+220
| async function remoteCommandExists( | ||
| runtime: LoadedRuntime, | ||
| tool: string, | ||
| ): Promise<boolean> { | ||
| const result = await runtime.runner.run({ | ||
| command: "ssh", | ||
| args: [ | ||
| "-o", | ||
| "BatchMode=yes", | ||
| "-o", | ||
| "ConnectTimeout=2", | ||
| "-o", | ||
| "NumberOfPasswordPrompts=0", | ||
| "-o", | ||
| "PreferredAuthentications=publickey", | ||
| "-p", | ||
| String(runtime.config.remote.port), | ||
| `${runtime.config.remote.user}@${runtime.config.remote.host}`, | ||
| `command -v ${tool} >/dev/null 2>&1`, | ||
| ], | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| allowFailure: true, | ||
| display: `check remote tool ${tool}`, | ||
| }); | ||
| return result.exitCode === 0; | ||
| } |
There was a problem hiding this comment.
Add a hard timeout to the remote prerequisite probe.
ConnectTimeout=2 only bounds SSH connection setup. If the remote shell accepts the login and then stalls running command -v, both assertDatabasePrereqs() and database.doctor() can block indefinitely before any sync starts. This helper should use the same bounded probe pattern as src/core/doctor.ts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tasks/database.ts` around lines 194 - 220, remoteCommandExists can hang
after SSH connects because ConnectTimeout only limits connection setup; add a
hard overall timeout to the probe (same bounded-probe pattern used by the doctor
code) so assertDatabasePrereqs()/database.doctor() can't block indefinitely.
Modify the runtime.runner.run call inside remoteCommandExists to include the
runner's timeout option (or wrap the call in a Promise.race that rejects after a
fixed timeout, e.g. 3–5s) and ensure the runner process is aborted/cancelled on
timeout; keep the probe behavior and return semantics (resolve true when
exitCode === 0, false otherwise) unchanged.
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.
This pull request introduces the initial implementation of the
tugCLI tool, which provides a TypeScript/Bun-based command-line interface for Craft CMS deployment and sync workflows. The changes establish the project structure, core configuration and validation logic, command definitions, and supporting files for development and linting.The most important changes are:
Core features and CLI structure:
src/cli/main.ts) for thetugCLI, defining top-level commands and subcommands for initialization, diagnostics, file/folder sync, database operations, composer sync, backups, and remote shell access. The CLI supports both interactive TUI and subcommand-driven workflows.src/cli/commands/*.ts), includinginit,doctor,folder,db,composer,backups, andshell, each delegating to corresponding task logic and supporting shared arguments and runtime context. [1] [2] [3] [4] [5] [6] [7]src/cli/common.ts) for argument definitions, runtime loading, and standardized task result printing.Configuration and diagnostics:
src/core/config.ts), including schema validation (with Zod), TOML parsing, example config rendering, and initial config file creation.src/core/doctor.ts) that checks for required tools, configuration validity, and SSH/remote prerequisites, with formatted reporting for CLI and JSON output.Project setup and tooling:
package.json) with dependencies for Bun, OpenTUI, Citty, linting, and type checking, as well as scripts for development and testing..env.examplefile with Craft-style database variables for local development.oxlint.json) and a comprehensiveREADME.mdwith usage, requirements, and development instructions. [1] [2]Summary by CodeRabbit
New Features
tugCLI + interactive TUI for Craft CMS deployments with tasks: folder sync (pull/push), database sync (pull/push), composer sync (pull/push), backups (list/open), and remote shellDocumentation
Tests
Chores