Merged
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed:
--output-filedoesn't disable interactive TUI picker- Modified logTargetResolutionOptionsForOutput to check outputFile parameter and disable interactive mode when --output-file is set, preventing scripts from hanging on ambiguous job selection.
Or push these changes by commenting:
@cursor push ecc73be0a7
Preview (ecc73be0a7)
diff --git a/pkg/cmd/ci/logs.go b/pkg/cmd/ci/logs.go
--- a/pkg/cmd/ci/logs.go
+++ b/pkg/cmd/ci/logs.go
@@ -129,7 +129,7 @@
reporter := newLogFollowReporter(reporterWriter, reporterInteractive)
// First, try resolving as a run ID (or job ID — the API accepts both).
- resolutionOptions := logTargetResolutionOptionsForOutput(outputOptions)
+ resolutionOptions := logTargetResolutionOptionsForOutput(outputOptions, outputFile)
resp, runErr := ciGetRunStatus(ctx, tokenVal, orgID, id)
if runErr == nil {
target, err := resolveLogTargetWithOptions(resp, id, job, workflow, resolutionOptions)
@@ -807,8 +807,8 @@
allowInteractive bool
}
-func logTargetResolutionOptionsForOutput(outputOptions logOutputOptions) logTargetResolutionOptions {
- return logTargetResolutionOptions{allowInteractive: !outputOptions.json()}
+func logTargetResolutionOptionsForOutput(outputOptions logOutputOptions, outputFile string) logTargetResolutionOptions {
+ return logTargetResolutionOptions{allowInteractive: !outputOptions.json() && outputFile == ""}
}
// jobKeyShort returns the short form of a job key (after the first colon),
diff --git a/pkg/cmd/ci/logs_test.go b/pkg/cmd/ci/logs_test.go
--- a/pkg/cmd/ci/logs_test.go
+++ b/pkg/cmd/ci/logs_test.go
@@ -75,7 +75,7 @@
},
}
- options := logTargetResolutionOptionsForOutput(logOutputOptions{output: logOutputJSON})
+ options := logTargetResolutionOptionsForOutput(logOutputOptions{output: logOutputJSON}, "")
if options.allowInteractive {
t.Fatal("json output should disable interactive job resolution")
}
@@ -89,6 +89,34 @@
}
}
+func TestResolveLogTargetOutputFileOptionsReturnNonInteractiveAmbiguity(t *testing.T) {
+ resp := &civ1.GetRunStatusResponse{
+ RunId: "run-1",
+ Workflows: []*civ1.WorkflowStatus{
+ {
+ WorkflowPath: ".depot/workflows/ci.yml",
+ Jobs: []*civ1.JobStatus{
+ {JobId: "job-1", JobKey: "build", Status: "finished"},
+ {JobId: "job-2", JobKey: "test", Status: "running"},
+ },
+ },
+ },
+ }
+
+ options := logTargetResolutionOptionsForOutput(logOutputOptions{output: logOutputText}, "/tmp/logs.txt")
+ if options.allowInteractive {
+ t.Fatal("output-file should disable interactive job resolution")
+ }
+
+ _, err := resolveLogTargetWithOptions(resp, "run-1", "", "", options)
+ if err == nil {
+ t.Fatal("expected ambiguity error")
+ }
+ if !strings.Contains(err.Error(), "run has multiple jobs, specify one with --job") {
+ t.Fatalf("expected multiple-jobs ambiguity error, got: %v", err)
+ }
+}
+
func TestFindLogsJob_MatchByJobKey(t *testing.T) {
resp := &civ1.GetRunStatusResponse{
RunId: "run-1",You can send follow-ups to the cloud agent here.
b0763ec to
8220e30
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Silent success without file when no logs available
- Modified code to write noLogsMessage to stderr via cmd.ErrOrStderr() when outputFile is set, ensuring user receives feedback instead of silent success.
Or push these changes by commenting:
@cursor push 937bcbab52
Preview (937bcbab52)
diff --git a/pkg/cmd/ci/logs.go b/pkg/cmd/ci/logs.go
--- a/pkg/cmd/ci/logs.go
+++ b/pkg/cmd/ci/logs.go
@@ -136,15 +136,19 @@
if follow && err != nil {
target, err = resolveLogTargetWithFollowRetry(ctx, tokenVal, orgID, id, job, workflow, err, reporter, resolutionOptions)
}
- if err != nil {
- return err
- }
- if target.noLogsMessage != "" {
+ if err != nil {
+ return err
+ }
+ if target.noLogsMessage != "" {
+ if outputFile != "" {
+ fmt.Fprintln(cmd.ErrOrStderr(), target.noLogsMessage)
+ } else {
reporter.Message(target.noLogsMessage)
- return nil
}
+ return nil
+ }
- if outputFile != "" {
+ if outputFile != "" {
return downloadLogsToFile(ctx, tokenVal, orgID, api.CILogStreamTarget{AttemptID: target.attemptID}, outputOptions, outputFile, cmd.ErrOrStderr())
} else if follow {
if err := streamLogsWithFollowUX(ctx, tokenVal, orgID, target, cmd.OutOrStdout(), reporter, outputOptions); err != nil {You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 8220e30. Configure here.
8220e30 to
29b7ba0
Compare
ischolten
approved these changes
May 5, 2026
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
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Summary
depot ci logscan now download exported job logs to a file through the public CI API instead of relying only on stdout rendering. The new--output-filepath streams RPC export chunks directly into a temp file and renames on success, keeping stdout clean for callers.The command prints a lightweight stderr indicator while the download is running and reports the final byte count after the file is written. Human status surfaces also advertise the download command where it is actually useful:
depot ci statusshows it only under finished attempts, anddepot ci workflow showshows it next to the latest-attempt log command only when that latest attempt is finished.What Changed
ExportJobAttemptLogsfrom depot/api#3631.depot ci logs --output-file <path>with timestamped text as the default export format.--output text --output-fileto text and--output json --output-fileto JSONL.--follow --output-fileand--output-file -before network work.--follow,--timestamps, and--output json.ci statusandci workflow showonly for finished attempts.Design Notes
The API export is a framed Connect/gRPC stream, not a raw HTTP download URL. The CLI is the shell-native path for writing export bytes to disk, and the displayed download command is intentionally limited to finished attempts so users do not mistake a partial snapshot for final logs.
The export RPC does not know the final response size before streaming; the CLI reports bytes after the atomic rename succeeds instead of showing a misleading progress bar.
Validation
make generategofmton touched Go filesgo test ./pkg/cmd/cigo test ./pkg/cmd/ci ./pkg/apigo test ./...go vet ./pkg/cmd/ci ./pkg/apimake bin/depotDEPOT_API_URL=http://127.0.0.1:8080 ./bin/depot ci logs ghj7phtgc7 --output-file /tmp/depot-logs-indicator.txtgit diff --checkNote
Medium Risk
Adds a new Connect streaming RPC for exporting CI logs and wires it into the CLI with new file-writing behavior; main risk is correctness/robustness of stream framing and atomic file handling across error cases.
Overview
Adds a new CI API surface for finite log exports: introduces
ExportJobAttemptLogsin the proto/Connect client and a Go wrapperCIExportJobAttemptLogsthat validates targets, requires metadata-before-chunks, and streams chunk bytes into a caller-provided writer.Extends
depot ci logswith--output-fileto download exports to disk (atomic temp file + rename), defaults to text export (or JSONL when--output json), suppresses interactive selection and stdout noise during downloads, and rejects invalid combos like--followwith--output-fileor--output-file -.Updates human-oriented
ci statusandci workflow showoutput to advertise a download command (usinglogs.txt) only for finished attempts, and adds targeted tests covering stream framing errors, fallback resolution, and temp-file cleanup on export failures.Reviewed by Cursor Bugbot for commit 29b7ba0. Bugbot is set up for automated code reviews on this repo. Configure here.