Skip to content

Commit 4624c45

Browse files
fix: address all remaining CodeRabbit review comments
- cmd/auth.go: use term.ReadPassword to hide API key input; handle ConfigFile() error gracefully in user-facing output - cmd/run.go: open isolated DB connection for background Prune() goroutine to avoid data race with deferred store.Close(); honour --fallback flag in runWithoutCache on zip/API/render failures - cmd/status.go: capture and handle store.Get() errors at both call sites; fix dry-run Long description (it does write to stdout) - hooks/hooks.json: split SessionStart into startup/clear matchers; add SessionEnd with clear matcher for context-reinjection after /clear - internal/api/client.go: check HTTP status before unmarshalling poll response — fail fast on 401/404, retry on 429/5xx - internal/config/config.go: return error for non-NotExist ReadFile failures instead of silently ignoring them - internal/hooks/hooks.go: broaden isAlreadyInstalled to detect uncompact-hook.sh wrapper form (idempotent across install methods); fail fast on invalid settings.json JSON instead of silently recreating; use mode 0600 when writing settings.json - internal/template/render.go: use time.Now().UTC() so timestamp label matches actual value; guard truncateToTokenBudget against required header exceeding token budget - internal/zip/zip.go: skip symlinks to prevent including files outside repo root - go.mod: add golang.org/x/term v0.22.0 as direct dependency Run 'go mod tidy' before building to regenerate go.sum. Co-authored-by: Grey Newell <greynewell@users.noreply.github.com>
1 parent 598c8ab commit 4624c45

File tree

10 files changed

+112
-21
lines changed

10 files changed

+112
-21
lines changed

cmd/auth.go

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package cmd
22

33
import (
4-
"bufio"
54
"context"
65
"fmt"
76
"os"
@@ -11,6 +10,7 @@ import (
1110
"github.com/spf13/cobra"
1211
"github.com/supermodeltools/uncompact/internal/api"
1312
"github.com/supermodeltools/uncompact/internal/config"
13+
"golang.org/x/term"
1414
)
1515

1616
var authCmd = &cobra.Command{
@@ -54,14 +54,22 @@ func authLoginHandler(cmd *cobra.Command, args []string) error {
5454
fmt.Println()
5555
fmt.Print("2. Paste your API key here: ")
5656

57-
scanner := bufio.NewScanner(os.Stdin)
58-
if !scanner.Scan() {
59-
if err := scanner.Err(); err != nil {
57+
var key string
58+
if term.IsTerminal(int(os.Stdin.Fd())) {
59+
b, err := term.ReadPassword(int(os.Stdin.Fd()))
60+
fmt.Println()
61+
if err != nil {
62+
return fmt.Errorf("reading API key: %w", err)
63+
}
64+
key = strings.TrimSpace(string(b))
65+
} else {
66+
// Non-interactive fallback (e.g. piped input in CI)
67+
var raw string
68+
if _, err := fmt.Fscanln(os.Stdin, &raw); err != nil {
6069
return fmt.Errorf("reading API key: %w", err)
6170
}
62-
return fmt.Errorf("no input provided")
71+
key = strings.TrimSpace(raw)
6372
}
64-
key := strings.TrimSpace(scanner.Text())
6573
if key == "" {
6674
return fmt.Errorf("API key cannot be empty")
6775
}
@@ -89,8 +97,11 @@ func authLoginHandler(cmd *cobra.Command, args []string) error {
8997
return fmt.Errorf("saving config: %w", err)
9098
}
9199

92-
cfgFile, _ := config.ConfigFile()
93-
fmt.Printf("\nAPI key saved to: %s\n", cfgFile)
100+
if cfgFile, err := config.ConfigFile(); err == nil {
101+
fmt.Printf("\nAPI key saved to: %s\n", cfgFile)
102+
} else {
103+
fmt.Println("\nAPI key saved.")
104+
}
94105
fmt.Println()
95106
fmt.Println("Next: run 'uncompact install' to add hooks to Claude Code.")
96107
return nil

cmd/run.go

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,15 @@ func runHandler(cmd *cobra.Command, args []string) error {
7272
}
7373
defer store.Close()
7474

75-
// Background prune (non-blocking)
76-
go func() { _ = store.Prune() }()
75+
// Background prune on an isolated DB handle to avoid racing with store.Close().
76+
go func(path string) {
77+
pruneStore, err := cache.Open(path)
78+
if err != nil {
79+
return
80+
}
81+
defer pruneStore.Close()
82+
_ = pruneStore.Prune()
83+
}(dbPath)
7784

7885
// Check cache
7986
var graph *api.ProjectGraph
@@ -181,20 +188,29 @@ func runWithoutCache(cfg *config.Config, proj *project.Info, logFn func(string,
181188
zipData, err := zip.RepoZip(proj.RootDir)
182189
if err != nil {
183190
logFn("[warn] zip error: %v", err)
191+
if fallback {
192+
printFallback(proj.Name)
193+
}
184194
return silentExit()
185195
}
186196

187197
apiClient := api.New(cfg.BaseURL, cfg.APIKey, debug, logFn)
188198
graph, err := apiClient.GetGraph(ctx, proj.Name, zipData)
189199
if err != nil {
190200
logFn("[warn] API error: %v", err)
201+
if fallback {
202+
printFallback(proj.Name)
203+
}
191204
return silentExit()
192205
}
193206

194207
opts := tmpl.RenderOptions{MaxTokens: maxTokens}
195208
output, _, err := tmpl.Render(graph, proj.Name, opts)
196209
if err != nil {
197210
logFn("[warn] render error: %v", err)
211+
if fallback {
212+
printFallback(proj.Name)
213+
}
198214
return silentExit()
199215
}
200216

cmd/status.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ var statsCmd = &cobra.Command{
3737
var dryRunCmd = &cobra.Command{
3838
Use: "dry-run",
3939
Short: "Preview the context bomb without emitting it",
40-
Long: `dry-run shows what would be injected without writing to stdout. Useful for debugging.`,
40+
Long: `dry-run shows what would be injected without performing injection. Writes a preview to stdout for debugging.`,
4141
RunE: dryRunHandler,
4242
}
4343

@@ -122,7 +122,11 @@ func statusHandler(cmd *cobra.Command, args []string) error {
122122
}
123123

124124
// Cache freshness
125-
graph, fresh, _, _ := store.Get(proj.Hash)
125+
graph, fresh, _, err := store.Get(proj.Hash)
126+
if err != nil {
127+
fmt.Printf("Cache: error (%v)\n", err)
128+
return nil
129+
}
126130
if graph == nil {
127131
fmt.Println("Cache: empty")
128132
} else if fresh {
@@ -225,7 +229,10 @@ func dryRunHandler(cmd *cobra.Command, args []string) error {
225229
defer store.Close()
226230

227231
// Try cache first
228-
cachedGraph, fresh, _, _ := store.Get(proj.Hash)
232+
cachedGraph, fresh, _, err := store.Get(proj.Hash)
233+
if err != nil {
234+
return fmt.Errorf("reading cache: %w", err)
235+
}
229236

230237
if cachedGraph != nil && !forceRefresh {
231238
if !fresh {

go.mod

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ go 1.22
44

55
require (
66
github.com/spf13/cobra v1.8.1
7+
golang.org/x/term v0.22.0
78
modernc.org/sqlite v1.37.0
89
)
910

@@ -16,7 +17,7 @@ require (
1617
github.com/ncruces/go-strftime v0.1.9 // indirect
1718
github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect
1819
github.com/spf13/pflag v1.0.5 // indirect
19-
golang.org/x/sys v0.22.0 // indirect
20+
golang.org/x/sys v0.22.0
2021
modernc.org/gc/v3 v3.0.0-20240107210532-573471604cb6 // indirect
2122
modernc.org/libc v1.55.3 // indirect
2223
modernc.org/mathutil v1.6.0 // indirect

hooks/hooks.json

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,22 @@
22
"hooks": {
33
"SessionStart": [
44
{
5+
"matcher": "startup",
6+
"hooks": [
7+
{
8+
"type": "command",
9+
"command": "bash ${CLAUDE_PLUGIN_ROOT}/scripts/setup.sh",
10+
"timeout": 60
11+
},
12+
{
13+
"type": "command",
14+
"command": "bash ${CLAUDE_PLUGIN_ROOT}/scripts/pregen-hook.sh",
15+
"timeout": 10
16+
}
17+
]
18+
},
19+
{
20+
"matcher": "clear",
521
"hooks": [
622
{
723
"type": "command",
@@ -38,6 +54,18 @@
3854
}
3955
]
4056
}
57+
],
58+
"SessionEnd": [
59+
{
60+
"matcher": "clear",
61+
"hooks": [
62+
{
63+
"type": "command",
64+
"command": "bash ${CLAUDE_PLUGIN_ROOT}/scripts/uncompact-hook.sh",
65+
"timeout": 120
66+
}
67+
]
68+
}
4169
]
4270
}
4371
}

internal/api/client.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,20 @@ func (c *Client) pollJob(ctx context.Context, jobID string) (*ProjectGraph, erro
217217
continue
218218
}
219219

220+
// Check HTTP status before parsing — non-retriable errors fail fast.
221+
switch {
222+
case resp.StatusCode == http.StatusUnauthorized:
223+
return nil, fmt.Errorf("authentication failed during poll: check your API key at %s", config.DashboardURL)
224+
case resp.StatusCode == http.StatusNotFound:
225+
return nil, fmt.Errorf("job %s not found (status 404)", jobID)
226+
case resp.StatusCode == http.StatusTooManyRequests || resp.StatusCode >= 500:
227+
// Transient — continue retry loop
228+
c.logFn("[debug] poll transient error %d (will retry)", resp.StatusCode)
229+
continue
230+
case resp.StatusCode < 200 || resp.StatusCode >= 300:
231+
return nil, fmt.Errorf("poll returned unexpected status %d: %s", resp.StatusCode, string(body))
232+
}
233+
220234
var status JobStatus
221235
if err := json.Unmarshal(body, &status); err != nil {
222236
continue

internal/config/config.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,13 @@ func Load(flagAPIKey string) (*Config, error) {
118118
// Load from config file
119119
cfgFile, err := ConfigFile()
120120
if err == nil {
121-
if data, err := os.ReadFile(cfgFile); err == nil {
121+
data, readErr := os.ReadFile(cfgFile)
122+
if readErr == nil {
122123
if err := json.Unmarshal(data, cfg); err != nil {
123124
return nil, fmt.Errorf("malformed config file %s: %w", cfgFile, err)
124125
}
126+
} else if !os.IsNotExist(readErr) {
127+
return nil, fmt.Errorf("reading config file %s: %w", cfgFile, readErr)
125128
}
126129
}
127130

internal/hooks/hooks.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,7 @@ func Install(settingsPath string, dryRun bool) (*InstallResult, error) {
102102
var rawJSON map[string]json.RawMessage
103103
if data, err := os.ReadFile(settingsPath); err == nil {
104104
if err := json.Unmarshal(data, &rawJSON); err != nil {
105-
// File exists but is invalid JSON — warn and treat as empty
106-
fmt.Fprintf(os.Stderr, "Warning: existing settings.json has invalid JSON, will recreate hooks section\n")
107-
rawJSON = make(map[string]json.RawMessage)
105+
return nil, fmt.Errorf("invalid settings.json at %s: %w", settingsPath, err)
108106
}
109107
} else if !os.IsNotExist(err) {
110108
return nil, fmt.Errorf("reading settings.json: %w", err)
@@ -155,22 +153,26 @@ func Install(settingsPath string, dryRun bool) (*InstallResult, error) {
155153
if err != nil {
156154
return nil, err
157155
}
158-
if err := os.WriteFile(settingsPath, finalJSON, 0644); err != nil {
156+
if err := os.WriteFile(settingsPath, finalJSON, 0600); err != nil {
159157
return nil, fmt.Errorf("writing settings.json: %w", err)
160158
}
161159

162160
return result, nil
163161
}
164162

165163
// isAlreadyInstalled checks if uncompact hooks are already present.
164+
// Recognises both the direct "uncompact run" form and the wrapper-script form
165+
// used by the plugin (uncompact-hook.sh) so re-running install is idempotent
166+
// regardless of which installation method was used.
166167
func isAlreadyInstalled(hooks map[string][]Hook) bool {
167168
stopHooks, ok := hooks["Stop"]
168169
if !ok {
169170
return false
170171
}
171172
for _, h := range stopHooks {
172173
for _, cmd := range h.Hooks {
173-
if strings.Contains(cmd.Command, "uncompact run") {
174+
if strings.Contains(cmd.Command, "uncompact run") ||
175+
strings.Contains(cmd.Command, "uncompact-hook.sh") {
174176
return true
175177
}
176178
}

internal/template/render.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func Render(graph *api.ProjectGraph, projectName string, opts RenderOptions) (st
5252
opts.MaxTokens = 2000
5353
}
5454

55-
now := time.Now()
55+
now := time.Now().UTC()
5656

5757
data := struct {
5858
ProjectName string
@@ -119,6 +119,10 @@ func truncateToTokenBudget(
119119
)
120120

121121
reqTokens := countTokens(required)
122+
if reqTokens > maxTokens {
123+
msg := "# Uncompact Context\n\n(Budget too small; increase --max-tokens)"
124+
return msg, countTokens(msg), nil
125+
}
122126
remaining := maxTokens - reqTokens
123127

124128
var domainSections []string

internal/zip/zip.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,11 @@ func RepoZip(root string) ([]byte, error) {
7474
return nil
7575
}
7676

77+
// Skip symlinks to avoid including files outside the repo root.
78+
if info.Mode()&os.ModeSymlink != 0 {
79+
return nil
80+
}
81+
7782
// Skip by extension
7883
ext := strings.ToLower(filepath.Ext(path))
7984
if skipExts[ext] {

0 commit comments

Comments
 (0)