refactor(apex-debugger): move sf-CLI check to debugger, drop core isCLIInstalled - W-23291340#7674
Merged
mshanemc merged 9 commits intoJul 6, 2026
Conversation
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…- W-23291340 Finish Effect migration of the ISV setup block (Effect.log, Effect.tryPromise + IsvAuthSetupError, catchTag) and drop the core isCLIInstalled gate. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- avoid `any` in activation.test.ts (narrow casts, matching repo pattern) - concise plan wording (fragments, numerals) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…iscliinstalled-to-terminalservice-mi
…rovide - W-23291340 Effect.provide narrowing left a TerminalService R leak against the effect's never-R type; cast the provided effect to match, mirroring orgOpen.test.ts. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…alled - W-23291340
isCliInstalled had a single consumer (apex-debugger) and baked a
consumer-specific catch-to-false policy into the shared TerminalService
seam. Move the check to the debugger: compose simpleExec('sf --version')
+ catchTag inline where the policy lives; services keeps only the
simpleExec primitive.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
mshanemc
changed the title
…3291340 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
/ai-auto approve |
peternhale
approved these changes
Jul 6, 2026
peternhale
left a comment
peternhale
left a comment
Contributor
There was a problem hiding this comment.
Peer-approved on behalf of @mshanemc per /ai-auto approve
mshanemc
deleted the
sm/W-23291340-add-iscliinstalled-to-terminalservice-mi
branch
July 6, 2026 13:16
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
2 participants
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
isCLIInstalled(sfdx --version) onto servicesTerminalService.simpleExec(sf --version). The CLI-presence check is composed inline at the call site —simpleExec({ command: 'sf --version', parse: () => true }).pipe(catchTag('TerminalServiceError', () => false))— because it has a single consumer and the catch-to-falsesemantics are the debugger's policy, not a shared-service concern. Services keeps only thesimpleExecprimitive.Effect.log,Effect.tryPromise+IsvAuthSetupErrortagged error,catchTagfor the non-fatal ISV-setup-failure path).isCLIInstalledfromsalesforcedx-vscode-core(util, API export, tests) — apex-debugger was its sole consumer.Design note
Earlier revisions added
isCliInstalledtoTerminalService. Reverted: a single consumer plus consumer-specific catch-to-falsesemantics don't justify widening the shared services API. The check now lives where its policy lives;simpleExecremains the shared seam.Reviewer notes
packages/salesforcedx-vscode-apex-debugger/src/index.ts(activateEffect): the ISV gate needs ~7 mocks to test becauseactivateEffectmixes vscode registration,TerminalServiceresolution, ISV setup, and telemetry. Could extract the "check CLI → run ISV setup →catchTagwarn" block into its own small Effect. Not applied — reshapes activation structure; the added test covers all 3 branches and passes.packages/salesforcedx-vscode-apex-debugger/src/index.ts: the migrated ISV catch keeps onlye.message, dropping the stack the oldconsole.error(e)captured. Fix would add acausefield toIsvAuthSetupError— a design decision on the error type, not applied.simpleExecinjectsSF_JSON_TO_STDOUT/FORCE_COLORfor anysfcommand even thoughsf --versiondoesn't use them. Harmless.Test plan
isvDebuggerSid/isvDebuggerUrlconfig → confirm "Setting up ISV Debugger environment variables" logs andsf:isv_debug_projectcontext becomes true.sfoff PATH → confirm the gate goes false (setup skipped) with no crash.What issues does this PR fix or reference?
@W-23291340@
🤖 Generated by auto-build pipeline. Original WI: https://gus.lightning.force.com/lightning/r/ADM_Work__c/a07EE00002dmx8MYAQ/view