|
138 | 138 | {"id":"mcs-e7j","title":"Refactor: Consider introducing Duration type for confirmation timeouts","description":"## Summary\nConfirmation timeouts are passed as int (seconds) and converted to time.Duration. A proper Duration type could improve clarity.\n\n## Proposed Solution\nConsider using time.Duration directly or a custom ConfirmTimeout type.\n\n## Files to Modify\n- `internal/cli/confirm.go`\n- Command files using confirmation\n\n## Development Guidelines\n- **TDD**: Write tests first, then implementation\n- **DTSTTCPW**: Do the simplest thing that could possibly work\n- **DRY**: Don't repeat yourself\n- **No lint ignores**: All code must pass `golangci-lint run` without ignores\n- **No failing tests**: All tests must pass before committing\n- **Small frequent commits**: Commit after each logical change\n- **Attention to detail and high quality**: Clean code, proper error handling, good naming","status":"closed","priority":3,"issue_type":"task","created_at":"2025-12-19T13:41:32.085167-08:00","updated_at":"2025-12-19T16:59:55.644152-08:00","closed_at":"2025-12-19T16:59:55.644152-08:00","close_reason":"Not needed - current int seconds pattern is standard for CLI tools","comments":[{"id":7,"issue_id":"mcs-e7j","author":"carlosvillela","text":"Decision: Not implementing - current pattern is optimal\n\nAfter thorough analysis, the current implementation is the right approach:\n\n**Current Pattern:**\n- Commands accept int (seconds) via --confirm-wait flag\n- Conversion to time.Duration happens in one place: executeConfirmableCommand line 383\n- Users type: mcs lock --confirm-wait 90\n\n**Why Not to Change:**\n1. UX: Requiring '90s' instead of '90' is less intuitive for most users\n2. Industry standard: kubectl, docker, curl all use plain int seconds for timeouts\n3. No duplication: Conversion happens in exactly one place (DRY principle satisfied)\n4. Type safety: Internal functions already use time.Duration for type safety\n5. ParseDuration requires units: time.ParseDuration('90') fails with 'missing unit' error\n\n**Evidence:**\n- pflag supports DurationVar BUT requires unit suffixes (90s, 1m30s)\n- This would make the CLI less user-friendly for the common case of simple timeouts\n\n**Conclusion:**\nThe current pattern of accepting int seconds from CLI flags and converting to time.Duration internally is the standard approach for CLI tools. No change needed.","created_at":"2025-12-20T00:59:11Z"}]} |
139 | 139 | {"id":"mcs-ehua","title":"Silent config file read error ignored - config.go swallows v.ReadInConfig() error","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-19T23:13:04.738857-08:00","updated_at":"2025-12-19T23:26:08.041395-08:00","closed_at":"2025-12-19T23:26:08.041395-08:00","close_reason":"Closed"} |
140 | 140 | {"id":"mcs-ej7","title":"Add Example field to root 'mcs' command","description":"Add Cobra Example field to root command showing common workflows: checking status, locking doors, etc. Document region codes (MNAO=North America, MME=Europe, MJO=Japan/Oceania).","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-19T08:15:10.603533-08:00","updated_at":"2025-12-19T09:27:54.917108-08:00","closed_at":"2025-12-19T09:27:54.917108-08:00","close_reason":"Added comprehensive Example field with status, lock, start, charge, and climate examples","dependencies":[{"issue_id":"mcs-ej7","depends_on_id":"mcs-kd7","type":"blocks","created_at":"2025-12-19T08:15:17.923559-08:00","created_by":"daemon"}]} |
141 | | -{"id":"mcs-eprl","title":"Command confirmation polling times out even when command succeeds","status":"open","priority":2,"issue_type":"bug","created_at":"2025-12-21T01:34:30.256577-08:00","updated_at":"2025-12-21T01:34:30.256577-08:00","comments":[{"id":11,"issue_id":"mcs-eprl","author":"carlosvillela","text":"## Bug Description\n\nCommands like `mcs unlock` and `mcs climate on` execute successfully (the vehicle responds correctly), but the confirmation polling times out after 90 seconds.\n\n## Steps to Reproduce\n\n1. Run `mcs unlock` or `mcs climate on`\n2. Observe the command succeeds (vehicle doors unlock / climate turns on)\n3. CLI polls for confirmation but times out\n\n## Expected Behavior\n\nConfirmation should succeed when the vehicle action completes.\n\n## Actual Behavior\n\n```\nClimate on command sent, waiting for confirmation...\nWaiting for confirmation... (5s/90s)\n...\nWaiting for confirmation... (90s/90s)\nWarning: HVAC on not confirmed within timeout period\nClimate on command sent (confirmation timeout)\n```\n\n## Possible Causes\n\n- Confirmation API endpoint returning unexpected status\n- Polling logic not recognizing success response\n- API response format changed","created_at":"2025-12-21T09:34:38Z"}]} |
| 141 | +{"id":"mcs-eprl","title":"Command confirmation polling times out even when command succeeds","status":"closed","priority":2,"issue_type":"bug","created_at":"2025-12-21T01:34:30.256577-08:00","updated_at":"2025-12-21T01:57:45.8004-08:00","closed_at":"2025-12-21T01:57:45.8004-08:00","close_reason":"Fixed in commits 495f021, 379dd42, effefca","comments":[{"id":11,"issue_id":"mcs-eprl","author":"carlosvillela","text":"## Bug Description\n\nCommands like `mcs unlock` and `mcs climate on` execute successfully (the vehicle responds correctly), but the confirmation polling times out after 90 seconds.\n\n## Steps to Reproduce\n\n1. Run `mcs unlock` or `mcs climate on`\n2. Observe the command succeeds (vehicle doors unlock / climate turns on)\n3. CLI polls for confirmation but times out\n\n## Expected Behavior\n\nConfirmation should succeed when the vehicle action completes.\n\n## Actual Behavior\n\n```\nClimate on command sent, waiting for confirmation...\nWaiting for confirmation... (5s/90s)\n...\nWaiting for confirmation... (90s/90s)\nWarning: HVAC on not confirmed within timeout period\nClimate on command sent (confirmation timeout)\n```\n\n## Possible Causes\n\n- Confirmation API endpoint returning unexpected status\n- Polling logic not recognizing success response\n- API response format changed","created_at":"2025-12-21T09:34:38Z"}]} |
142 | 142 | {"id":"mcs-ewrg","title":"Error wrapping may lose underlying error context and cause debugging difficulties","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-19T23:13:14.417535-08:00","updated_at":"2025-12-20T16:21:39.895679-08:00","closed_at":"2025-12-20T16:21:39.895679-08:00","close_reason":"Vague ticket with no specific examples. All error wrapping in the codebase uses fmt.Errorf with %w which preserves the error chain. errors.Is and errors.As work correctly."} |
143 | 143 | {"id":"mcs-f43g","title":"Debug logging printed directly to stdout may corrupt CLI output","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-19T23:13:09.038123-08:00","updated_at":"2025-12-20T12:48:01.393288-08:00","closed_at":"2025-12-20T12:48:01.393288-08:00","close_reason":"Closed"} |
144 | 144 | {"id":"mcs-fkm","title":"Add Example field to 'mcs charge' commands","description":"Add Cobra Example field to charge start and charge stop. Show success output examples.","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-19T08:15:09.567753-08:00","updated_at":"2025-12-19T09:35:40.366658-08:00","closed_at":"2025-12-19T09:35:40.366658-08:00","close_reason":"Added Example fields to charge parent command and start/stop subcommands","dependencies":[{"issue_id":"mcs-fkm","depends_on_id":"mcs-kd7","type":"blocks","created_at":"2025-12-19T08:15:17.324114-08:00","created_by":"daemon"}]} |
| 145 | +{"id":"mcs-fy02","title":"Eliminate real-time sleeps from tests","status":"open","priority":2,"issue_type":"task","created_at":"2025-12-21T01:55:28.970636-08:00","updated_at":"2025-12-21T01:55:28.970636-08:00","comments":[{"id":12,"issue_id":"mcs-fy02","author":"carlosvillela","text":"## Problem\n\nTests are sleeping real wall-clock time, making the build unacceptably slow. Current test suite takes ~60s when it should take \u003c2s.\n\n### Worst Offenders\n\n| Test | Time | Location |\n|------|------|----------|\n| `TestAPIRequest_MaxRetries` | **23s** | `internal/api/client_integration_test.go` |\n| `TestConfirmationFlow_*` | 5-6s each | `internal/cli/confirm_integration_test.go` |\n| `TestWaitFor*` (8 tests) | 5s each | `internal/cli/confirm_test.go` |\n| `TestAPIRequest_*Retry*` | 1s each | `internal/api/client_integration_test.go` |\n\n**Total wasted time: ~70 seconds**\n\n### Root Causes\n\n1. **API retry tests** use real `sleepWithContext()` with exponential backoff (1s, 2s, 4s, 8s)\n2. **Confirmation tests** use `5*time.Second` timeout for \"never matches\" scenarios\n3. No time abstraction - tests call real `time.Sleep` via `sleepWithContext()`\n\n## Solution\n\n### 1. Inject sleep function into Client (for API retry tests)\n\n```go\n// In client.go\ntype Client struct {\n // ... existing fields\n sleepFunc func(context.Context, time.Duration) error // defaults to sleepWithContext\n}\n\n// Allow tests to override\nfunc (c *Client) SetSleepFunc(f func(context.Context, time.Duration) error) {\n c.sleepFunc = f\n}\n```\n\nIn tests:\n```go\nclient.SetSleepFunc(func(ctx context.Context, d time.Duration) error {\n return nil // instant \"sleep\"\n})\n```\n\n### 2. Reduce timeouts for \"never matches\" test cases\n\nIn `confirm_test.go`, change tests like `TestWaitForDoorsLocked` from:\n```go\n// Current: waits full 5s when doors never lock\nresult := waitForDoorsLocked(ctx, \u0026buf, mockClient, vin, 5*time.Second, 50*time.Millisecond)\n```\n\nTo:\n```go\n// Fixed: 200ms timeout for \"never matches\" cases\ntimeout := 5*time.Second\nif tt.name == \"doors never lock\" {\n timeout = 200*time.Millisecond\n}\nresult := waitForDoorsLocked(ctx, \u0026buf, mockClient, vin, timeout, 50*time.Millisecond)\n```\n\n### 3. Use instant polling for confirmation integration tests\n\nPass shorter timeouts/intervals to integration tests, or inject a mock clock.\n\n## Acceptance Criteria\n\n- [ ] `go test ./...` completes in \u003c5 seconds\n- [ ] No test uses real sleeps \u003e100ms\n- [ ] Test coverage unchanged\n- [ ] Tests still catch timing-related bugs (context cancellation, timeout behavior)\n\n## Files to Modify\n\n- `internal/api/client.go` - Add sleepFunc field\n- `internal/api/client_integration_test.go` - Inject no-op sleep\n- `internal/cli/confirm_test.go` - Reduce timeouts for \"never matches\" cases\n- `internal/cli/confirm_integration_test.go` - Reduce timeouts","created_at":"2025-12-21T09:55:49Z"}]} |
145 | 146 | {"id":"mcs-g57","title":"Add table-driven tests for simple CLI commands","description":"lock_test.go, engine_test.go have separate functions for lock/unlock and start/stop tests when they follow identical patterns. Consolidate into single table-driven test.","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-19T08:10:34.734673-08:00","updated_at":"2025-12-19T08:46:42.304897-08:00","closed_at":"2025-12-19T08:46:42.304897-08:00","close_reason":"Created table-driven simple_commands_test.go, consolidated 4 separate test files","dependencies":[{"issue_id":"mcs-g57","depends_on_id":"mcs-qcz","type":"blocks","created_at":"2025-12-19T08:11:00.597061-08:00","created_by":"daemon"}]} |
146 | 147 | {"id":"mcs-ghp7","title":"Potential race condition in client reuse","description":"The Client struct appears to be reused across goroutines without synchronization, which may cause race conditions when accessing shared fields like accessToken or encryption keys","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-19T23:14:43.990401-08:00","updated_at":"2025-12-20T16:23:38.628801-08:00","closed_at":"2025-12-20T16:23:38.628801-08:00","close_reason":"Not applicable. This is a CLI tool that runs sequentially. No goroutines share the Client. The background goroutine was removed in mcs-2mrp. Test goroutines are properly isolated."} |
147 | 148 | {"id":"mcs-gu8y","title":"Missing timeout on HTTP requests may cause hanging","description":"HTTP requests made by the client do not set a timeout, which can lead to indefinite blocking on slow or unresponsive endpoints","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-19T23:14:31.011286-08:00","updated_at":"2025-12-20T16:20:03.875024-08:00","closed_at":"2025-12-20T16:20:03.875024-08:00","close_reason":"Already addressed: httpClient has 30s timeout, auth functions use AuthRequestTimeout (15s) added in mcs-mnhn"} |
|
167 | 168 | {"id":"mcs-ngg","title":"CLAUDE.md: Project Structure section is missing files","description":"## Problem\n\nThe Project Structure section in CLAUDE.md is incomplete. It lists files that exist but is missing several files that have been added to the codebase.\n\n### Missing files in internal/cli/:\n- color.go - Color/formatting utilities for terminal output\n- confirm.go - Confirmation polling logic for commands\n\n### Missing files in internal/sensordata/:\n- key_event.go - Key event generation\n- background_event.go - Background event generation \n- touch_event.go - Touch event generation\n- system_info.go - System info generation\n- performance.go - Performance test result generation\n\nThe documented structure shows only 'sensor_data.go' for sensordata, but there are 5 additional files.\n\n### command_factory.go listed but empty\nThe file is listed in CLAUDE.md as 'Command builder helpers' but the actual file is essentially empty (just package declaration).\n\n## Location\n- /Users/carlosvillela/src/dummy/mysmartcar/mcs/CLAUDE.md (lines 5-35)\n\n## How to Fix\nEither:\n1. Add the missing files to the structure with brief descriptions, OR\n2. Keep the structure high-level and note it's not exhaustive\n\n## Development Guidelines\n- TDD, DTSTTCPW, DRY\n- No lint ignores, no failing tests\n- Small commits, quality focus","status":"closed","priority":3,"issue_type":"task","created_at":"2025-12-19T17:47:40.226771-08:00","updated_at":"2025-12-20T16:39:13.115294-08:00","closed_at":"2025-12-20T16:39:13.115294-08:00","close_reason":"Fixed in docs commit a4d70e1"} |
168 | 169 | {"id":"mcs-olye","title":"Token cache Load returns nil without error, potential nil dereference","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-19T23:13:10.376479-08:00","updated_at":"2025-12-20T12:49:03.038724-08:00","closed_at":"2025-12-20T12:49:03.038724-08:00","close_reason":"Closed"} |
169 | 170 | {"id":"mcs-otp","title":"Add Example field to 'mcs start' and 'mcs stop' commands","description":"Add Cobra Example field to engine start and stop commands. Show success output examples.","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-19T08:15:10.141073-08:00","updated_at":"2025-12-19T09:33:30.362651-08:00","closed_at":"2025-12-19T09:33:30.362651-08:00","close_reason":"Added Example fields to start and stop commands","dependencies":[{"issue_id":"mcs-otp","depends_on_id":"mcs-kd7","type":"blocks","created_at":"2025-12-19T08:15:17.684345-08:00","created_by":"daemon"}]} |
| 171 | +{"id":"mcs-p3a5","title":"Confirmation polling should refresh vehicle status before polling","status":"open","priority":1,"issue_type":"bug","created_at":"2025-12-21T02:07:52.678903-08:00","updated_at":"2025-12-21T02:07:52.678903-08:00","comments":[{"id":13,"issue_id":"mcs-p3a5","author":"carlosvillela","text":"## Root Cause\n\nConfirmation polling reads stale cached data from the server. The status command calls `RefreshVehicleStatus` (activeRealTimeVehicleStatus endpoint) before fetching status, but confirmation polling doesn't.\n\n## Evidence\n\n- `mcs unlock` succeeds (doors physically unlock)\n- `mcs raw status` shows `LockLinkSw*: 0` (locked) - stale data\n- Status command (`internal/cli/status_cmd.go:265`) calls `RefreshVehicleStatus` first\n- Confirmation polling (`internal/cli/confirm.go`) does NOT call refresh\n\n## Fix\n\nCall `RefreshVehicleStatus` at the start of confirmation polling to wake the vehicle and get fresh data. May also need to call it periodically during polling if status remains stale.\n\n## Files\n\n- `internal/cli/confirm.go` - Add RefreshVehicleStatus call\n- `internal/api/control.go:114-116` - RefreshVehicleStatus function","created_at":"2025-12-21T10:08:01Z"}]} |
170 | 172 | {"id":"mcs-qcz","title":"Create shared CLI test helpers for command basics","description":"lock_test.go, engine_test.go, charge_test.go, climate_test.go all have identical test patterns for Use field and Short description. Create testCommandBasics helper.","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-19T08:10:06.665551-08:00","updated_at":"2025-12-19T08:45:12.492118-08:00","closed_at":"2025-12-19T08:45:12.492118-08:00","close_reason":"Created assertCommandBasics and assertNoArgsCommand helpers, refactored 4 test files, eliminated ~40 lines"} |
171 | 173 | {"id":"mcs-rmk","title":"Add backoff delay with context check in API retry logic","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-19T02:15:20.198613-08:00","updated_at":"2025-12-19T07:47:40.370862-08:00","closed_at":"2025-12-19T07:47:40.370862-08:00","close_reason":"Closed","comments":[{"id":4,"issue_id":"mcs-rmk","author":"carlosvillela","text":"The retry logic in internal/api/client.go:50-71 retries immediately without any backoff delay. If auth fails, it hammers the API repeatedly. Should add exponential backoff with context cancellation check during the sleep.\n\nFiles: internal/api/client.go:50-71","created_at":"2025-12-19T10:15:37Z"}]} |
172 | 174 | {"id":"mcs-sfjx","title":"Fix post-v0.5.1 quality issues","status":"closed","priority":1,"issue_type":"epic","created_at":"2025-12-20T13:46:18.189242-08:00","updated_at":"2025-12-20T16:08:46.282415-08:00","closed_at":"2025-12-20T16:08:46.282415-08:00","close_reason":"All subtasks completed: build fixed, goroutine removed, test verified, timeout constant extracted.","dependencies":[{"issue_id":"mcs-sfjx","depends_on_id":"mcs-u32x","type":"blocks","created_at":"2025-12-20T13:46:45.917354-08:00","created_by":"daemon"},{"issue_id":"mcs-sfjx","depends_on_id":"mcs-2mrp","type":"blocks","created_at":"2025-12-20T13:46:46.036475-08:00","created_by":"daemon"},{"issue_id":"mcs-sfjx","depends_on_id":"mcs-mnhn","type":"blocks","created_at":"2025-12-20T13:46:46.153929-08:00","created_by":"daemon"},{"issue_id":"mcs-sfjx","depends_on_id":"mcs-7ob6","type":"blocks","created_at":"2025-12-20T13:46:46.272032-08:00","created_by":"daemon"}]} |
|
0 commit comments