refactor(cli): reuse fetchExistingResourcesWithLabels in deploy planning - #1613
Merged
Conversation
The deploy planners for auth, IdP, resolver, secret manager, function registry, and TailorDB each inlined the same list-then-attach-labels sequence that the shared fetchExistingResourcesWithLabels helper already encapsulates (and that executor/workflow/staticwebsite/aigateway already use). Route these six through the helper. No behavior change. Claude-Session: https://claude.ai/code/session_01Xyr7Zg3HUNUJH1qg6AMqZq
Drop implementation details (helper name, per-service list) from the changeset body; a user-invisible internal refactor only needs the one-line user-facing summary. Claude-Session: https://claude.ai/code/session_0113cu8MxKAhhutUGaXJqmQH
🦋 Changeset detectedLatest commit: 342a22a The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
commit: |
This comment has been minimized.
This comment has been minimized.
![github-actions[bot]](https://avatars.github.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
📖 Docs Quality & Consistency Check
✅ Docs are consistent with the implementation and contain no user-facing internal-detail leaks.
Checked areas:
- CLI documentation (
packages/sdk/docs/cli/application.md) - Deploy command behavior documentation remains accurate; no changes needed since this is a pure refactoring with no user-visible behavior changes - Service documentation (
packages/sdk/docs/services/*.md) - Not affected by internal deploy command refactoring - Developer guide (
AGENTS.md) - No updates needed; no pattern or gotcha changes - Example code (
example/) - Not affected; examples don't use internal deploy command functions - CLI public API (
packages/sdk/src/cli/lib.ts) - Verified thatfetchExistingResourcesWithLabelsand related functions are not exposed to SDK users - Changeset (
.changeset/dedup-deploy-fetchers.md) - Accurately describes this as "Internal refactoring" with "No user-facing behavior change"
PR Summary:
This PR refactors the deploy command to deduplicate existing-resource fetching logic by consolidating six similar code blocks (in auth, IdP, resolver, secret manager, function registry, and TailorDB planning) into a shared helper function fetchExistingResourcesWithLabels. The changes are:
- Purely internal to
packages/sdk/src/cli/commands/deploy/(not part of the public API) - No behavior changes to the deploy command
- No new user-facing APIs or configuration options
- JSDoc comments in
owned-resource.tsdocument internal functions not exposed to users
Verification:
- Confirmed the helper function is not exported from
packages/sdk/src/cli/lib.ts(the public CLI API entry point) - Verified user-facing documentation describes observable behavior (unmanaged resources, ownership conflicts) without mentioning implementation details
- Checked that no new internal details were leaked to published docs, examples, or JSDoc on exported symbols
Re-run this check by adding the
docs-checklabel to the PR.
toiroakr
reviewed
Jul 1, 2026
| } | ||
| }, | ||
| getName: (resource) => resource.namespace?.name, | ||
| getTrn: (workspaceId, name) => resourceTrn(workspaceId, "auth", name), |
Contributor
There was a problem hiding this comment.
The workspaceId argument in getTrn is redundant as it simply passes the workspaceId from the closure at the call site.
By changing the getTrn(name) signature in owned-resource.ts and updating the call site to (name) => resourceTrn(workspaceId, "auth", name), you can remove this argument in all six locations.
Contributor
|
The same pattern for deduplication remains in Since there is a possibility that this same pattern was added in parallel in a separate PR, it would be helpful if you could also align this with |
toiroakr
reviewed
Jul 1, 2026
| // Fetch existing function registry entries | ||
| const existingFunctions = await fetchAll(async (pageToken, maxPageSize) => { | ||
| try { | ||
| // Fetch existing function registry entries with their labels |
Contributor
There was a problem hiding this comment.
This comment is merely a paraphrase of the function name fetchExistingResourcesWithLabels and does not provide any information that cannot already be gathered from the code.
Suggested change
| // Fetch existing function registry entries with their labels |
This comment has been minimized.
This comment has been minimized.
…-fetchers # Conflicts: # packages/sdk/src/cli/shared/context.ts
Code Metrics Report (packages/sdk)
Details | | main (a58efe9) | #1613 (8858c6f) | +/- |
|--------------------|----------------|-----------------|-------|
+ | Coverage | 71.1% | 71.2% | +0.0% |
| Files | 428 | 428 | 0 |
| Lines | 15781 | 15737 | -44 |
- | Covered | 11235 | 11211 | -24 |
+ | Code to Test Ratio | 1:0.4 | 1:0.4 | +0.0 |
| Code | 107143 | 107004 | -139 |
| Test | 48418 | 48418 | 0 |Code coverage of files in pull request scope (77.2% → 77.7%)
SDK Configure Bundle Size
Runtime Performance
Type Performance (instantiations)
Reported by octocov |
Merged
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
Deduplicate existing-resource fetching in the deploy command by routing auth, IdP, resolver, secret manager, function registry, and TailorDB planning through the shared
fetchExistingResourcesWithLabelshelper; no user-facing behavior change.