feat(autoconf): add kdn autoconf command#379
Merged
feloy merged 10 commits intoopenkaiden:mainfrom May 5, 2026
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new autoconf feature: CLI command and runner that detect secrets from environment via registered secret services, filter already-configured secrets, optionally create secrets in the store, and record secret references into global/project/local configuration targets. ChangesAutoconf feature (single cohesive DAG)
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as User
participant CLI as "kdn autoconf"
participant Detector as SecretDetector
participant Filter as SecretFilter
participant Store as SecretStore
participant Updaters as ConfigUpdaters
User->>CLI: run autoconf [--yes]
CLI->>Detector: Detect()
Detector->>Filter: (optional) Filter detected secrets
Detector-->>CLI: FilterResult (NeedsAction, Configured)
CLI->>CLI: print Configured list
loop each secret in NeedsAction
alt interactive (no --yes)
CLI->>User: Confirm create?
User-->>CLI: yes/no
else --yes
CLI-->>CLI: auto-confirm
end
alt confirmed
CLI->>Store: Get / Create secret
Store-->>CLI: success / already exists / error
CLI->>User: SelectTarget (global/project/local)
User-->>CLI: target / abort
alt target selected
CLI->>Updaters: AddSecret(target)
Updaters-->>CLI: updated
else aborted (ErrSkipped)
CLI-->>CLI: print "Skipped adding"
end
else declined
CLI-->>CLI: print "Skipped"
end
end
CLI-->>User: done
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
pkg/config/workspaceupdater_test.go (1)
65-91: Consider checking errors from setup calls.Several tests ignore errors from
NewWorkspaceConfigUpdaterandAddSecretusing_. While the tests would fail on subsequent assertions if these errored, explicit error checking improves debuggability.♻️ Suggested improvement
func TestWorkspaceUpdater_Idempotent(t *testing.T) { t.Parallel() dir := t.TempDir() - u, _ := NewWorkspaceConfigUpdater(dir) - _ = u.AddSecret("github") - _ = u.AddSecret("github") + u, err := NewWorkspaceConfigUpdater(dir) + if err != nil { + t.Fatalf("NewWorkspaceConfigUpdater: %v", err) + } + if err := u.AddSecret("github"); err != nil { + t.Fatalf("AddSecret: %v", err) + } + if err := u.AddSecret("github"); err != nil { + t.Fatalf("AddSecret: %v", err) + } secrets := readWorkspaceSecrets(t, dir)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/config/workspaceupdater_test.go` around lines 65 - 91, Update the two tests (TestWorkspaceUpdater_Idempotent and TestWorkspaceUpdater_MultipleCalls_Accumulate) to stop discarding errors: capture and check the error returned by NewWorkspaceConfigUpdater and each AddSecret call (use variables like u, err := NewWorkspaceConfigUpdater(...) and if err != nil { t.Fatalf(...) } and similarly check err after u.AddSecret(...)); this ensures failures produce immediate, clear test failures and easier debugging.pkg/autoconf/autoconf.go (2)
136-142: Consider handlingfmt.Fprintf/fmt.Fprintlnreturn values.Static analysis flags unchecked error returns from
fmt.Fprintfandfmt.Fprintln. While ignoring these is common practice for CLI output to stdout, theoutparameter is an injectedio.Writerthat could be any destination. For robustness, consider using a helper or explicitly discarding with_ =.♻️ Optional: Explicitly discard or wrap output calls
One approach is to use a helper function that wraps the writes:
func fprint(w io.Writer, format string, args ...any) { _, _ = fmt.Fprintf(w, format, args...) }Or simply prefix with
_, _ =to explicitly acknowledge the ignored return values.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/autoconf/autoconf.go` around lines 136 - 142, The fmt.Fprintf and fmt.Fprintln calls in the loop over result.Configured and the subsequent No-secrets branch ignore their returned (int, error) values; because out is an injected io.Writer this can fail, so update these calls to explicitly discard or handle the returns (e.g., prefix with "_, _ =" or centralize writes via a small helper like fprint(w io.Writer, format string, args ...any) that calls fmt.Fprintf and discards the return). Locate the writes that reference fmt.Fprintf(out, ...) and fmt.Fprintln(out, ...) and change them to use the chosen pattern so the returned error is acknowledged.
273-294: VerifyapplyTargetsafety forConfigTargetLocal.The
ConfigTargetLocalcase at line 287-291 callsa.workspaceUpdater.AddSecret()without a nil check. This is safe becausebuildTargetOptions()only includes the local option whenworkspaceUpdater != nil, but this relies on an implicit invariant.Consider adding a defensive check or a comment documenting the invariant for future maintainers.
♻️ Optional: Add defensive nil check
case ConfigTargetLocal: + if a.workspaceUpdater == nil { + return fmt.Errorf("workspace updater not configured for local target") + } if err := a.workspaceUpdater.AddSecret(secretName); err != nil {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/autoconf/autoconf.go` around lines 273 - 294, The ConfigTargetLocal branch in applyTarget calls a.workspaceUpdater.AddSecret without verifying workspaceUpdater is non-nil, relying on buildTargetOptions to only add local when workspaceUpdater != nil; update applyTarget to either (1) defensively check if a.workspaceUpdater == nil and return a clear error (or skip) before calling workspaceUpdater.AddSecret, or (2) add a concise comment above the ConfigTargetLocal case referencing buildTargetOptions as the invariant ensuring workspaceUpdater != nil (mentioning applyTarget, ConfigTargetLocal, workspaceUpdater.AddSecret, and buildTargetOptions) so future maintainers understand the implicit guarantee.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 680-682: The README currently shows raw secrets inline in a shell
command ("ANTHROPIC_API_KEY=sk-ant-... GH_TOKEN=ghp_... kdn autoconf --yes");
replace that example with a safe pattern: remove literal secret values and
demonstrate using environment variables or an env-file (e.g., export
ANTHROPIC_API_KEY="<REDACTED>" && export GH_TOKEN="<REDACTED>" or reference a
.env file with a placeholder line ANTHROPIC_API_KEY=<YOUR_KEY> and
GH_TOKEN=<YOUR_TOKEN>), and optionally add a note to use secret stores (e.g., gh
secret set or CI secret mechanisms) instead of embedding credentials in command
history to avoid credential leakage.
---
Nitpick comments:
In `@pkg/autoconf/autoconf.go`:
- Around line 136-142: The fmt.Fprintf and fmt.Fprintln calls in the loop over
result.Configured and the subsequent No-secrets branch ignore their returned
(int, error) values; because out is an injected io.Writer this can fail, so
update these calls to explicitly discard or handle the returns (e.g., prefix
with "_, _ =" or centralize writes via a small helper like fprint(w io.Writer,
format string, args ...any) that calls fmt.Fprintf and discards the return).
Locate the writes that reference fmt.Fprintf(out, ...) and fmt.Fprintln(out,
...) and change them to use the chosen pattern so the returned error is
acknowledged.
- Around line 273-294: The ConfigTargetLocal branch in applyTarget calls
a.workspaceUpdater.AddSecret without verifying workspaceUpdater is non-nil,
relying on buildTargetOptions to only add local when workspaceUpdater != nil;
update applyTarget to either (1) defensively check if a.workspaceUpdater == nil
and return a clear error (or skip) before calling workspaceUpdater.AddSecret, or
(2) add a concise comment above the ConfigTargetLocal case referencing
buildTargetOptions as the invariant ensuring workspaceUpdater != nil (mentioning
applyTarget, ConfigTargetLocal, workspaceUpdater.AddSecret, and
buildTargetOptions) so future maintainers understand the implicit guarantee.
In `@pkg/config/workspaceupdater_test.go`:
- Around line 65-91: Update the two tests (TestWorkspaceUpdater_Idempotent and
TestWorkspaceUpdater_MultipleCalls_Accumulate) to stop discarding errors:
capture and check the error returned by NewWorkspaceConfigUpdater and each
AddSecret call (use variables like u, err := NewWorkspaceConfigUpdater(...) and
if err != nil { t.Fatalf(...) } and similarly check err after u.AddSecret(...));
this ensures failures produce immediate, clear test failures and easier
debugging.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c1fe8dcd-e05b-4209-9d2b-8683559400a9
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (20)
.agents/skills/working-with-autoconf/SKILL.mdAGENTS.mdREADME.mdgo.modpkg/autoconf/autoconf.gopkg/autoconf/autoconf_test.gopkg/autoconf/detect.gopkg/autoconf/detect_test.gopkg/autoconf/filter.gopkg/autoconf/filter_test.gopkg/cmd/autoconf.gopkg/cmd/autoconf_test.gopkg/cmd/root.gopkg/cmd/testutil/example_validator.gopkg/cmd/testutil/example_validator_test.gopkg/config/projectsupdater.gopkg/config/projectsupdater_test.gopkg/config/workspaceupdater.gopkg/config/workspaceupdater_test.gopkg/secretservicesetup/register.go
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Contributor
|
./kdn autoconf 3s When asked to create .kaiden/workspace.json I said yes, and then set to share GEMINI_API_KEY globally. So I end up with an empty .kaiden/workspace.json file. It should only be created if necessary, i.e. when user selects workspace scope |
Contributor
Author
Yes you're right, I'll change |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
pkg/cmd/testutil/example_validator.go (1)
80-83: Optional: simplify the character-class condition for readability.The predicate at Line 81 is correct but dense; introducing small booleans makes maintenance easier.
♻️ Suggested readability refactor
for _, ch := range key { - if !((ch >= 'A' && ch <= 'Z') || (ch >= 'a' && ch <= 'z') || (ch >= '0' && ch <= '9') || ch == '_') { + isUpper := ch >= 'A' && ch <= 'Z' + isLower := ch >= 'a' && ch <= 'z' + isDigit := ch >= '0' && ch <= '9' + if !(isUpper || isLower || isDigit || ch == '_') { return false } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cmd/testutil/example_validator.go` around lines 80 - 83, The character-class check inside the loop over "key" is correct but hard to read; refactor the condition in the loop (for _, ch := range key) by introducing small boolean locals like isUpper, isLower, isDigit and isUnderscore and then replace the dense if with if !(isUpper || isLower || isDigit || isUnderscore) { return false }, which preserves behavior while improving readability and maintainability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/autoconf/autoconf.go`:
- Around line 239-258: applyTarget currently silently ignores unknown
ConfigTarget values (returning nil), which can cause secrets to be created
without updating configs; update the applyTarget method to handle unexpected
enum values by adding a default case that returns a clear error (e.g.,
fmt.Errorf("unknown ConfigTarget: %v", target)) so callers like SelectTarget
cannot inject a bad value and cause silent failures; keep the existing branches
for ConfigTargetGlobal, ConfigTargetProject, and ConfigTargetLocal and ensure
the function returns an error for the default path.
- Around line 121-259: The writes to the provided out writer are currently
ignoring returned errors; update all fmt.Fprintf/Fprintln calls in Run,
processSecret, createSecret, addToConfig and applyTarget to check their error
return and propagate it (wrap with context, e.g. fmt.Errorf("write failed: %w")
or similar) so the command stops on broken pipes/closed writers; specifically
locate calls that print status lines (including the "Secret %q already
configured", "Detected secret %q", skip/create/added messages that use
greenCheck/greyDash, and the "No secrets detected" message) and return any write
errors instead of dropping them.
---
Nitpick comments:
In `@pkg/cmd/testutil/example_validator.go`:
- Around line 80-83: The character-class check inside the loop over "key" is
correct but hard to read; refactor the condition in the loop (for _, ch := range
key) by introducing small boolean locals like isUpper, isLower, isDigit and
isUnderscore and then replace the dense if with if !(isUpper || isLower ||
isDigit || isUnderscore) { return false }, which preserves behavior while
improving readability and maintainability.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 052c04cc-7973-4a99-ab82-e520c3aab6ae
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (20)
.agents/skills/working-with-autoconf/SKILL.mdAGENTS.mdREADME.mdgo.modpkg/autoconf/autoconf.gopkg/autoconf/autoconf_test.gopkg/autoconf/detect.gopkg/autoconf/detect_test.gopkg/autoconf/filter.gopkg/autoconf/filter_test.gopkg/cmd/autoconf.gopkg/cmd/autoconf_test.gopkg/cmd/root.gopkg/cmd/testutil/example_validator.gopkg/cmd/testutil/example_validator_test.gopkg/config/projectsupdater.gopkg/config/projectsupdater_test.gopkg/config/workspaceupdater.gopkg/config/workspaceupdater_test.gopkg/secretservicesetup/register.go
✅ Files skipped from review due to trivial changes (4)
- AGENTS.md
- README.md
- .agents/skills/working-with-autoconf/SKILL.md
- go.mod
🚧 Files skipped from review as they are similar to previous changes (8)
- pkg/cmd/root.go
- pkg/cmd/testutil/example_validator_test.go
- pkg/secretservicesetup/register.go
- pkg/cmd/autoconf_test.go
- pkg/config/projectsupdater.go
- pkg/config/workspaceupdater_test.go
- pkg/autoconf/filter.go
- pkg/autoconf/detect.go
feloy
force-pushed
the
autoconf-secrets
branch
2 times, most recently
from
99cdb3a to
3e8c452
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/autoconf/autoconf.go`:
- Around line 253-255: The ConfigTargetLocal branch calls
a.workspaceUpdater.AddSecret without checking for nil, which can panic if
SelectTarget returns local but no workspace updater is configured; update the
case for ConfigTargetLocal in pkg/autoconf/autoconf.go to first check that
a.workspaceUpdater is not nil and return a clear error (or handle appropriately)
when it is nil, otherwise call a.workspaceUpdater.AddSecret(secretName) and
propagate any returned error (preserving the existing fmt.Errorf wrapping).
In `@pkg/autoconf/detect.go`:
- Around line 30-35: DetectedSecret is currently an exported concrete struct;
change it to an exported interface and provide an unexported concrete
implementation plus a factory: declare type DetectedSecret interface {
ServiceName() string; EnvVarName() string; Value() string }, add an unexported
struct (e.g. detectedSecretImpl) that implements those methods, and add a
constructor NewDetectedSecret(service, envVar, value string) DetectedSecret that
returns the unexported implementation; update any code referencing the struct
fields to use the interface accessor methods or the factory return type
(references: DetectedSecret, NewDetectedSecret, detectedSecretImpl).
In `@pkg/autoconf/filter.go`:
- Around line 32-46: Change the exported structs ConfiguredSecret and
FilterResult into interfaces and provide unexported concrete implementations and
constructors: define interfaces named ConfiguredSecret and FilterResult, add
unexported structs configuredSecret and filterResult that implement those
interfaces, and supply factory functions NewConfiguredSecret(...) and
NewFilterResult(...) that return the interface types. Update any creation sites
and the SecretFilter.Filter signature/return type to use the FilterResult
interface (and use NewConfiguredSecret to create configured entries), and ensure
fields (Locations, NeedsAction, Configured) are exposed via interface methods on
the new interfaces so existing callers use the methods instead of struct fields.
- Around line 104-106: The code currently treats any error from
f.store.Get(d.ServiceName) as "not in store" by setting inStore := storeErr ==
nil; instead, detect whether storeErr means "not found" vs an actual failure:
call f.store.Get(d.ServiceName), if storeErr == nil set inStore=true; else if
errors.Is(storeErr, <store.NotFoundErr> or whatever sentinel your store uses)
set inStore=false; otherwise return or propagate storeErr (do not downgrade to
NeedsAction). Update the classification logic around f.store.Get/d.ServiceName
to handle non-not-found errors by returning the error up the call stack. Ensure
you use errors.Is (or the store package's exported sentinel) rather than string
matching.
In `@pkg/cmd/autoconf.go`:
- Around line 97-101: The workspace config updater (created via
config.NewWorkspaceConfigUpdater) is being instantiated in preRun causing
.kaiden/workspace.json to be created even when the user later selects a global
target; defer creation until the user actually selects ConfigTargetLocal by
removing the NewWorkspaceConfigUpdater call from preRun and instead
instantiating a.workspaceUpdater only at the point where ConfigTargetLocal is
confirmed (e.g., in the run/command execution path or the target-selection
handler), and propagate or handle any error from that call there so you only
create the workspace updater when needed.
In `@pkg/config/workspaceupdater.go`:
- Around line 80-93: The readConfig function currently fails when workspace.json
exists but is zero-bytes or only whitespace because json.Unmarshal returns EOF;
update workspaceConfigUpdater.readConfig to treat empty or whitespace-only file
contents as an empty configuration by trimming whitespace from data (e.g.,
bytes.TrimSpace) and returning &workspace.WorkspaceConfiguration{} when the
trimmed length is zero before calling json.Unmarshal, keeping existing error
handling for other read/parse errors.
In `@pkg/project/project.go`:
- Around line 52-57: In DetectProject, when gitDetector.DetectRepository fails
and you take the non-git fallback, convert the supplied dir to an absolute path
before returning it: call filepath.Abs(dir) inside the error branch of
detector.DetectProject, handle the Abs error (e.g., if Abs returns an error,
still return the original dir as a safe fallback) and return the absolute path
string so project IDs don’t vary with the caller’s cwd; update imports to
include "path/filepath" if needed and reference the existing DetectProject
method and gitDetector.DetectRepository in your change.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f4d5c008-cbdb-4083-9643-d591e0cf4c14
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (27)
.agents/skills/working-with-autoconf/SKILL.mdAGENTS.mdREADME.mdgo.modpkg/autoconf/autoconf.gopkg/autoconf/autoconf_test.gopkg/autoconf/detect.gopkg/autoconf/detect_test.gopkg/autoconf/filter.gopkg/autoconf/filter_test.gopkg/cmd/autoconf.gopkg/cmd/autoconf_test.gopkg/cmd/root.gopkg/cmd/testutil/example_validator.gopkg/cmd/testutil/example_validator_test.gopkg/config/projectsupdater.gopkg/config/projectsupdater_test.gopkg/config/workspaceupdater.gopkg/config/workspaceupdater_test.gopkg/instances/manager.gopkg/instances/manager_project_test.gopkg/instances/manager_terminal_test.gopkg/instances/manager_test.gopkg/project/project.gopkg/project/project_test.gopkg/secretservicesetup/register.gopkg/secretservicesetup/register_test.go
💤 Files with no reviewable changes (1)
- pkg/instances/manager_project_test.go
✅ Files skipped from review due to trivial changes (2)
- pkg/config/projectsupdater.go
- .agents/skills/working-with-autoconf/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (3)
- pkg/cmd/testutil/example_validator_test.go
- README.md
- pkg/autoconf/autoconf_test.go
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/autoconf/autoconf.go`:
- Around line 225-229: buildTargetOptions currently always adds a
ConfigTargetProject entry even when a.projectID is empty, which lets users
select "Project" but results in AddSecret("", ...) writing as global; change
buildTargetOptions to only append the {Target: ConfigTargetProject, Label: ...}
option when a.projectID != "" and add a defensive check where the project-target
is handled (the code path that calls AddSecret / the project-branch logic
referenced around lines 247-249) to reject or error if
Target==ConfigTargetProject but a.projectID == "" so we never perform a silent
project-scoped write with an empty project ID.
In `@pkg/autoconf/filter_test.go`:
- Around line 400-403: Replace the weak length-only assertion on
got.Configured[0].Locations with a concrete equality check against the exact
expected order [ConfigTargetProject, ConfigTargetLocal]; e.g., compare locs
(from got.Configured[0].Locations) to the expected slice using reflect.DeepEqual
(import reflect if needed) or the testing/compare utility and call t.Errorf or
t.Fatalf with both expected and actual when they differ so the test fails if the
wrong pair (like global+local) is returned.
In `@pkg/config/workspaceupdater_test.go`:
- Around line 205-207: The test uses os.Getuid() which is unavailable on Windows
and causes compile errors; either add a platform build constraint (add
//go:build !windows at the top of workspaceupdater_test.go before the package
declaration) to exclude the file on Windows, or replace the os.Getuid() check
with a cross-platform alternative (use user.Current() from the os/user package
and inspect u.Uid or compare u.Username to detect root-equivalent) and update
the test guard around that code (references: os.Getuid(),
workspaceupdater_test.go test function containing the runtime skip).
In `@pkg/project/project_test.go`:
- Around line 35-47: The test TestDetectProject_NotGitRepo assumes DetectProject
returns the literal "/some/dir", but DetectProject normalizes paths with
filepath.Abs; update the test to compute the expected absolute path instead of
hardcoding "/some/dir": inside TestDetectProject_NotGitRepo call
filepath.Abs("/some/dir") (handle the returned error with t.Fatal or t.Fatalf)
to produce expected, then compare got to that expected; reference functions:
TestDetectProject_NotGitRepo, detect, and DetectProject.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 95d826e9-b18b-4339-9b90-418b9d35d4e1
📒 Files selected for processing (20)
.agents/skills/cross-platform-development/SKILL.md.agents/skills/working-with-autoconf/SKILL.mdAGENTS.mdREADME.mdpkg/autoconf/autoconf.gopkg/autoconf/autoconf_test.gopkg/autoconf/detect_test.gopkg/autoconf/filter.gopkg/autoconf/filter_test.gopkg/cmd/autoconf.gopkg/cmd/autoconf_test.gopkg/config/projectsupdater_test.gopkg/config/workspaceupdater.gopkg/config/workspaceupdater_test.gopkg/instances/manager.gopkg/instances/manager_project_test.gopkg/instances/manager_terminal_test.gopkg/instances/manager_test.gopkg/project/project.gopkg/project/project_test.go
💤 Files with no reviewable changes (1)
- pkg/instances/manager_project_test.go
✅ Files skipped from review due to trivial changes (4)
- .agents/skills/working-with-autoconf/SKILL.md
- pkg/cmd/autoconf.go
- README.md
- pkg/instances/manager_terminal_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- AGENTS.md
- pkg/autoconf/filter.go
- pkg/instances/manager_test.go
Contributor
Author
|
@benoitf @jeffmaury @fbricon this PR is ready for review |
benoitf
approved these changes
May 5, 2026
Scans registered secret services' environment variables, classifies detected secrets against the store and all config sources (global, project-specific, local workspace.json), and for each unset secret confirms creation and prompts for a config target. --yes skips all prompts and defaults to the global config. New supporting types: ProjectConfigUpdater, WorkspaceConfigUpdater (pkg/config), ListServices() (secretservicesetup), and a "Configuration Commands" group in the root command. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
…on demand Removed the upfront workspace init prompt and WorkspaceFileExists flag. WorkspaceConfigUpdater.AddSecret already handles a missing file via MkdirAll, so the local target is now always listed and the file is created only if the user actually selects it. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
Adds tests for preRun dependency wiring (store, updater, detector, confirm, selectTarget, projectID), the preRun nil-guard for injected detectors, run() with no secrets and with --yes, and all four detectProjectID scenarios (non-git, no-remote, with-remote, with-remote + subdir). Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
…ilable/ListServices Adds tests for the four previously uncovered functions: listServicesWithFactories, listAvailableWithFactories, ListAvailable, and ListServices. Coverage rises from 59% to 94%. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
t.TempDir() on macOS returns paths under /var/folders/... which is a symlink to /private/var/..., while git rev-parse --show-toplevel resolves to the real path. Calling filepath.EvalSymlinks on the temp dir before use ensures the paths match and the relative-path computation produces the expected result. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
…ctor instances.manager.detectProject and cmd.detectProjectID contained identical logic for computing a project identifier from a source directory. The new pkg/project package exposes a Detector interface backed by git.Detector, giving both callers a single authoritative implementation with its own test suite. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com>
- Propagate non-ErrSecretNotFound store.Get errors in filter.go instead of silently treating backend failures as "not in store" - Guard ConfigTargetLocal against nil workspaceUpdater in applyTarget - Add default case to applyTarget to reject unknown ConfigTarget values - Treat empty/whitespace-only workspace.json as missing in readConfig - Normalize non-git dir to absolute path in project.DetectProject - Replace raw inline secrets in examples with command substitution Tests added for each new behaviour. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
The hardcoded Unix path "/some/dir" was converted to a drive-prefixed Windows path by filepath.Abs, causing the test to fail on Windows. Using t.TempDir() provides a real absolute path that is stable across all platforms. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
buildTargetOptions was always adding the "Project" option even when no
project was detected; selecting it called AddSecret("", ...), silently
writing to the global scope instead. The project option is now
conditional on projectID != "", and applyTarget returns an explicit
error if the project target is somehow selected with an empty ID.
Also tightens TestFilter_ProjectScopedLocation_WithLocal to assert the
actual location values, fixes discarded errors in two workspace updater
test setups, and simplifies the char-class condition in
isEnvVarAssignment.
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com>
This was referenced May 7, 2026
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
3 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.
Autoconfiguration based on user environment, 1st part: secrets
Scans registered secret services' environment variables, classifies detected secrets against the store and all config sources (global, project-specific, local workspace.json), and for each unset secret confirms creation and prompts for a config target. --yes skips all prompts and defaults to the global config.
New supporting types: ProjectConfigUpdater, WorkspaceConfigUpdater (pkg/config), ListServices() (secretservicesetup), and a "Configuration Commands" group in the root command.
Move detectProject to a dedicated package pkg/project, so it can be reused by autoconf.
Part of #377
To test it: