fix(cli): route launch-metadata warnings to a log file#202
Open
willwashburn wants to merge 1 commit into
Open
Conversation
|
CodeAnt AI is reviewing your PR. |
Contributor
📝 WalkthroughWalkthroughThis PR introduces a new CLI logging infrastructure with environment-driven configuration and file output capabilities, then integrates it into the CLI's launch-metadata recording to route warnings through the logger instead of the terminal. A relay workspace state file is also added to track polling and reconciliation metadata. ChangesCLI Logging Infrastructure and Integration
Relay Workspace State
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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. Comment |
codeant-ai
Bot
added
the
size:L
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a lightweight file logger for the AgentWorkforce CLI to write structured logs to a file, and integrates it to route background launch metadata warnings away from the terminal. The review feedback identifies three key improvement opportunities: validating the log level in getLogLevel to prevent runtime issues, optimizing ensureLogDir to avoid synchronous fs.existsSync calls on every log write by caching existing directories, and spreading the extra payload first in the log entry to prevent standard fields from being overwritten.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+56
to
+58
| function getLogLevel(): LogLevel { | ||
| return (process.env.AGENTWORKFORCE_LOG_LEVEL ?? 'INFO').toUpperCase() as LogLevel; | ||
| } |
There was a problem hiding this comment.
The getLogLevel function casts the environment variable directly to LogLevel without validating if it is a valid log level. If an invalid log level is provided, LEVEL_PRIORITY[getLogLevel()] will return undefined at runtime. While the fallback ?? LEVEL_PRIORITY.INFO handles this, it is safer and more robust to validate the log level and return a guaranteed valid LogLevel.
Suggested change
| function getLogLevel(): LogLevel { | |
| return (process.env.AGENTWORKFORCE_LOG_LEVEL ?? 'INFO').toUpperCase() as LogLevel; | |
| } | |
| function getLogLevel(): LogLevel { | |
| const level = (process.env.AGENTWORKFORCE_LOG_LEVEL ?? 'INFO').toUpperCase(); | |
| return level in LEVEL_PRIORITY ? (level as LogLevel) : 'INFO'; | |
| } |
Comment on lines
+67
to
+73
| function ensureLogDir(logFile: string): void { | ||
| const logDir = path.dirname(logFile); | ||
| if (!createdLogDirs.has(logDir) && !fs.existsSync(logDir)) { | ||
| fs.mkdirSync(logDir, { recursive: true }); | ||
| createdLogDirs.add(logDir); | ||
| } | ||
| } |
There was a problem hiding this comment.
The ensureLogDir function only adds the directory to createdLogDirs if it does not already exist. If the directory already exists (which is the common case after the first run), fs.existsSync will be called on every single log write. Since fs.existsSync is a synchronous I/O operation, this introduces an unnecessary performance bottleneck. We should cache the directory in createdLogDirs regardless of whether we had to create it or if it already existed.
Suggested change
| function ensureLogDir(logFile: string): void { | |
| const logDir = path.dirname(logFile); | |
| if (!createdLogDirs.has(logDir) && !fs.existsSync(logDir)) { | |
| fs.mkdirSync(logDir, { recursive: true }); | |
| createdLogDirs.add(logDir); | |
| } | |
| } | |
| function ensureLogDir(logFile: string): void { | |
| const logDir = path.dirname(logFile); | |
| if (createdLogDirs.has(logDir)) return; | |
| if (!fs.existsSync(logDir)) { | |
| fs.mkdirSync(logDir, { recursive: true }); | |
| } | |
| createdLogDirs.add(logDir); | |
| } |
Comment on lines
+97
to
+103
| const entry: LogEntry = { | ||
| ts: new Date().toISOString(), | ||
| level, | ||
| component, | ||
| msg, | ||
| ...extra | ||
| }; |
There was a problem hiding this comment.
Standard log fields (ts, level, component, msg) can be accidentally or maliciously overwritten if they are also present in the extra payload, because ...extra is spread after the standard fields. Spreading extra first ensures that standard fields always take precedence and cannot be overwritten.
Suggested change
| const entry: LogEntry = { | |
| ts: new Date().toISOString(), | |
| level, | |
| component, | |
| msg, | |
| ...extra | |
| }; | |
| const entry: LogEntry = { | |
| ...extra, | |
| ts: new Date().toISOString(), | |
| level, | |
| component, | |
| msg | |
| }; |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79dddd7214
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -0,0 +1 @@ | |||
| {"workspaceId":"rw_7ccfea89","remoteRoot":"/memory/workspace","localRoot":"/home/daytona/workspace/memory/workspace","mode":"poll","intervalMs":30000,"lastReconcileAt":"2026-06-05T12:44:28.728205937Z","lastSuccessfulReconcileAt":"2026-06-05T12:44:28.728205937Z","staleAfter":"2026-06-05T12:45:28.728205937Z","status":"ready","states":{"stale":false,"offline":false,"hasConflicts":false,"hasPendingWriteback":false},"pendingWriteback":0,"pendingConflicts":0,"deniedPaths":0,"counters":{"snapshotDeleteBlocked":12},"circuit":{"open":false,"openedAt":"0001-01-01T00:00:00Z","windowMs":60000,"cooldownMs":30000,"threshold":5,"nextRetry":"0001-01-01T00:00:00Z"}} No newline at end of file | |||
There was a problem hiding this comment.
This generated state file is under memory/, which .gitignore already excludes, and it embeds machine-specific Relay metadata such as workspaceId and localRoot=/home/daytona/.... Keeping it tracked means every checkout now carries one developer's stale mount state; if Relay tooling is run against memory/workspace, it can reuse that state instead of creating local state for the current workspace. Please drop this ignored runtime file from the commit.
Useful? React with 👍 / 👎.
|
CodeAnt AI finished reviewing your PR. |
![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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@memory/workspace/.relay/state.json`:
- Line 1: The committed relay state contains machine-specific runtime values
(fields like localRoot, lastReconcileAt, lastSuccessfulReconcileAt, staleAfter,
status, counters.snapshotDeleteBlocked, circuit.open/ timestamps) and should be
replaced with deterministic neutral defaults: set localRoot and remoteRoot to
empty or templated values, zero out or use epoch/null for timestamp fields
(lastReconcileAt, lastSuccessfulReconcileAt, staleAfter, circuit.openedAt,
circuit.nextRetry), set status to a neutral value like "init" or "unknown", zero
all counters and pending* fields, and keep mode/intervalMs/circuit thresholds
sensible defaults; update the JSON initializer that produces this file (the
state template or generator emitting workspace state) to emit those neutral
defaults instead of captured runtime values (refer to the keys workspaceId,
localRoot, lastReconcileAt, status, counters.snapshotDeleteBlocked, circuit) so
new environments start deterministic and portable.
In `@packages/cli/src/logger.ts`:
- Around line 97-103: The current construction of the LogEntry lets callers
overwrite reserved fields because ...extra is spread after
ts/level/component/msg; update the merge so reserved fields in entry cannot be
clobbered by extra: either filter out reserved keys from the extra object (ts,
level, component, msg) before spreading, or spread extra first and then spread
the fixed fields (ts, level, component, msg) to overwrite any duplicates; update
the code around the LogEntry creation in packages/cli/src/logger.ts (the entry
variable / LogEntry construction) and ensure shouldLog() still uses the intended
level value.
- Around line 79-121: The log() function can still throw because formatMessage()
(when isLogJson() is true) or process.stderr.write() may throw; update log()
and/or formatMessage() so all formatting and write operations are wrapped in
try/catch paths that never propagate exceptions: call formatMessage() inside a
try block and if it throws, build a minimal safe fallback string (e.g., `${new
Date().toISOString()} [${level}] [${component}] ${msg}` or include a short
"[logging_error]" marker) and continue; also wrap the final writes
(fs.appendFileSync and process.stderr.write) in try/catch and swallow any errors
(no throw), ensuring the function always returns void; reference formatMessage,
isLogJson, log, getLogFile, ensureLogDir, and process.stderr.write when making
the changes.
🪄 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 Plus
Run ID: ded2544f-2155-4aa0-9455-18882bc3a68f
📒 Files selected for processing (3)
memory/workspace/.relay/state.jsonpackages/cli/src/cli.tspackages/cli/src/logger.ts
| @@ -0,0 +1 @@ | |||
| {"workspaceId":"rw_7ccfea89","remoteRoot":"/memory/workspace","localRoot":"/home/daytona/workspace/memory/workspace","mode":"poll","intervalMs":30000,"lastReconcileAt":"2026-06-05T12:44:28.728205937Z","lastSuccessfulReconcileAt":"2026-06-05T12:44:28.728205937Z","staleAfter":"2026-06-05T12:45:28.728205937Z","status":"ready","states":{"stale":false,"offline":false,"hasConflicts":false,"hasPendingWriteback":false},"pendingWriteback":0,"pendingConflicts":0,"deniedPaths":0,"counters":{"snapshotDeleteBlocked":12},"circuit":{"open":false,"openedAt":"0001-01-01T00:00:00Z","windowMs":60000,"cooldownMs":30000,"threshold":5,"nextRetry":"0001-01-01T00:00:00Z"}} No newline at end of file | |||
Contributor
There was a problem hiding this comment.
Initialize relay state with neutral defaults, not captured runtime values.
This committed “initial” state includes machine/session-specific data (localRoot, concrete timestamps, non-zero counters, ready status). That can seed incorrect behavior in fresh environments (staleness/reconcile timing, health, and counters) and reduces portability.
Proposed shape for deterministic initialization
-{"workspaceId":"rw_7ccfea89","remoteRoot":"/memory/workspace","localRoot":"/home/daytona/workspace/memory/workspace","mode":"poll","intervalMs":30000,"lastReconcileAt":"2026-06-05T12:44:28.728205937Z","lastSuccessfulReconcileAt":"2026-06-05T12:44:28.728205937Z","staleAfter":"2026-06-05T12:45:28.728205937Z","status":"ready","states":{"stale":false,"offline":false,"hasConflicts":false,"hasPendingWriteback":false},"pendingWriteback":0,"pendingConflicts":0,"deniedPaths":0,"counters":{"snapshotDeleteBlocked":12},"circuit":{"open":false,"openedAt":"0001-01-01T00:00:00Z","windowMs":60000,"cooldownMs":30000,"threshold":5,"nextRetry":"0001-01-01T00:00:00Z"}}
+{"workspaceId":"","remoteRoot":"","localRoot":"","mode":"poll","intervalMs":30000,"lastReconcileAt":"0001-01-01T00:00:00Z","lastSuccessfulReconcileAt":"0001-01-01T00:00:00Z","staleAfter":"0001-01-01T00:00:00Z","status":"initializing","states":{"stale":false,"offline":false,"hasConflicts":false,"hasPendingWriteback":false},"pendingWriteback":0,"pendingConflicts":0,"deniedPaths":0,"counters":{"snapshotDeleteBlocked":0},"circuit":{"open":false,"openedAt":"0001-01-01T00:00:00Z","windowMs":60000,"cooldownMs":30000,"threshold":5,"nextRetry":"0001-01-01T00:00:00Z"}}📝 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
| {"workspaceId":"rw_7ccfea89","remoteRoot":"/memory/workspace","localRoot":"/home/daytona/workspace/memory/workspace","mode":"poll","intervalMs":30000,"lastReconcileAt":"2026-06-05T12:44:28.728205937Z","lastSuccessfulReconcileAt":"2026-06-05T12:44:28.728205937Z","staleAfter":"2026-06-05T12:45:28.728205937Z","status":"ready","states":{"stale":false,"offline":false,"hasConflicts":false,"hasPendingWriteback":false},"pendingWriteback":0,"pendingConflicts":0,"deniedPaths":0,"counters":{"snapshotDeleteBlocked":12},"circuit":{"open":false,"openedAt":"0001-01-01T00:00:00Z","windowMs":60000,"cooldownMs":30000,"threshold":5,"nextRetry":"0001-01-01T00:00:00Z"}} | |
| {"workspaceId":"","remoteRoot":"","localRoot":"","mode":"poll","intervalMs":30000,"lastReconcileAt":"0001-01-01T00:00:00Z","lastSuccessfulReconcileAt":"0001-01-01T00:00:00Z","staleAfter":"0001-01-01T00:00:00Z","status":"initializing","states":{"stale":false,"offline":false,"hasConflicts":false,"hasPendingWriteback":false},"pendingWriteback":0,"pendingConflicts":0,"deniedPaths":0,"counters":{"snapshotDeleteBlocked":0},"circuit":{"open":false,"openedAt":"0001-01-01T00:00:00Z","windowMs":60000,"cooldownMs":30000,"threshold":5,"nextRetry":"0001-01-01T00:00:00Z"}} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@memory/workspace/.relay/state.json` at line 1, The committed relay state
contains machine-specific runtime values (fields like localRoot,
lastReconcileAt, lastSuccessfulReconcileAt, staleAfter, status,
counters.snapshotDeleteBlocked, circuit.open/ timestamps) and should be replaced
with deterministic neutral defaults: set localRoot and remoteRoot to empty or
templated values, zero out or use epoch/null for timestamp fields
(lastReconcileAt, lastSuccessfulReconcileAt, staleAfter, circuit.openedAt,
circuit.nextRetry), set status to a neutral value like "init" or "unknown", zero
all counters and pending* fields, and keep mode/intervalMs/circuit thresholds
sensible defaults; update the JSON initializer that produces this file (the
state template or generator emitting workspace state) to emit those neutral
defaults instead of captured runtime values (refer to the keys workspaceId,
localRoot, lastReconcileAt, status, counters.snapshotDeleteBlocked, circuit) so
new environments start deterministic and portable.
Comment on lines
+79
to
+121
| function formatMessage(entry: LogEntry): string { | ||
| if (isLogJson()) { | ||
| return JSON.stringify(entry); | ||
| } | ||
| const { ts, level, component, msg, ...extra } = entry; | ||
| const extraStr = | ||
| Object.keys(extra).length > 0 | ||
| ? ' ' + | ||
| Object.entries(extra) | ||
| .map(([k, v]) => `${k}=${v}`) | ||
| .join(' ') | ||
| : ''; | ||
| return `${ts} [${level}] [${component}] ${msg}${extraStr}`; | ||
| } | ||
|
|
||
| function log(level: LogLevel, component: string, msg: string, extra?: Record<string, unknown>): void { | ||
| if (!shouldLog(level)) return; | ||
|
|
||
| const entry: LogEntry = { | ||
| ts: new Date().toISOString(), | ||
| level, | ||
| component, | ||
| msg, | ||
| ...extra | ||
| }; | ||
|
|
||
| const formatted = formatMessage(entry); | ||
|
|
||
| const logFile = getLogFile(); | ||
| if (logFile) { | ||
| try { | ||
| ensureLogDir(logFile); | ||
| fs.appendFileSync(logFile, formatted + '\n'); | ||
| return; | ||
| } catch { | ||
| // Fall through to stderr if the log file is unwritable — never throw | ||
| // from a logging call. | ||
| } | ||
| } | ||
|
|
||
| // No log file configured (or it failed): fall back to stderr so the | ||
| // diagnostic isn't lost entirely. We never write to stdout. | ||
| process.stderr.write(formatted + '\n'); |
Contributor
There was a problem hiding this comment.
Keep the logger's no-throw guarantee on the fallback path.
formatMessage() can still throw in JSON mode for cyclic or BigInt extras, and process.stderr.write() can throw if stderr has already been destroyed. Either case escapes log() today, which breaks the module's explicit "never throws from a log call" contract.
Suggested fix
function log(level: LogLevel, component: string, msg: string, extra?: Record<string, unknown>): void {
if (!shouldLog(level)) return;
- const entry: LogEntry = {
- ts: new Date().toISOString(),
- level,
- component,
- msg,
- ...extra
- };
-
- const formatted = formatMessage(entry);
-
- const logFile = getLogFile();
- if (logFile) {
- try {
- ensureLogDir(logFile);
- fs.appendFileSync(logFile, formatted + '\n');
- return;
- } catch {
- // Fall through to stderr if the log file is unwritable — never throw
- // from a logging call.
- }
- }
-
- // No log file configured (or it failed): fall back to stderr so the
- // diagnostic isn't lost entirely. We never write to stdout.
- process.stderr.write(formatted + '\n');
+ try {
+ const entry: LogEntry = {
+ ...extra,
+ ts: new Date().toISOString(),
+ level,
+ component,
+ msg
+ };
+ const line = formatMessage(entry) + '\n';
+
+ const logFile = getLogFile();
+ if (logFile) {
+ try {
+ ensureLogDir(logFile);
+ fs.appendFileSync(logFile, line);
+ return;
+ } catch {
+ // Fall through to stderr if the log file is unwritable.
+ }
+ }
+
+ try {
+ // No log file configured (or it failed): fall back to stderr so the
+ // diagnostic isn't lost entirely. We never write to stdout.
+ process.stderr.write(line);
+ } catch {
+ // Swallow to preserve the logger's no-throw contract.
+ }
+ } catch {
+ // Swallow formatting/serialization failures too.
+ }
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/logger.ts` around lines 79 - 121, The log() function can
still throw because formatMessage() (when isLogJson() is true) or
process.stderr.write() may throw; update log() and/or formatMessage() so all
formatting and write operations are wrapped in try/catch paths that never
propagate exceptions: call formatMessage() inside a try block and if it throws,
build a minimal safe fallback string (e.g., `${new Date().toISOString()}
[${level}] [${component}] ${msg}` or include a short "[logging_error]" marker)
and continue; also wrap the final writes (fs.appendFileSync and
process.stderr.write) in try/catch and swallow any errors (no throw), ensuring
the function always returns void; reference formatMessage, isLogJson, log,
getLogFile, ensureLogDir, and process.stderr.write when making the changes.
Comment on lines
+97
to
+103
| const entry: LogEntry = { | ||
| ts: new Date().toISOString(), | ||
| level, | ||
| component, | ||
| msg, | ||
| ...extra | ||
| }; |
Contributor
There was a problem hiding this comment.
Preserve reserved log fields when merging extra.
Because ...extra is applied after ts, level, component, and msg, a caller can silently overwrite the record metadata while shouldLog() was evaluated against a different level. That makes the emitted line internally inconsistent.
Suggested fix
const entry: LogEntry = {
- ts: new Date().toISOString(),
- level,
- component,
- msg,
- ...extra
+ ...extra,
+ ts: new Date().toISOString(),
+ level,
+ component,
+ msg
};📝 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
| const entry: LogEntry = { | |
| ts: new Date().toISOString(), | |
| level, | |
| component, | |
| msg, | |
| ...extra | |
| }; | |
| const entry: LogEntry = { | |
| ...extra, | |
| ts: new Date().toISOString(), | |
| level, | |
| component, | |
| msg | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/logger.ts` around lines 97 - 103, The current construction
of the LogEntry lets callers overwrite reserved fields because ...extra is
spread after ts/level/component/msg; update the merge so reserved fields in
entry cannot be clobbered by extra: either filter out reserved keys from the
extra object (ts, level, component, msg) before spreading, or spread extra first
and then spread the fixed fields (ts, level, component, msg) to overwrite any
duplicates; update the code around the LogEntry creation in
packages/cli/src/logger.ts (the entry variable / LogEntry construction) and
ensure shouldLog() still uses the intended level value.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="memory/workspace/.relay/state.json">
<violation number="1" location="memory/workspace/.relay/state.json:1">
P1: Hardcoded machine-specific absolute path in state.json — this auto-generated relay workspace state file should not be committed to version control. `localRoot` points to `/home/daytona/workspace/memory/workspace`, which is unique to the current machine and will break for any other developer or CI environment. The file also contains point-in-time timestamps, counters, and circuit-breaker state that are transient runtime artifacts, not meaningful committed source.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| @@ -0,0 +1 @@ | |||
| {"workspaceId":"rw_7ccfea89","remoteRoot":"/memory/workspace","localRoot":"/home/daytona/workspace/memory/workspace","mode":"poll","intervalMs":5000,"lastReconcileAt":"2026-06-05T12:52:46.487111815Z","lastSuccessfulReconcileAt":"2026-06-05T12:52:46.487111815Z","staleAfter":"2026-06-05T12:52:56.487111815Z","status":"ready","states":{"stale":false,"offline":false,"hasConflicts":false,"hasPendingWriteback":false},"pendingWriteback":0,"pendingConflicts":0,"deniedPaths":0,"counters":{"snapshotDeleteBlocked":215},"circuit":{"open":false,"openedAt":"0001-01-01T00:00:00Z","windowMs":60000,"cooldownMs":30000,"threshold":5,"nextRetry":"0001-01-01T00:00:00Z"}} No newline at end of file | |||
Contributor
There was a problem hiding this comment.
P1: Hardcoded machine-specific absolute path in state.json — this auto-generated relay workspace state file should not be committed to version control. localRoot points to /home/daytona/workspace/memory/workspace, which is unique to the current machine and will break for any other developer or CI environment. The file also contains point-in-time timestamps, counters, and circuit-breaker state that are transient runtime artifacts, not meaningful committed source.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At memory/workspace/.relay/state.json, line 1:
<comment>Hardcoded machine-specific absolute path in state.json — this auto-generated relay workspace state file should not be committed to version control. `localRoot` points to `/home/daytona/workspace/memory/workspace`, which is unique to the current machine and will break for any other developer or CI environment. The file also contains point-in-time timestamps, counters, and circuit-breaker state that are transient runtime artifacts, not meaningful committed source.</comment>
<file context>
@@ -0,0 +1 @@
+{"workspaceId":"rw_7ccfea89","remoteRoot":"/memory/workspace","localRoot":"/home/daytona/workspace/memory/workspace","mode":"poll","intervalMs":5000,"lastReconcileAt":"2026-06-05T12:52:46.487111815Z","lastSuccessfulReconcileAt":"2026-06-05T12:52:46.487111815Z","staleAfter":"2026-06-05T12:52:56.487111815Z","status":"ready","states":{"stale":false,"offline":false,"hasConflicts":false,"hasPendingWriteback":false},"pendingWriteback":0,"pendingConflicts":0,"deniedPaths":0,"counters":{"snapshotDeleteBlocked":215},"circuit":{"open":false,"openedAt":"0001-01-01T00:00:00Z","windowMs":60000,"cooldownMs":30000,"threshold":5,"nextRetry":"0001-01-01T00:00:00Z"}}
\ No newline at end of file
</file context>
Contributor
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
Contributor
There was a problem hiding this comment.
19 issues found across 136 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/renderer/src/components/agents/CloudAgentDialog.tsx">
<violation number="1" location="src/renderer/src/components/agents/CloudAgentDialog.tsx:50">
P2: Modal dialog is missing keyboard Escape handling and focus trapping, so keyboard users can’t reliably dismiss it or stay within the modal.</violation>
</file>
<file name="src/renderer/src/stores/typing-store.ts">
<violation number="1" location="src/renderer/src/stores/typing-store.ts:113">
P3: Duplicate agent-key formatting logic is introduced instead of using a single shared key utility, creating drift risk between typing writes and reads.</violation>
</file>
<file name="src/main/filesystem.ts">
<violation number="1" location="src/main/filesystem.ts:35">
P2: File preview follows symlinks, allowing reads outside project roots despite path guard.</violation>
</file>
<file name="src/main/integration-symlinks.ts">
<violation number="1" location="src/main/integration-symlinks.ts:126">
P2: Symlink ownership check is too broad (`includes`), so foreign `.integrations` links can be removed accidentally.</violation>
</file>
<file name="src/main/integrations.types.ts">
<violation number="1" location="src/main/integrations.types.ts:1">
P2: This new file duplicates integration type contracts that are already defined elsewhere, creating multiple sources of truth and drift risk.</violation>
</file>
<file name="src/main/avatar-cache.ts">
<violation number="1" location="src/main/avatar-cache.ts:179">
P1: `entry.fileName` is not constrained to stay inside `CACHE_DIR`, enabling path traversal when serving `pear-avatar://` files.</violation>
<violation number="2" location="src/main/avatar-cache.ts:189">
P2: Registration state is set before `protocol.handle` succeeds, so a registration failure can permanently disable retries.</violation>
</file>
<file name="src/main/relayfile-mount-launcher.ts">
<violation number="1" location="src/main/relayfile-mount-launcher.ts:57">
P3: The error message suggests running a non-existent `npm run relayfile-mount:install` script, which gives users a dead-end recovery path.</violation>
</file>
<file name="src/main/ai-hist.ts">
<violation number="1" location="src/main/ai-hist.ts:260">
P2: `reload()`/`dispose()` do not handle in-flight SDK initialization, causing a race that can repopulate `instance` after reset.</violation>
</file>
<file name="src/main/proactive-agent.types.ts">
<violation number="1" location="src/main/proactive-agent.types.ts:1">
P2: New proactive-agent type declarations duplicate existing shared IPC types, creating contract drift risk and extra maintenance.</violation>
</file>
<file name="src/renderer/src/stores/pty-buffer-store.ts">
<violation number="1" location="src/renderer/src/stores/pty-buffer-store.ts:14">
P2: getPtyChunks returns the mutable internal buffer array, allowing external mutation that bypasses capping and notifications.</violation>
</file>
<file name="src/main/cli-install.ts">
<violation number="1" location="src/main/cli-install.ts:32">
P2: The generated shell shim does not safely escape `exePath`, so shell expansions/quoting can break or alter execution for valid filesystem paths.</violation>
<violation number="2" location="src/main/cli-install.ts:40">
P1: The admin-privileged `osascript` command interpolates an unescaped path, creating a command-injection/command-break risk when the temp path contains quotes.</violation>
</file>
<file name="src/renderer/src/lib/diff-selection.ts">
<violation number="1" location="src/renderer/src/lib/diff-selection.ts:71">
P1: Partial-patch hunk headers use full-diff new line numbers, so selecting a subset of changes can generate misnumbered `+newStart` coordinates.</violation>
</file>
<file name="memory/workspace/.relay/state.json">
<violation number="1" location="memory/workspace/.relay/state.json:1">
P2: Runtime workspace-relay state file committed to version control — contains mutable counters, timestamps, and machine-local paths that should be gitignored (like `.agent-relay` already is)</violation>
</file>
<file name="src/main/auth.ts">
<violation number="1" location="src/main/auth.ts:392">
P1: OAuth callback accepts tokens without validating the `state` parameter, enabling token injection/session swap.</violation>
<violation number="2" location="src/main/auth.ts:440">
P2: `getAuthStatus` incorrectly reports logged-in when stored tokens are unreadable or invalid.</violation>
<violation number="3" location="src/main/auth.ts:545">
P1: In-flight token refresh can restore stale credentials after logout/account change because refresh success always calls `saveTokens(next)`.</violation>
</file>
<file name="src/main/schemas.ts">
<violation number="1" location="src/main/schemas.ts:12">
P2: Optional string fields are strict-typed, so non-string values fail whole-object parsing instead of being dropped as intended for external/user-edited JSON.</violation>
</file>
Note: This PR contains a large number of files. Free-tier reviews are limited to 40 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review. Paid plans can review more files.
Re-trigger cubic
| const entry = loadManifest().avatars[key] | ||
| if (!entry || !isAvatarCacheKey(entry.key)) return null | ||
|
|
||
| const filePath = join(CACHE_DIR, entry.fileName) |
Contributor
There was a problem hiding this comment.
P1: entry.fileName is not constrained to stay inside CACHE_DIR, enabling path traversal when serving pear-avatar:// files.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/main/avatar-cache.ts, line 179:
<comment>`entry.fileName` is not constrained to stay inside `CACHE_DIR`, enabling path traversal when serving `pear-avatar://` files.</comment>
<file context>
@@ -0,0 +1,203 @@
+ const entry = loadManifest().avatars[key]
+ if (!entry || !isAvatarCacheKey(entry.key)) return null
+
+ const filePath = join(CACHE_DIR, entry.fileName)
+ if (!existsSync(filePath)) return null
+ return { filePath, contentType: entry.contentType || 'image/*' }
</file context>
|
|
||
| // Single privileged step: ensure the dir exists, copy the shim, make it | ||
| // executable. Quotes guard against spaces in the staging path. | ||
| const script = `do shell script "mkdir -p /usr/local/bin && cp '${stagingPath}' '${CLI_TARGET}' && chmod 755 '${CLI_TARGET}'" with administrator privileges` |
Contributor
There was a problem hiding this comment.
P1: The admin-privileged osascript command interpolates an unescaped path, creating a command-injection/command-break risk when the temp path contains quotes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/main/cli-install.ts, line 40:
<comment>The admin-privileged `osascript` command interpolates an unescaped path, creating a command-injection/command-break risk when the temp path contains quotes.</comment>
<file context>
@@ -0,0 +1,69 @@
+
+ // Single privileged step: ensure the dir exists, copy the shim, make it
+ // executable. Quotes guard against spaces in the staging path.
+ const script = `do shell script "mkdir -p /usr/local/bin && cp '${stagingPath}' '${CLI_TARGET}' && chmod 755 '${CLI_TARGET}'" with administrator privileges`
+
+ try {
</file context>
| ? firstDelete.lineNumber | ||
| : getOldAnchorForInsert(hunk, first.index) | ||
| const newStart = firstInsert && firstInsert.type === 'insert' | ||
| ? firstInsert.lineNumber |
Contributor
There was a problem hiding this comment.
P1: Partial-patch hunk headers use full-diff new line numbers, so selecting a subset of changes can generate misnumbered +newStart coordinates.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/renderer/src/lib/diff-selection.ts, line 71:
<comment>Partial-patch hunk headers use full-diff new line numbers, so selecting a subset of changes can generate misnumbered `+newStart` coordinates.</comment>
<file context>
@@ -0,0 +1,145 @@
+ ? firstDelete.lineNumber
+ : getOldAnchorForInsert(hunk, first.index)
+ const newStart = firstInsert && firstInsert.type === 'insert'
+ ? firstInsert.lineNumber
+ : getNewAnchorForDelete(hunk, first.index)
+ const oldLines = deletes.length
</file context>
| ? { expiresAt: data.accessTokenExpiresAt.trim() } | ||
| : {}) | ||
| } | ||
| saveTokens(next) |
Contributor
There was a problem hiding this comment.
P1: In-flight token refresh can restore stale credentials after logout/account change because refresh success always calls saveTokens(next).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/main/auth.ts, line 545:
<comment>In-flight token refresh can restore stale credentials after logout/account change because refresh success always calls `saveTokens(next)`.</comment>
<file context>
@@ -0,0 +1,691 @@
+ ? { expiresAt: data.accessTokenExpiresAt.trim() }
+ : {})
+ }
+ saveTokens(next)
+ return next
+ } catch {
</file context>
| } | ||
|
|
||
| const port = addr.port | ||
| const state = crypto.randomUUID() |
Contributor
There was a problem hiding this comment.
P1: OAuth callback accepts tokens without validating the state parameter, enabling token injection/session swap.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/main/auth.ts, line 392:
<comment>OAuth callback accepts tokens without validating the `state` parameter, enabling token injection/session swap.</comment>
<file context>
@@ -0,0 +1,691 @@
+ }
+
+ const port = addr.port
+ const state = crypto.randomUUID()
+ const redirectUri = encodeURIComponent(`http://127.0.0.1:${port}/auth`)
+ const loginUrl = `${CLOUD_API_URL}/api/v1/cli/login?redirect_uri=${redirectUri}&state=${state}`
</file context>
| @@ -0,0 +1 @@ | |||
| {"workspaceId":"rw_7ccfea89","remoteRoot":"/memory/workspace","localRoot":"/home/daytona/workspace/memory/workspace","mode":"poll","intervalMs":5000,"lastReconcileAt":"2026-06-05T13:15:51.959695444Z","lastSuccessfulReconcileAt":"2026-06-05T13:15:51.959695444Z","staleAfter":"2026-06-05T13:16:01.959695444Z","status":"ready","states":{"stale":false,"offline":false,"hasConflicts":false,"hasPendingWriteback":false},"pendingWriteback":0,"pendingConflicts":0,"deniedPaths":0,"counters":{"snapshotDeleteBlocked":105},"circuit":{"open":false,"openedAt":"0001-01-01T00:00:00Z","windowMs":60000,"cooldownMs":30000,"threshold":5,"nextRetry":"0001-01-01T00:00:00Z"}} No newline at end of file | |||
Contributor
There was a problem hiding this comment.
P2: Runtime workspace-relay state file committed to version control — contains mutable counters, timestamps, and machine-local paths that should be gitignored (like .agent-relay already is)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At memory/workspace/.relay/state.json, line 1:
<comment>Runtime workspace-relay state file committed to version control — contains mutable counters, timestamps, and machine-local paths that should be gitignored (like `.agent-relay` already is)</comment>
<file context>
@@ -0,0 +1 @@
+{"workspaceId":"rw_7ccfea89","remoteRoot":"/memory/workspace","localRoot":"/home/daytona/workspace/memory/workspace","mode":"poll","intervalMs":5000,"lastReconcileAt":"2026-06-05T13:15:51.959695444Z","lastSuccessfulReconcileAt":"2026-06-05T13:15:51.959695444Z","staleAfter":"2026-06-05T13:16:01.959695444Z","status":"ready","states":{"stale":false,"offline":false,"hasConflicts":false,"hasPendingWriteback":false},"pendingWriteback":0,"pendingConflicts":0,"deniedPaths":0,"counters":{"snapshotDeleteBlocked":105},"circuit":{"open":false,"openedAt":"0001-01-01T00:00:00Z","windowMs":60000,"cooldownMs":30000,"threshold":5,"nextRetry":"0001-01-01T00:00:00Z"}}
\ No newline at end of file
</file context>
| } | ||
|
|
||
| const meta = loadAuthMeta() | ||
| return { loggedIn: true, apiUrl: meta.apiUrl, user: meta.user } |
Contributor
There was a problem hiding this comment.
P2: getAuthStatus incorrectly reports logged-in when stored tokens are unreadable or invalid.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/main/auth.ts, line 440:
<comment>`getAuthStatus` incorrectly reports logged-in when stored tokens are unreadable or invalid.</comment>
<file context>
@@ -0,0 +1,691 @@
+ }
+
+ const meta = loadAuthMeta()
+ return { loggedIn: true, apiUrl: meta.apiUrl, user: meta.user }
+}
+
</file context>
| */ | ||
|
|
||
| const trimmedString = z | ||
| .string() |
Contributor
There was a problem hiding this comment.
P2: Optional string fields are strict-typed, so non-string values fail whole-object parsing instead of being dropped as intended for external/user-edited JSON.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/main/schemas.ts, line 12:
<comment>Optional string fields are strict-typed, so non-string values fail whole-object parsing instead of being dropped as intended for external/user-edited JSON.</comment>
<file context>
@@ -0,0 +1,109 @@
+ */
+
+const trimmedString = z
+ .string()
+ .transform((value) => value.trim())
+ .pipe(z.string().min(1))
</file context>
| export function useIsAgentTyping( | ||
| agent: { projectId?: string; name: string; currentState: AgentCurrentState } | ||
| ): boolean { | ||
| const key = `${agent.projectId || 'unknown'}:${agent.name}` |
Contributor
There was a problem hiding this comment.
P3: Duplicate agent-key formatting logic is introduced instead of using a single shared key utility, creating drift risk between typing writes and reads.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/renderer/src/stores/typing-store.ts, line 113:
<comment>Duplicate agent-key formatting logic is introduced instead of using a single shared key utility, creating drift risk between typing writes and reads.</comment>
<file context>
@@ -0,0 +1,120 @@
+export function useIsAgentTyping(
+ agent: { projectId?: string; name: string; currentState: AgentCurrentState }
+): boolean {
+ const key = `${agent.projectId || 'unknown'}:${agent.name}`
+ return useTypingStore((state) => {
+ if (agent.currentState !== 'working') return false
</file context>
| const binary = resolveRelayfileMountBinary() | ||
| if (!binary) { | ||
| throw new Error( | ||
| 'relayfile-mount binary not found. Run `npm run relayfile-mount:install` or set RELAYFILE_MOUNT_BIN.' |
Contributor
There was a problem hiding this comment.
P3: The error message suggests running a non-existent npm run relayfile-mount:install script, which gives users a dead-end recovery path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/main/relayfile-mount-launcher.ts, line 57:
<comment>The error message suggests running a non-existent `npm run relayfile-mount:install` script, which gives users a dead-end recovery path.</comment>
<file context>
@@ -0,0 +1,82 @@
+ const binary = resolveRelayfileMountBinary()
+ if (!binary) {
+ throw new Error(
+ 'relayfile-mount binary not found. Run `npm run relayfile-mount:install` or set RELAYFILE_MOUNT_BIN.'
+ )
+ }
</file context>
The background launch-metadata ingest loop warns after several
consecutive failures (e.g. "launch metadata ingest failed: schema
downgrade: db at version 6..."). The CLI started that loop without an
onWarn handler, so it fell back to writing "warning: ..." straight to
the terminal — scary noise for a transient, non-fatal backend hiccup
while a persona is loading.
Add a small dependency-free file logger modeled on Agent Relay's
(relay/packages/utils/src/logger.ts) and route the launch-metadata
warnings to it. Logs default to ~/.agentworkforce/workforce/logs/cli.log
and never touch stdout.
- packages/cli/src/logger.ts: createLogger(component) with DEBUG/INFO/
WARN/ERROR. Env overrides: AGENTWORKFORCE_LOG_FILE ('-' or '' -> stderr),
AGENTWORKFORCE_LOG_LEVEL (default INFO), AGENTWORKFORCE_LOG_JSON=1.
Never throws; falls back to stderr (never stdout) if the file is
unwritable.
- packages/cli/src/cli.ts: pass onWarn routing launch-metadata warnings
to the log file.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
willwashburn
force-pushed
the
fix/launch-metadata-log-to-file
branch
from
9d84a80 to
ce39654
Compare
|
CodeAnt AI is running Incremental review |
codeant-ai
Bot
added
size:L
|
CodeAnt AI Incremental review completed. |
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.
User description
Problem
Loading a persona surfaces scary terminal noise like:
This comes from the background launch-metadata ingest loop (
packages/cli/src/launch-metadata.ts), which warns after several consecutive failures. The CLI was starting that loop without anonWarnhandler, so it fell back to the module default — writingwarning: …straight toprocess.stderr. The failure is transient and non-fatal, so it shouldn't be polluting the terminal the user is staring at.Fix
Following Agent Relay's pattern (
relay/packages/utils/src/logger.ts), add a tiny dependency-free file logger and route the launch-metadata warnings to it instead of the terminal.packages/cli/src/logger.ts(new):createLogger(component)→.debug/.info/.warn/.error.~/.agentworkforce/workforce/logs/cli.log, derived from the existingdefaultWorkforceHomeDir()helper (honorsAGENT_WORKFORCE_HOME).AGENTWORKFORCE_LOG_FILE(-or empty → stderr),AGENTWORKFORCE_LOG_LEVEL(defaultINFO),AGENTWORKFORCE_LOG_JSON=1.packages/cli/src/cli.ts: passonWarn: (msg) => launchMetadataLog.warn(msg)intostartLaunchMetadataRecording.The lower-level
launch-metadata.tsstays decoupled (it takesonWarnvia injection); the logger is wired at thecli.tslayer, keeping the module testable.Verification
onWarn, so they're unaffected.schema downgrade…message lands in…/logs/cli.log; withAGENTWORKFORCE_LOG_FILE=-it falls back to stderr — never stdout.main(stale sibling-package dist + a missing external@relayfile/adapter-coredep). Confirmed viagit stashthat the count is 5 with and without these changes — this PR adds zero new type errors. A full workspace build isn't runnable locally due to that missing external dependency.🤖 Generated with Claude Code
CodeAnt-AI Description
Route launch-metadata warnings to a log file instead of the terminal
What Changed
Impact
✅ Cleaner persona loading✅ Fewer scary terminal warnings✅ Easier CLI diagnostics💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.