Context
Follow-up from multi-agent review of #164/#165/#167/#168 cleanup on branch fix/cli-chat-cleanup.
TestSnapshotChatOptions verifies the global -> snapshot isolation invariant (post-snapshot mutation of chatModel does not leak into the snapshot). The symmetric invariant — that in-session /model switches mutate the local opts.Model and NOT the package-level chatModel — is the load-bearing contract of #164 but is currently unverified by tests.
Why deferred
Testing handleCommand directly requires instantiating a real *coordinator.Coordinator, which performs provider setup. That would need a mock coordinator abstraction, which is a larger refactor than the cleanup PR warranted.
Proposal
Option A: Introduce a small coordinatorSetter interface in cmd/aixgo/cmd that *coordinator.Coordinator satisfies (just SetModel(string) error + ClearHistory()), then have handleCommand accept the interface. A fake can be stubbed for the test.
Option B: Extract the /model branch of handleCommand into a pure helper that takes (opts *chatOptions, sess *session.Session, newModel string, setter func(string) error) and write a direct unit test on the helper.
Either approach is cheap once chosen; flag for bikeshedding.
Acceptance
Labels
cli, test, follow-up
Context
Follow-up from multi-agent review of #164/#165/#167/#168 cleanup on branch
fix/cli-chat-cleanup.TestSnapshotChatOptionsverifies the global -> snapshot isolation invariant (post-snapshot mutation ofchatModeldoes not leak into the snapshot). The symmetric invariant — that in-session/modelswitches mutate the localopts.Modeland NOT the package-levelchatModel— is the load-bearing contract of #164 but is currently unverified by tests.Why deferred
Testing
handleCommanddirectly requires instantiating a real*coordinator.Coordinator, which performs provider setup. That would need a mock coordinator abstraction, which is a larger refactor than the cleanup PR warranted.Proposal
Option A: Introduce a small
coordinatorSetterinterface in cmd/aixgo/cmd that*coordinator.Coordinatorsatisfies (justSetModel(string) error+ClearHistory()), then havehandleCommandaccept the interface. A fake can be stubbed for the test.Option B: Extract the
/modelbranch ofhandleCommandinto a pure helper that takes(opts *chatOptions, sess *session.Session, newModel string, setter func(string) error)and write a direct unit test on the helper.Either approach is cheap once chosen; flag for bikeshedding.
Acceptance
chatModel = newModelin the /model handler-racewithout new globalsLabels
cli, test, follow-up