Skip to content

Commit 0ed4e4a

Browse files
aksOpsclaude
aksOps
and
claude
committed
refactor(claude): extract shared atomic JSON-patch helper
Both EnsureRemoteControlAtStartup and EnsureTUIFullscreen duplicated ~75% of their bodies (stat, read, unmarshal, marshal, temp+rename). Extract to patchJSONFile + atomicWriteFile in internal/claude/ jsonpatch.go so each caller collapses to a ~10-line closure. Future fixes (e.g., fsync before rename, better error handling) now land in one place. Also: - Clarify in both function doc comments that JSON null is treated as "key present" = explicit user choice = leave alone. The previous prose was ambiguous. - Add TestEnsureTUIFullscreen_IdempotentFromDefault covering the "default" → "fullscreen" upgrade path. The existing Idempotent test only exercised the absent-key case. No behavior change. Full test suite passes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 9b44a25 commit 0ed4e4a

4 files changed

Lines changed: 166 additions & 134 deletions

File tree

‎internal/claude/jsonpatch.go‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
package claude
2+
3+
import (
4+
"encoding/json"
5+
"fmt"
6+
"os"
7+
"path/filepath"
8+
)
9+
10+
// patchJSONFile reads path, applies patch to the top-level JSON object, and
11+
// writes the result back atomically. Used to safely flip single keys inside
12+
// JSON files owned by other tools (Claude Code's ~/.claude.json and
13+
// ~/.claude/settings.json) without clobbering sibling keys.
14+
//
15+
// Contract:
16+
// - Missing file → no-op. Never creates it; the owning tool controls
17+
// lifecycle. Returns nil.
18+
// - Invalid JSON → returns a parse error without modifying the file.
19+
// - patch mutates the map in place and returns true to trigger a write,
20+
// false to skip. Returning false is a valid no-op.
21+
// - Writes are atomic (temp file in same dir + rename) and preserve the
22+
// original file mode.
23+
//
24+
// Concurrency caveat: if the owning tool writes to path between our Read and
25+
// Rename, that write is lost. Acceptable when this runs before the
26+
// competing writer launches (ctm bootstrap runs before `claude` starts).
27+
//
28+
// Key ordering: the map round-trip sorts keys alphabetically on marshal. The
29+
// file's semantics are preserved (JSON parsers ignore order) but visual
30+
// layout changes on the first write that mutates the object.
31+
func patchJSONFile(path string, patch func(obj map[string]json.RawMessage) bool) error {
32+
info, err := os.Stat(path)
33+
if err != nil {
34+
return nil
35+
}
36+
37+
data, err := os.ReadFile(path)
38+
if err != nil {
39+
return fmt.Errorf("reading %s: %w", path, err)
40+
}
41+
42+
var obj map[string]json.RawMessage
43+
if err := json.Unmarshal(data, &obj); err != nil {
44+
return fmt.Errorf("parsing %s: %w", path, err)
45+
}
46+
47+
if !patch(obj) {
48+
return nil
49+
}
50+
51+
out, err := json.MarshalIndent(obj, "", " ")
52+
if err != nil {
53+
return fmt.Errorf("marshalling %s: %w", path, err)
54+
}
55+
56+
return atomicWriteFile(path, out, info.Mode().Perm())
57+
}
58+
59+
// atomicWriteFile writes data to path via a temp file in the same directory
60+
// followed by rename(2), so readers never see a half-written file. The temp
61+
// file's mode is forced to perm before close to avoid the default 0600 from
62+
// os.CreateTemp overriding the caller's intent after rename.
63+
func atomicWriteFile(path string, data []byte, perm os.FileMode) error {
64+
dir := filepath.Dir(path)
65+
base := filepath.Base(path) + ".*"
66+
tmp, err := os.CreateTemp(dir, base)
67+
if err != nil {
68+
return fmt.Errorf("creating temp file: %w", err)
69+
}
70+
tmpPath := tmp.Name()
71+
defer os.Remove(tmpPath) //nolint:errcheck
72+
73+
if _, err := tmp.Write(data); err != nil {
74+
tmp.Close() //nolint:errcheck
75+
return fmt.Errorf("writing temp file: %w", err)
76+
}
77+
if err := tmp.Chmod(perm); err != nil {
78+
tmp.Close() //nolint:errcheck
79+
return fmt.Errorf("chmod temp file: %w", err)
80+
}
81+
if err := tmp.Close(); err != nil {
82+
return fmt.Errorf("closing temp file: %w", err)
83+
}
84+
if err := os.Rename(tmpPath, path); err != nil {
85+
return fmt.Errorf("renaming temp file: %w", err)
86+
}
87+
return nil
88+
}

‎internal/claude/remote_control.go‎

Lines changed: 21 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package claude
22

33
import (
44
"encoding/json"
5-
"fmt"
65
"os"
76
"path/filepath"
87
)
@@ -19,85 +18,34 @@ func ClaudeJSONPath() (string, error) {
1918
}
2019

2120
// EnsureRemoteControlAtStartup sets "remoteControlAtStartup": true in
22-
// ~/.claude.json so Claude Code's Remote Control feature is on by default for
23-
// any new Claude session — including those spawned by ctm on a freshly-
21+
// ~/.claude.json so Claude Code's Remote Control feature is on by default
22+
// for any new Claude session — including those spawned by ctm on a freshly-
2423
// bootstrapped machine.
2524
//
26-
// Semantics (strictly conservative — the file is Claude Code's state, not ours):
27-
// - If the file does not exist, do nothing. Never create it; Claude Code
28-
// owns the lifecycle of ~/.claude.json.
29-
// - If the key is already present (true OR false), do nothing. This
30-
// respects an explicit user choice made via `/config`, including
31-
// opt-out. "missing" and "false" are deliberately treated differently.
25+
// Semantics (strictly conservative — the file is Claude Code's state, not
26+
// ours):
27+
// - If the file does not exist, do nothing. Never create it.
28+
// - If the key is already present (true, false, or JSON null) we treat
29+
// that as a deliberate user choice and leave it alone. Only an absent
30+
// key triggers the write.
3231
// - Only when the key is absent do we write it true, preserving all other
33-
// keys byte-for-byte via json.RawMessage round-trip.
34-
// - Writes are atomic (temp file + rename) and preserve the original file
35-
// mode (typically 0600).
32+
// keys via json.RawMessage round-trip (values byte-exact; top-level
33+
// key order becomes alphabetical — see patchJSONFile).
3634
//
3735
// The key `remoteControlAtStartup` was discovered empirically by toggling
3836
// "Enable Remote Control for all sessions" in `/config` and diffing the
3937
// resulting JSON. It is not a documented/stable API; if Claude Code renames
40-
// it, future runs of this function silently no-op (the old key would be
41-
// absent, we'd add our own, Claude Code would ignore it — harmless).
38+
// it, future runs silently no-op (harmless).
4239
//
43-
// Concurrency caveat: if Claude Code writes to ~/.claude.json between our
44-
// Read and our Rename, that write is lost. In practice ctm bootstrap runs
45-
// before `claude` launches, so the race window is small and the impact
46-
// (losing one Claude Code-authored update) is acceptable for this best-
47-
// effort convenience feature. No flock to keep the bootstrap fast.
48-
//
49-
// Errors are returned so the caller can log; callers in ctm's boot path
50-
// should swallow — remote-control defaults are convenience, not correctness,
51-
// and must never block claude launch.
40+
// Errors are returned so callers can log; callers in ctm's boot path should
41+
// swallow — remote-control defaults are convenience, not correctness, and
42+
// must never block claude launch.
5243
func EnsureRemoteControlAtStartup(path string) error {
53-
info, err := os.Stat(path)
54-
if err != nil {
55-
return nil
56-
}
57-
58-
data, err := os.ReadFile(path)
59-
if err != nil {
60-
return fmt.Errorf("reading %s: %w", path, err)
61-
}
62-
63-
var obj map[string]json.RawMessage
64-
if err := json.Unmarshal(data, &obj); err != nil {
65-
return fmt.Errorf("parsing %s: %w", path, err)
66-
}
67-
68-
if _, present := obj["remoteControlAtStartup"]; present {
69-
return nil
70-
}
71-
72-
obj["remoteControlAtStartup"] = json.RawMessage("true")
73-
74-
out, err := json.MarshalIndent(obj, "", " ")
75-
if err != nil {
76-
return fmt.Errorf("marshalling %s: %w", path, err)
77-
}
78-
79-
dir := filepath.Dir(path)
80-
tmp, err := os.CreateTemp(dir, ".claude.json.*")
81-
if err != nil {
82-
return fmt.Errorf("creating temp file: %w", err)
83-
}
84-
tmpPath := tmp.Name()
85-
// best-effort cleanup if we bail before rename
86-
defer os.Remove(tmpPath) //nolint:errcheck
87-
88-
if _, err := tmp.Write(out); err != nil {
89-
tmp.Close() //nolint:errcheck
90-
return fmt.Errorf("writing temp file: %w", err)
91-
}
92-
if err := tmp.Chmod(info.Mode().Perm()); err != nil {
93-
tmp.Close() //nolint:errcheck
94-
return fmt.Errorf("chmod temp file: %w", err)
95-
}
96-
if err := tmp.Close(); err != nil {
97-
return fmt.Errorf("closing temp file: %w", err)
98-
}
99-
if err := os.Rename(tmpPath, path); err != nil {
100-
return fmt.Errorf("renaming temp file: %w", err)
101-
}
102-
return nil
44+
return patchJSONFile(path, func(obj map[string]json.RawMessage) bool {
45+
if _, present := obj["remoteControlAtStartup"]; present {
46+
return false
47+
}
48+
obj["remoteControlAtStartup"] = json.RawMessage("true")
49+
return true
50+
})
10351
}

‎internal/claude/tui.go‎

Lines changed: 17 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package claude
22

33
import (
44
"encoding/json"
5-
"fmt"
65
"os"
76
"path/filepath"
87
)
@@ -25,69 +24,26 @@ func SettingsJSONPath() (string, error) {
2524
//
2625
// Rationale: at the time of writing, Claude Code's "default" renderer IS the
2726
// fullscreen renderer — `/tui fullscreen` reports "Already using the
28-
// fullscreen renderer" when the setting is "default". Pinning to "fullscreen"
29-
// is a forward-looking hedge so new machines keep the fullscreen UI even if
30-
// Claude Code later redefines what "default" means.
27+
// fullscreen renderer" when the setting is "default". Pinning to
28+
// "fullscreen" is a forward-looking hedge so new machines keep the
29+
// fullscreen UI even if Claude Code later redefines what "default" means.
3130
//
32-
// Semantics (parallel to EnsureRemoteControlAtStartup):
33-
// - Missing settings.json → no-op. Never create it; Claude Code owns the
34-
// file's lifecycle.
31+
// Semantics:
32+
// - Missing settings.json → no-op. Never create it.
3533
// - Invalid JSON → return error; never clobber the user's file.
36-
// - Atomic write via temp + rename; preserves the original file mode.
37-
// - Concurrency caveat: a concurrent Claude Code write between our Read
38-
// and Rename is lost. Acceptable for a best-effort convenience default
39-
// that runs before `claude` launches.
34+
// - Absent key OR value == "default" → write "fullscreen".
35+
// - Any other value (including JSON null, "compact", or a custom mode)
36+
// is respected as an explicit user choice and left alone.
37+
// - Atomic write via temp + rename; preserves original file mode.
4038
//
4139
// Errors are returned so callers can log; ctm's boot path swallows them.
4240
func EnsureTUIFullscreen(path string) error {
43-
info, err := os.Stat(path)
44-
if err != nil {
45-
return nil
46-
}
47-
48-
data, err := os.ReadFile(path)
49-
if err != nil {
50-
return fmt.Errorf("reading %s: %w", path, err)
51-
}
52-
53-
var obj map[string]json.RawMessage
54-
if err := json.Unmarshal(data, &obj); err != nil {
55-
return fmt.Errorf("parsing %s: %w", path, err)
56-
}
57-
58-
cur, present := obj["tui"]
59-
if present && string(cur) != `"default"` {
60-
return nil
61-
}
62-
63-
obj["tui"] = json.RawMessage(`"fullscreen"`)
64-
65-
out, err := json.MarshalIndent(obj, "", " ")
66-
if err != nil {
67-
return fmt.Errorf("marshalling %s: %w", path, err)
68-
}
69-
70-
dir := filepath.Dir(path)
71-
tmp, err := os.CreateTemp(dir, "settings.json.*")
72-
if err != nil {
73-
return fmt.Errorf("creating temp file: %w", err)
74-
}
75-
tmpPath := tmp.Name()
76-
defer os.Remove(tmpPath) //nolint:errcheck
77-
78-
if _, err := tmp.Write(out); err != nil {
79-
tmp.Close() //nolint:errcheck
80-
return fmt.Errorf("writing temp file: %w", err)
81-
}
82-
if err := tmp.Chmod(info.Mode().Perm()); err != nil {
83-
tmp.Close() //nolint:errcheck
84-
return fmt.Errorf("chmod temp file: %w", err)
85-
}
86-
if err := tmp.Close(); err != nil {
87-
return fmt.Errorf("closing temp file: %w", err)
88-
}
89-
if err := os.Rename(tmpPath, path); err != nil {
90-
return fmt.Errorf("renaming temp file: %w", err)
91-
}
92-
return nil
41+
return patchJSONFile(path, func(obj map[string]json.RawMessage) bool {
42+
cur, present := obj["tui"]
43+
if present && string(cur) != `"default"` {
44+
return false
45+
}
46+
obj["tui"] = json.RawMessage(`"fullscreen"`)
47+
return true
48+
})
9349
}

‎internal/claude/tui_test.go‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,3 +180,43 @@ func TestEnsureTUIFullscreen_Idempotent(t *testing.T) {
180180
t.Errorf("second run modified file:\nfirst: %s\nsecond: %s", first, second)
181181
}
182182
}
183+
184+
// TestEnsureTUIFullscreen_IdempotentFromDefault locks in that the
185+
// "default" → "fullscreen" upgrade path is itself idempotent: run 1
186+
// upgrades, run 2 must be a no-op because the value is no longer "default".
187+
// The generic Idempotent test above only covers the absent-key path.
188+
func TestEnsureTUIFullscreen_IdempotentFromDefault(t *testing.T) {
189+
dir := t.TempDir()
190+
path := filepath.Join(dir, "settings.json")
191+
if err := os.WriteFile(path, []byte(`{"tui":"default","theme":"dark"}`), 0644); err != nil {
192+
t.Fatalf("setup: %v", err)
193+
}
194+
195+
if err := EnsureTUIFullscreen(path); err != nil {
196+
t.Fatalf("run 1: %v", err)
197+
}
198+
first, err := os.ReadFile(path)
199+
if err != nil {
200+
t.Fatalf("read 1: %v", err)
201+
}
202+
203+
var firstObj map[string]json.RawMessage
204+
if err := json.Unmarshal(first, &firstObj); err != nil {
205+
t.Fatalf("parsing run 1: %v", err)
206+
}
207+
if got := string(firstObj["tui"]); got != `"fullscreen"` {
208+
t.Fatalf("after run 1: tui = %s, want \"fullscreen\"", got)
209+
}
210+
211+
if err := EnsureTUIFullscreen(path); err != nil {
212+
t.Fatalf("run 2: %v", err)
213+
}
214+
second, err := os.ReadFile(path)
215+
if err != nil {
216+
t.Fatalf("read 2: %v", err)
217+
}
218+
219+
if string(first) != string(second) {
220+
t.Errorf("second run modified file:\nfirst: %s\nsecond: %s", first, second)
221+
}
222+
}

0 commit comments

Comments
 (0)