feat(model): report storage engine in tracking metadata#285
Conversation
Follow-up to #284: the server now wants to know which local storage engine buffered each synced command. Add Engine() to the CommandStore interface (file/bolt) and attach it as cliEngine to TrackingMetaData in BuildTrackingData, so every send path — daemon bolt handler, CLI file fallback, and the legacy CLI->daemon socket sync — reports the engine that actually held the data. https://claude.ai/code/session_015ZzRC212TzpPdHwwcxKohA
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces tracking of the CLI storage engine (either File or Bolt) used to buffer commands by adding an Engine() method to the CommandStore interface and including this metadata in the payload sent to the server. The review feedback suggests two improvements to the test suite: avoiding platform-dependent environment variables by passing t.TempDir() directly to InitFolder, and explicitly asserting that JSON decoding succeeds in the test HTTP server rather than ignoring the error.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| t.Setenv("HOME", t.TempDir()) | ||
| InitFolder("") // reset globals to the default .shelltime under the temp HOME |
There was a problem hiding this comment.
Hardcoding the 'HOME' environment variable is platform-dependent and violates the general rule of avoiding environment variables like '$HOME' for path resolution. It will fail on Windows where 'os.UserHomeDir()' looks up 'USERPROFILE'. Since 'InitFolder' accepts a custom path, pass 't.TempDir()' directly to it to ensure platform independence.
| t.Setenv("HOME", t.TempDir()) | |
| InitFolder("") // reset globals to the default .shelltime under the temp HOME | |
| InitFolder(t.TempDir()) |
References
- For platform-independent paths, use filepath.Join to combine segments and os.UserHomeDir() to get the home directory, rather than hardcoding path separators or environment variables like $HOME.
| func (s *TrackHandlerTestSuite) TestTrackPostFlushSyncsAndPrunes() { | ||
| var sentPayload model.PostTrackArgs | ||
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| _ = json.NewDecoder(r.Body).Decode(&sentPayload) |
There was a problem hiding this comment.
Ignoring the error from 'json.NewDecoder().Decode()' can lead to silent test failures or confusing assertion errors if the request body is malformed or empty. It is better to explicitly assert that decoding succeeded.
| _ = json.NewDecoder(r.Body).Decode(&sentPayload) | |
| err := json.NewDecoder(r.Body).Decode(&sentPayload) | |
| assert.NoError(s.T(), err) |
2 participants
Why
Follow-up to #284: the server (shelltime/server#451) now records which local storage engine buffered each synced command, so web/iOS can display it and suggest switching to bolt.
What
CommandStoregainsEngine() string(StorageEngineFile/StorageEngineBolt), implemented by both storesTrackingMetaDatagainscliEngine(omitempty), populated inBuildTrackingDatafrom the store the payload was actually read fromSetting it at payload-assembly time means all three send paths report the truth without further changes:
daemon/handlers.track.go)commands/track.go)PostTrackArgs;sendTrackArgsToServerdoesn't touch it)A config that says
boltwhile the daemon is down still correctly reportsfile, because the engine comes from the store, not the config.Verification
go vet ./...clean;go test ./model/ ./commands/ ./daemon/passBuildTrackingDatameta reportsboltfor the bolt store andfilefor the file store; the daemon flush test now decodes the HTTP body and assertsmeta.cliEngine == "bolt"andsource == 1TestGitInfoTestSuitefails in this sandbox environment, same as onmain(documented in feat: bbolt-backed command store (daemon-owned, feature-gated) #284)Companion PRs: shelltime/server#451, shelltime/web, shelltime/ios.
https://claude.ai/code/session_015ZzRC212TzpPdHwwcxKohA
Generated by Claude Code