Refactor NewCliSession for improved keyring configuration

[evgenyk]

Enhance the NewCliSession function to utilize default options for keyring backends, streamlining configuration handling.

[coderabbitai]

Walkthrough

Reorders option application in NewCliSession: defines a defaultOpts slice and applies it before user-provided opts, ensuring user options override defaults. No exported signatures change; session assembly (config build, keyring open, session return) remains otherwise unchanged.

Changes

Cohort / File(s)	Summary
CLI session construction frameworks/cli/cliSession.go	Introduces defaultOpts and applies defaults before user opts. Adjusts option merge order so user-supplied options override defaults. No other logic changes to config creation, keyring access, or session return.

Sequence Diagram(s)

sequenceDiagram
  participant C as Caller
  participant CS as NewCliSession
  participant Cfg as Config Builder
  participant KR as Keyring

  Note over CS: Order change: defaults → user opts
  C->>CS: NewCliSession(opts...)
  CS->>CS: Prepare defaultOpts (backends, trust, names, pw funcs, config dir)
  CS->>Cfg: Apply defaultOpts
  CS->>Cfg: Apply user opts (override defaults)
  Cfg-->>CS: Built config
  CS->>KR: Open keyring per config
  KR-->>CS: Keyring handle
  CS-->>C: CliSession

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• Add support for KINDE_KEYCHAIN_PASS environment variable #31 — Also alters CLI session construction to an options-driven approach and adjusts keyring/option assembly.
• Enhance keychain initialization with custom password prompt for interactive terminal #30 — Modifies NewCliSession with keychain name and password prompt behavior; closely related to option handling changes.

✨ Finishing Touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch ev/fixed_default_cli_opts

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)

frameworks/cli/cliSession.go (4)

84-91: Compile error: ranging over an int.

chunks is an int; for i := range chunks doesn’t compile. Iterate by index.

- for i := range chunks {
+ for i := 0; i < chunks; i++ {

---

110-116: Same compile error in both cleanup loops.

Replace range chunks with an indexed loop in both places.

- for i := range chunks {
+ for i := 0; i < chunks; i++ {

- for i := range chunks {
+ for i := 0; i < chunks; i++ {

Also applies to: 132-138

---

141-151: Save-chunks loop: range over int and undefined min().

• Use indexed loop.
• min isn’t a Go builtin; compute end explicitly.

- for i := range chunks {
-   start := i * chunkSize
-   end := min(start+chunkSize, len(t))
+ for i := 0; i < chunks; i++ {
+   start := i * chunkSize
+   end := start + chunkSize
+   if end > len(t) {
+     end = len(t)
+   }

---

16-18: Namespace keyring keys by service to avoid collisions across sessions.

Keys like "kinde_code_verifier" and "kinde_token" aren’t scoped to serviceName; multiple sessions for different services can overwrite each other. Consider storing serviceName (normalized) in cliSession and prefixing keys with it (e.g., "kinde//token").

Happy to propose a small refactor that adds a service-scoped prefix field to cliSession and updates the Get/Set methods accordingly. Want me to draft it?

🧹 Nitpick comments (1)

frameworks/cli/cliSession.go (1)

155-161: fmt.Appendf portability.

fmt.Appendf requires newer Go; if your module’s go version < 1.20/1.21, prefer strconv for portability.

- countData := fmt.Appendf(nil, "%d", chunks)
+ countData := []byte(strconv.Itoa(chunks))

Outside this hunk, add:

import "strconv"

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6e29f31 and b6a6ecb.

📒 Files selected for processing (1)

• frameworks/cli/cliSession.go (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

frameworks/cli/cliSession.go (1)

frameworks/cli/options.go (8)

• Option (5-5)
• WithAllowedBackends (8-12)
• WithKeychainTrustApplication (50-54)
• WithServiceName (22-26)
• WithKeychainName (29-33)
• WithKeychainPasswordFunc (36-40)
• WithFilePasswordFunc (43-47)
• WithFileDir (15-19)

🔇 Additional comments (1)

frameworks/cli/cliSession.go (1)

229-229: Option precedence LGTM.

Prepending defaults then appending user opts makes user-provided options win. Good.