feat: infer --pipeline from heroku.remote git config in CI commands (W-23597910, #1318) - #3845
feat: infer --pipeline from heroku.remote git config in CI commands (W-23597910, #1318)#3845heroku-johnny wants to merge 3 commits into
Conversation
…W-23597910, #1318) When neither --pipeline nor --app is provided, CI commands now resolve the target app from the --remote flag or heroku.remote git config, matching the behavior of heroku run and other app-flag commands. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
michaelmalave
left a comment
There was a problem hiding this comment.
Dev Tools Standards Review Results (machine-generated)
🟡 2 advisory.
| if ((!flags.pipeline) && (!flags.app)) { | ||
| // Resolve app from --remote flag or heroku.remote git config when --pipeline and --app are absent | ||
| let resolvedApp = flags.app | ||
| if (!flags.pipeline && !resolvedApp) { |
There was a problem hiding this comment.
merge-duplicate-conditionals (Code style · minor · 50% confidence): It looks like we're checking the same predicate !flags.pipeline && !resolvedApp in two consecutive if blocks — one to resolve the app from the remote, the next to emit the required-flag error. Should we merge them into a single block that owns the condition (attempt resolution, then error if it's still unresolved) so the predicate only lives in one place?
| import {APIClient} from '@heroku-cli/command' | ||
| import {expect} from 'chai' | ||
| import nock from 'nock' | ||
| import {createSandbox} from 'sinon' |
There was a problem hiding this comment.
delete-unused-code (Code style · minor · 85% confidence): It looks like the sinon sandbox (createSandbox import, the sandbox variable, and its beforeEach/afterEach) is set up but never used — all three tests inject behavior by subclassing PipelineService and overriding resolveAppFromRemote. Should we delete the sandbox scaffolding, or wire the tests to stub through it instead?
michaelmalave
left a comment
There was a problem hiding this comment.
Some non-blocking comments - code functionality tested and confirmed. Nice!
Summary
--remoteflag andheroku.remotegit config intogetPipeline()insrc/lib/ci/pipelines.ts, so CI commands (ci:run,ci:rerun,ci:debug,ci:info,ci:last,ci:open,ci:config, etc.) can resolve their pipeline the same wayheroku runand other app-flag commands already doresolveAppFromRemote()helper onPipelineServicethat callsconfigRemote()/getGitRemotes()from@heroku-cli/command, keeping the git resolution logic injectable and testable via subclassinggetPipeline's flag type signature to includeremote?: null | stringWhat changed
src/lib/ci/pipelines.ts:configRemoteandgetGitRemotesfrom@heroku-cli/command--pipeline/--app, try to resolve an app fromflags.remoteor theheroku.remotegit config value; if a matching Heroku git remote is found, use that app to look up the pipeline coupling as usualtest/unit/lib/ci/pipelines.unit.test.ts:remote inferenceblock that cover: resolution via--remote, resolution via git config, and the fall-through error when no remote resolves an appWhy
Fixes GUS W-23597910 / GitHub #1318. Users who set
heroku.remotein their git config (or pass--remote) expect CI commands to pick up the pipeline automatically, consistent with all other Heroku CLI commands that accept--remote.Test plan
test/unit/lib/ci/pipelines.unit.test.ts— 5 tests pass (3 new + 2 existing)test/unit/commands/ci/run.unit.test.ts— 3 tests passtest/unit/commands/ci/rerun.unit.test.ts— 3 tests passnpm run build— TypeScript compiles cleanlynpx eslint src/lib/ci/pipelines.ts test/unit/lib/ci/pipelines.unit.test.ts— 0 errors, 2 pre-existinganywarnings🤖 Generated with Claude Code