You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Extract the profile-resolution chain from buildClaudeLaunchPlan (cmd/databricks-claude/launch_claude.go:75-79) into a pure, testable resolveProfile helper, so the guarantee from #35 ("remove env var from profile resolution — state file always wins", e26f7ca) is actually covered by a test.
Framing (read this first)
This is "make #35 testable", not"restore coverage deleted by #216". There is no coverage to restore.
#216's investigation found that TestProfileResolution_StateFileWins (main_test.go:688-720) had #35's provenance but not its behavior. Its closure was:
— it read neither the settings.json env block nor os.Getenv, so two of its four subtests were tautological:
"state file wins over settings.json env block" built env := envBlock(settingsDoc) and never passed it to resolve.
"state file wins over process env var" called t.Setenv(...); the closure never called os.Getenv.
Both passed by absence of code and were structurally incapable of failing. The test was a fossil mirroring an implementation that had since moved into buildClaudeLaunchPlan (via fa659ae/#40). It was deleted in #216 rather than retargeted, because retargeting it onto loadState() would still not read env — yielding a duplicate of state_test.go:9-23 plus two tests that cannot fail.
So #35's guarantee was already untested before #216, and is untested now. This issue closes that gap for real.
Scope
Extract resolveProfile from buildClaudeLaunchPlan as a pure function taking the flag value, the loaded state, and the settings.json env block — returning the resolved profile. The resolution chain is: --profile flag > state file > "DEFAULT", with the env block and DATABRICKS_CONFIG_PROFILE deliberately not consulted (that's the fix: remove env var from profile resolution — state file always wins #35 guarantee).
Add a test that genuinely exercises it: pass a non-empty env block and set DATABRICKS_CONFIG_PROFILE via t.Setenv, then assert the state file still wins. Unlike the deleted test, the inputs must actually reach the function under test.
Mirror the existing pure-resolver pattern (resolveConfigOTEL / resolveConfigWebSearch in config.go, diffModelRouting in doctor.go) — load-then-resolve, testable in isolation.
#216 was a state-persistence cleanup (pure deletion onto an already-tested generic). This is a production refactor of the auth-adjacent launch path. Both the Architect and Critic passes on #216 flagged bundling them as scope creep that would sprawl into buildClaudeLaunchPlan. Kept separate deliberately.
Notes
May fall out naturally of #219 (table-driven parseArgs core in internal/core/cli), which is already scoped to touch arg parsing and resolution. If #219 lands first, fold this in there and close this issue.
Summary
Extract the profile-resolution chain from
buildClaudeLaunchPlan(cmd/databricks-claude/launch_claude.go:75-79) into a pure, testableresolveProfilehelper, so the guarantee from #35 ("remove env var from profile resolution — state file always wins",e26f7ca) is actually covered by a test.Framing (read this first)
This is "make #35 testable", not "restore coverage deleted by #216". There is no coverage to restore.
#216's investigation found that
TestProfileResolution_StateFileWins(main_test.go:688-720) had #35's provenance but not its behavior. Its closure was:— it read neither the settings.json env block nor
os.Getenv, so two of its four subtests were tautological:env := envBlock(settingsDoc)and never passed it toresolve.t.Setenv(...); the closure never calledos.Getenv.Both passed by absence of code and were structurally incapable of failing. The test was a fossil mirroring an implementation that had since moved into
buildClaudeLaunchPlan(viafa659ae/#40). It was deleted in #216 rather than retargeted, because retargeting it ontoloadState()would still not read env — yielding a duplicate ofstate_test.go:9-23plus two tests that cannot fail.So #35's guarantee was already untested before #216, and is untested now. This issue closes that gap for real.
Scope
resolveProfilefrombuildClaudeLaunchPlanas a pure function taking the flag value, the loaded state, and the settings.json env block — returning the resolved profile. The resolution chain is:--profileflag > state file >"DEFAULT", with the env block andDATABRICKS_CONFIG_PROFILEdeliberately not consulted (that's the fix: remove env var from profile resolution — state file always wins #35 guarantee).DATABRICKS_CONFIG_PROFILEviat.Setenv, then assert the state file still wins. Unlike the deleted test, the inputs must actually reach the function under test.resolveConfigOTEL/resolveConfigWebSearchinconfig.go,diffModelRoutingindoctor.go) — load-then-resolve, testable in isolation.Why not in #216
#216 was a state-persistence cleanup (pure deletion onto an already-tested generic). This is a production refactor of the auth-adjacent launch path. Both the Architect and Critic passes on #216 flagged bundling them as scope creep that would sprawl into
buildClaudeLaunchPlan. Kept separate deliberately.Notes
May fall out naturally of #219 (table-driven
parseArgscore ininternal/core/cli), which is already scoped to touch arg parsing and resolution. If #219 lands first, fold this in there and close this issue.Follow-up to #216. Part of #196.