fix(cli): keep docker login output out of build logs - #4909
Conversation
🦋 Changeset detectedLatest commit: c4266ec The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The registry login step's output was collected into the shared build log buffer, so build logs on self-hosted and local builds always carried docker's unencrypted-credentials warning — most confusingly right next to the real error on failed builds. Route login output to debug logging only, push a single 'Logged in to <registry>' marker into the build log on success, and keep returning the full login output when the login itself fails.
5600f7f to
c4266ec
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
Walkthrough
Priority: ⬇️ Low — Defer the Docker login log cleanup because it is a narrow patch to local and self-hosted build output with low issue severity. Severity of issue fixed: Low Merge Risk: 🔵 Low · up to Registry login output is now isolated from build logs while preserving failure output and adding a success marker. The implementation has a minor repository instrumentation-policy gap that should be addressed before final merge. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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.
🧹 Nitpick comments (1)
packages/cli-v3/src/deploy/buildImage.ts (1)
517-519: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a crumbs marker to the new login-output block.
The changed block adds login-output handling but has no
//@Crumbsmarker or `// `#region` `@crumbswrapper. Add one before this block soagentcrumbs stripcan remove the temporary instrumentation.As per coding guidelines, add crumbs as you write code and mark lines with
//@Crumbsor wrap blocks in `// `#region` `@crumbs.Proposed fix
+ // `@crumbs` // Login output (incl. docker's credential-storage warning) stays out of the buildSource: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 51032387-cc84-4771-b42b-6bbb1132668c
📒 Files selected for processing (2)
.changeset/dry-mirrors-shake.mdpackages/cli-v3/src/deploy/buildImage.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 24)
🧰 Additional context used
📓 Path-based instructions (7)
**Prefer static imports over dynamic imports.**
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/cli-v3/src/deploy/buildImage.ts
Add crumbs as you write code — not just when debugging.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/cli-v3/src/deploy/buildImage.ts
Build Docker images using `src/deploy/buildImage.ts` for local Docker/Depot or remote builds
📄 CodeRabbit inference engine (packages/cli-v3/CLAUDE.md)
Files:
packages/cli-v3/src/deploy/buildImage.ts
Deploy mode code should be located in `src/deploy/` and handles bundling, archiving, building Docker images, and pushing to registry
📄 CodeRabbit inference engine (packages/cli-v3/CLAUDE.md)
Files:
packages/cli-v3/src/deploy/buildImage.ts
Use function declarations instead of default exports
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/cli-v3/src/deploy/buildImage.ts
Use types over interfaces for TypeScript Avoid using enums; prefer string unions or const objects instead
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/cli-v3/src/deploy/buildImage.ts
When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs Do not use high-cardinality attributes in OTEL metr...
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
packages/cli-v3/src/deploy/buildImage.ts
🔇 Additional comments (1)
.changeset/dry-mirrors-shake.md (1)
5-5: LGTM!
2 participants
On self-hosted and local builds, the registry login step's output landed in the shared build log buffer, so every build log carried docker's
WARNING! Your credentials are stored unencryptedblock. The warning is misleading in our context: the credentials are per-project, short-lived deploy tokens on an ephemeral builder machine.Login output now goes to debug logging only; a single
Logged in to <registry>marker is pushed into the build log on success (so failure logs still show auth ran), and a failed login still returns its full output.