feat(credential): resolve the active app from the invocation context#1409
Open
evandance wants to merge 1 commit into
Open
feat(credential): resolve the active app from the invocation context#1409evandance wants to merge 1 commit into
evandance wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR adds environment-based profile pinning via ChangesProfile Resolution via Environment Variable
Multi-App Config Binding
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant OpenClawBinder
participant Keychain
participant Store
User->>CLI: run `config bind --all`
CLI->>OpenClawBinder: Resolve source and call BuildAll()
OpenClawBinder->>OpenClawBinder: buildFromCandidate per app (resolve secrets)
OpenClawBinder->>Keychain: store secrets via core.ForStorage
OpenClawBinder->>CLI: return []AppConfig
CLI->>CLI: sortAppsDefaultFirst -> dedupAndOrderApps
CLI->>Store: commitMultiAppBinding (write MultiAppConfig)
CLI->>Keychain: cleanupKeychainFromDataMulti (remove replaced secrets)
CLI->>User: print banner and stdout JSON envelope
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
github-actions
Bot
added
the
size/L
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/config/bind_test.go`:
- Around line 1939-1953: Update the failing tests (e.g.,
TestConfigBindRun_AllRejectsAppID) to assert typed error metadata instead of
string matching: call errs.ProblemOf(err) and verify the returned Problem's
Category/Subtype values, and use errors.As(err, &ve) with var ve
*errs.ValidationError to assert ve.Param equals the expected flag (e.g., "--all"
or "--app-id"); locate the failure by the test name and the call to
configBindRun/Bin dOptions and apply the same typed assertions to the sibling
test in the 2019-2032 range so both error-path tests validate problem metadata
and param preservation.
In `@cmd/config/bind.go`:
- Around line 134-136: The early return in configBindRun when opts.All calls
configBindAllRun and skips finalizeSource, allowing --all to bypass the
source/env mismatch guard; change the flow so finalizeSource (and its agent env
checks) runs before delegating to configBindAllRun (and similarly ensure the
same fix is applied to the other all-path that mirrors this logic), i.e.,
validate and finalize the source via finalizeSource within configBindRun (or
call a common validate-then-bind helper) before calling configBindAllRun to
preserve the source vs detected agent environment check.
- Around line 733-751: The --all path in configBindAllRun applies opts.Identity
to every app but never invokes warnIdentityEscalation, allowing
bot-only→user-default escalation without --force; fix by invoking
warnIdentityEscalation for each app after applyPreferences (inside the for i :=
range apps loop) or just after building apps and before commitMultiAppBinding so
the escalation gate runs per-app; call warnIdentityEscalation(app, opts, prior)
(or the existing signature) and propagate/return any error it produces so
--force behavior and safety checks are enforced.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ac42fd33-ce8e-4b28-bc52-299fd7842347
📒 Files selected for processing (7)
cmd/bootstrap.gocmd/bootstrap_test.gocmd/config/bind.gocmd/config/bind_test.gocmd/config/binder.gointernal/credential/default_provider.gointernal/envvars/envvars.go
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@417b0d18203dcad7a299bd7573171af2d6c85620🧩 Skill updatenpx skills add larksuite/cli#feat/runtime-app-id -y -g |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1409 +/- ##
========================================
Coverage 72.83% 72.83%
========================================
Files 731 732 +1
Lines 69111 69264 +153
========================================
+ Hits 50335 50451 +116
- Misses 14999 15020 +21
- Partials 3777 3793 +16 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Each lark-cli invocation can now carry its own app identity, so callers bound to different apps no longer compete over a single stored default profile.
evandance
force-pushed
the
feat/runtime-app-id
branch
from
10f0b7e to
417b0d1
Compare
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
1 participant
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
lark-cli stores a single default app profile, so concurrent callers bound to different apps compete over one global default. This PR resolves the active profile per invocation —
--profileflag first, then theLARKSUITE_CLI_RUNTIME_APP_IDenv var, falling back to the stored default — and adds a hiddenconfig bind --allthat binds every account from the agent source as a multi-app config in one pass.Changes
--profile/LARKSUITE_CLI_RUNTIME_APP_IDincmd/bootstrap.goLARKSUITE_CLI_RUNTIME_APP_IDconstant ininternal/envvars/envvars.go--allflag toconfig bindwith multi-app commit and keychain cleanup incmd/config/bind.goBuildAll/ per-candidate build to the openclaw binder incmd/config/binder.gointernal/credential/default_provider.gocmd/bootstrap_test.go;--allvalidation, dedup ordering, and run paths incmd/config/bind_test.goTest Plan
make unit-testpassedgofmt -l,go vet ./...,go build ./...all clean)shortcuts/orskills/surface touchedgo test ./cmd/ -run TestBootstrapInvocationContext -vandgo test ./cmd/config/...— precedence and--allpaths passRelated Issues
N/A
Summary by CodeRabbit
New Features
Tests