Skip to content

Commit 3b6287d

Browse files
committed
refactor: eliminate cwd-worktree assumption, centralize sessionId validation in HookInput
- WorktreeContext now resolved from git rather than assuming cwd is a worktree - GetDiffOutput, WarnBaseBranchEdit, and other hooks use git-based resolution - HookInput.validateSessionId() uses requireThat Jackson validator for fail-fast validation - Remove HookInput.empty() ??? callers now pass valid hard-coded session IDs - Remove getRequiredSessionId() ??? getSessionId() always returns a valid value - Rename getCompositeAgentId() to getCatAgentId() - Remove static SESSION_CACHE from WorktreeLock (caused test contamination) - All tests updated: inject valid session_id, expect IllegalArgumentException on blank/missing
1 parent 7094ddd commit 3b6287d

51 files changed

Lines changed: 1707 additions & 524 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
# State
22

3-
- **Status:** open
4-
- **Progress:** 0%
3+
- **Status:** closed
4+
- **Progress:** 100%
5+
- **Resolution:** implemented
56
- **Dependencies:** []
67
- **Blocks:** []
8+
- **Target Branch:** v2.1

client/src/main/java/io/github/cowwoc/cat/hooks/AotTraining.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ public static void main(String[] args) throws Exception
6363
try (JvmScope scope = new MainJvmScope())
6464
{
6565
JsonMapper mapper = scope.getJsonMapper();
66-
HookInput input = HookInput.empty(mapper);
66+
HookInput input = HookInput.readFrom(mapper, new ByteArrayInputStream(
67+
"{\"session_id\": \"aot-training-session\"}".getBytes(StandardCharsets.UTF_8)));
6768
HookOutput output = new HookOutput(scope);
6869

6970
// Hook handlers with run(HookInput, HookOutput)

client/src/main/java/io/github/cowwoc/cat/hooks/HookInput.java

Lines changed: 24 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*/
77
package io.github.cowwoc.cat.hooks;
88

9+
import static io.github.cowwoc.requirements13.jackson.DefaultJacksonValidators.requireThat;
910
import static io.github.cowwoc.requirements13.java.DefaultJavaValidators.requireThat;
1011

1112
import io.github.cowwoc.cat.hooks.util.SkillLoader;
@@ -67,18 +68,18 @@ private HookInput(JsonMapper mapper, JsonNode data)
6768
* Validates and returns the session ID from the JSON data.
6869
*
6970
* @param data the parsed JSON data
70-
* @return the session ID, or empty string if missing or empty
71-
* @throws IllegalArgumentException if the session_id field is present but contains characters other than
72-
* alphanumerics, hyphens, and underscores
71+
* @return the session ID (never empty)
72+
* @throws IllegalArgumentException if the session_id field is missing, blank, or contains characters
73+
* other than alphanumerics, hyphens, and underscores
7374
*/
7475
private static String validateSessionId(JsonNode data)
7576
{
76-
JsonNode node = data.get("session_id");
77-
if (node == null || !node.isString())
78-
return "";
79-
String value = node.asString();
80-
if (value == null || value.isBlank())
81-
return "";
77+
String value = requireThat(data, "data").property("session_id").isString().getValue().asString();
78+
if (value.isBlank())
79+
{
80+
throw new IllegalArgumentException(
81+
"sessionId is empty. Hook infrastructure must provide CLAUDE_SESSION_ID.");
82+
}
8283
if (!SESSION_ID_PATTERN.matcher(value).matches())
8384
{
8485
throw new IllegalArgumentException("Invalid session_id format: '" + value +
@@ -115,23 +116,22 @@ private static String validateAgentId(JsonNode data)
115116
* Read and parse JSON input from stdin.
116117
*
117118
* @param mapper the JSON mapper to use for parsing
118-
* @return parsed hook input, or empty input if stdin is not available or contains invalid JSON
119+
* @return parsed hook input
119120
* @throws NullPointerException if mapper is null
121+
* @throws IllegalStateException if stdin has no piped input, contains blank/malformed JSON, or is missing
122+
* a session_id
120123
*/
121124
public static HookInput readFromStdin(JsonMapper mapper)
122125
{
123126
requireThat(mapper, "mapper").isNotNull();
124127
try
125128
{
126129
if (System.console() != null && System.in.available() == 0)
127-
{
128-
// Interactive terminal with no piped input
129-
return new HookInput(mapper, mapper.createObjectNode());
130-
}
130+
throw new IllegalStateException("No piped input available on stdin.");
131131
}
132-
catch (IOException _)
132+
catch (IOException e)
133133
{
134-
return new HookInput(mapper, mapper.createObjectNode());
134+
throw new IllegalStateException("Failed to check stdin availability.", e);
135135
}
136136
return readFrom(mapper, System.in);
137137
}
@@ -141,8 +141,9 @@ public static HookInput readFromStdin(JsonMapper mapper)
141141
*
142142
* @param mapper the JSON mapper to use for parsing
143143
* @param inputStream the stream to read from
144-
* @return parsed hook input, or empty input if the stream is not available or contains invalid JSON
144+
* @return parsed hook input
145145
* @throws NullPointerException if mapper or inputStream is null
146+
* @throws IllegalStateException if the stream contains blank/malformed JSON, or is missing a session_id
146147
*/
147148
public static HookInput readFrom(JsonMapper mapper, InputStream inputStream)
148149
{
@@ -153,31 +154,18 @@ public static HookInput readFrom(JsonMapper mapper, InputStream inputStream)
153154
BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8));
154155
String raw = reader.lines().collect(Collectors.joining("\n"));
155156

156-
if (raw == null || raw.isBlank())
157-
return new HookInput(mapper, mapper.createObjectNode());
157+
if (raw.isBlank())
158+
throw new IllegalStateException("Hook input is blank.");
158159

159160
JsonNode node = mapper.readTree(raw);
160161
return new HookInput(mapper, node);
161162
}
162-
catch (JacksonException _)
163+
catch (JacksonException e)
163164
{
164-
return new HookInput(mapper, mapper.createObjectNode());
165+
throw new IllegalStateException("Hook input contains malformed JSON.", e);
165166
}
166167
}
167168

168-
/**
169-
* Create an empty hook input.
170-
*
171-
* @param mapper the JSON mapper to use
172-
* @return an empty HookInput instance
173-
* @throws NullPointerException if mapper is null
174-
*/
175-
public static HookInput empty(JsonMapper mapper)
176-
{
177-
requireThat(mapper, "mapper").isNotNull();
178-
return new HookInput(mapper, mapper.createObjectNode());
179-
}
180-
181169
/**
182170
* Create a HookInput for a bash command with explicit field values.
183171
* <p>
@@ -358,11 +346,8 @@ public boolean isEmpty()
358346

359347
/**
360348
* Get the session ID from standard hook input locations.
361-
* <p>
362-
* Returns empty string if the session ID is missing or empty. Throws if the session ID is present but
363-
* contains characters other than alphanumerics, hyphens, and underscores.
364349
*
365-
* @return the session ID, or empty string if not present
350+
* @return the session ID (never empty)
366351
*/
367352
public String getSessionId()
368353
{
@@ -392,7 +377,7 @@ public String getAgentId()
392377
* @return the composite agent ID
393378
* @throws NullPointerException if {@code sessionId} is null
394379
*/
395-
public String getCompositeAgentId(String sessionId)
380+
public String getCatAgentId(String sessionId)
396381
{
397382
requireThat(sessionId, "sessionId").isNotNull();
398383
String nativeAgentId = getAgentId();

client/src/main/java/io/github/cowwoc/cat/hooks/PostToolUseFailureHook.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,6 @@ public HookResult run(HookInput input, HookOutput output)
7474
requireThat(output, "output").isNotNull();
7575

7676
String sessionId = input.getSessionId();
77-
if (sessionId.isEmpty())
78-
return HookResult.withoutWarnings(output.empty());
79-
8077
// Create handlers using sessionId from HookInput
8178
Path sessionDirectory = scope.getSessionBasePath().resolve(sessionId);
8279
List<PostToolHandler> handlers = List.of(

client/src/main/java/io/github/cowwoc/cat/hooks/PreReadHook.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import static io.github.cowwoc.requirements13.java.DefaultJavaValidators.requireThat;
1010

11+
import io.github.cowwoc.cat.hooks.write.EnforceWorktreePathIsolation;
1112
import tools.jackson.databind.JsonNode;
1213

1314
import java.util.ArrayList;
@@ -41,7 +42,9 @@ public final class PreReadHook implements HookHandler
4142
public PreReadHook(JvmScope scope)
4243
{
4344
requireThat(scope, "scope").isNotNull();
44-
this.handlers = List.of(scope.getPredictBatchOpportunity());
45+
this.handlers = List.of(
46+
scope.getPredictBatchOpportunity(),
47+
new EnforceWorktreePathIsolation(scope));
4548
}
4649

4750
/**

client/src/main/java/io/github/cowwoc/cat/hooks/WorktreeLock.java

Lines changed: 1 addition & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515
import java.nio.file.DirectoryStream;
1616
import java.nio.file.Files;
1717
import java.nio.file.Path;
18-
import java.util.concurrent.ConcurrentHashMap;
19-
import java.util.concurrent.ConcurrentMap;
2018

2119
/**
2220
* Utility for looking up the worktree lock associated with a session.
@@ -27,10 +25,6 @@
2725
*/
2826
public final class WorktreeLock
2927
{
30-
// Cache from sessionId to issueId. Empty string "" is a sentinel meaning "no lock found".
31-
private static final ConcurrentMap<String, String> SESSION_CACHE = new ConcurrentHashMap<>();
32-
private static final int MAX_CACHE_SIZE = 100;
33-
3428
/**
3529
* Prevent instantiation.
3630
*/
@@ -58,20 +52,9 @@ public static String findIssueIdForSession(Path projectCatDir, JsonMapper jsonMa
5852
requireThat(jsonMapper, "jsonMapper").isNotNull();
5953
requireThat(sessionId, "sessionId").isNotBlank();
6054

61-
String cached = SESSION_CACHE.get(sessionId);
62-
if (cached != null)
63-
{
64-
if (cached.isEmpty())
65-
return null;
66-
return cached;
67-
}
68-
6955
Path lockDir = projectCatDir.resolve("locks");
7056
if (!Files.isDirectory(lockDir))
71-
{
72-
cacheResult(sessionId, "");
7357
return null;
74-
}
7558

7659
try (DirectoryStream<Path> stream = Files.newDirectoryStream(lockDir, "*.lock"))
7760
{
@@ -88,9 +71,7 @@ public static String findIssueIdForSession(Path projectCatDir, JsonMapper jsonMa
8871
if (sessionId.equals(sessionNode.asString()))
8972
{
9073
String filename = lockFile.getFileName().toString();
91-
String issueId = filename.substring(0, filename.length() - ".lock".length());
92-
cacheResult(sessionId, issueId);
93-
return issueId;
74+
return filename.substring(0, filename.length() - ".lock".length());
9475
}
9576
}
9677
catch (IOException _)
@@ -104,20 +85,6 @@ public static String findIssueIdForSession(Path projectCatDir, JsonMapper jsonMa
10485
// Lock directory not accessible - no active lock context
10586
}
10687

107-
cacheResult(sessionId, "");
10888
return null;
10989
}
110-
111-
/**
112-
* Stores a result in the session cache, clearing the cache first if the size limit is reached.
113-
*
114-
* @param sessionId the session ID to cache
115-
* @param value the issue ID, or an empty string to indicate no lock was found
116-
*/
117-
private static void cacheResult(String sessionId, String value)
118-
{
119-
if (SESSION_CACHE.size() >= MAX_CACHE_SIZE)
120-
SESSION_CACHE.clear();
121-
SESSION_CACHE.put(sessionId, value);
122-
}
12390
}

client/src/main/java/io/github/cowwoc/cat/hooks/ask/WarnApprovalWithoutRenderDiff.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,6 @@ public Result check(JsonNode toolInput, String sessionId)
6565
if (!Files.isDirectory(catDir))
6666
return Result.allow();
6767

68-
if (sessionId.isEmpty())
69-
return Result.allow();
70-
7168
Path sessionFile = scope.getSessionBasePath().resolve(sessionId + ".jsonl");
7269

7370
if (!Files.exists(sessionFile))

client/src/main/java/io/github/cowwoc/cat/hooks/bash/BlockMainRebase.java

Lines changed: 34 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import io.github.cowwoc.cat.hooks.BashHandler;
1010
import io.github.cowwoc.cat.hooks.HookInput;
1111
import io.github.cowwoc.cat.hooks.JvmScope;
12+
import io.github.cowwoc.cat.hooks.WorktreeContext;
1213
import io.github.cowwoc.cat.hooks.util.GitCommands;
1314

1415
import java.io.IOException;
@@ -56,11 +57,12 @@ public Result check(HookInput input)
5657
{
5758
String command = input.getCommand();
5859
String commandLower = GitCommands.toLowerCase(command);
60+
String sessionId = input.getSessionId();
5961

6062
// Check for git checkout/switch in main worktree
6163
if (CHECKOUT_PATTERN.matcher(commandLower).find())
6264
{
63-
Result checkoutResult = checkCheckoutInMainWorktree(command);
65+
Result checkoutResult = checkCheckoutInMainWorktree(command, sessionId);
6466
if (checkoutResult != null)
6567
return checkoutResult;
6668
}
@@ -70,7 +72,7 @@ public Result check(HookInput input)
7072
return Result.allow();
7173

7274
// Check if rebasing on main
73-
String currentBranch = getCurrentBranch(command);
75+
String currentBranch = getCurrentBranch(command, sessionId);
7476
if (currentBranch == null)
7577
{
7678
return Result.warn(
@@ -106,9 +108,10 @@ public Result check(HookInput input)
106108
* Checks if a checkout command is targeting the main worktree.
107109
*
108110
* @param command the bash command
111+
* @param sessionId the session ID for worktree context lookup
109112
* @return a block result if checkout in main worktree detected, null otherwise
110113
*/
111-
private Result checkCheckoutInMainWorktree(String command)
114+
private Result checkCheckoutInMainWorktree(String command, String sessionId)
112115
{
113116
// Check if command cd's to the project directory
114117
if (cdProjectPattern.matcher(command).find())
@@ -135,30 +138,17 @@ private Result checkCheckoutInMainWorktree(String command)
135138
}
136139
}
137140

138-
// Check if currently in the project directory (main worktree)
139-
String cwd = System.getProperty("user.dir");
140-
if (projectDir.toString().equals(cwd))
141+
WorktreeContext context = WorktreeContext.forSession(
142+
scope.getProjectCatDir(), projectDir, scope.getJsonMapper(), sessionId);
143+
if (context == null)
141144
{
142-
boolean mainWorktree;
143-
try
144-
{
145-
mainWorktree = GitCommands.isMainWorktree();
146-
}
147-
catch (IOException _)
148-
{
149-
return Result.warn(
150-
"Failed to determine if this is the main worktree while checking checkout safety.\n" +
151-
"Proceeding without main worktree check.");
152-
}
153-
if (mainWorktree)
145+
// No active worktree for this session — this is the main context; block checkout
146+
String target = extractCheckoutTarget(command);
147+
if (!isCheckoutFlag(target))
154148
{
155-
String target = extractCheckoutTarget(command);
156-
if (!isCheckoutFlag(target))
157-
{
158-
return Result.block(String.format(
159-
"Blocked: Cannot checkout '%s' in main worktree. Use issue worktrees instead.",
160-
target));
161-
}
149+
return Result.block(String.format(
150+
"Blocked: Cannot checkout '%s' in main worktree. Use issue worktrees instead.",
151+
target));
162152
}
163153
}
164154

@@ -194,9 +184,10 @@ private String extractCheckoutTarget(String command)
194184
* Determines the current branch for the command's target directory.
195185
*
196186
* @param command the bash command (may contain cd to another directory)
187+
* @param sessionId the session ID for worktree context lookup
197188
* @return the branch name, or {@code null} if branch detection failed
198189
*/
199-
private String getCurrentBranch(String command)
190+
private String getCurrentBranch(String command, String sessionId)
200191
{
201192
// Check if command cd's to the project directory
202193
if (cdProjectPattern.matcher(command).find())
@@ -217,10 +208,25 @@ private String getCurrentBranch(String command)
217208
}
218209
}
219210

220-
// Fallback to current directory
211+
// Use lock-based worktree context to determine branch
212+
WorktreeContext context = WorktreeContext.forSession(
213+
scope.getProjectCatDir(), projectDir, scope.getJsonMapper(), sessionId);
214+
if (context == null)
215+
{
216+
// No active worktree for this session — commands run in main context
217+
try
218+
{
219+
return GitCommands.getCurrentBranch(projectDir.toString());
220+
}
221+
catch (IOException _)
222+
{
223+
return null;
224+
}
225+
}
226+
// In a worktree — determine branch from worktree directory
221227
try
222228
{
223-
return GitCommands.getCurrentBranch();
229+
return GitCommands.getCurrentBranch(context.absoluteWorktreePath().toString());
224230
}
225231
catch (IOException _)
226232
{

client/src/main/java/io/github/cowwoc/cat/hooks/bash/BlockUnsafeRemoval.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ public Result check(HookInput input)
105105
String command = input.getCommand();
106106
String workingDirectory = input.getString("cwd");
107107
String sessionId = input.getSessionId();
108-
String catAgentId = input.getCompositeAgentId(sessionId);
108+
String catAgentId = input.getCatAgentId(sessionId);
109109

110110
try
111111
{

0 commit comments

Comments
 (0)