feat(server): in-app project hooks — live config, fail-closed, in-app approvals - #1
Merged
Merged
Conversation
Reviewer's GuideImplements live, fail-closed project hook behavior for PreToolUse across Claude and Codex runtimes, adds unsupported-event warnings, wires Codex approval routing to live hook presence, and documents the new behavior in user docs and an execution plan. Sequence diagram for live PreToolUse evaluation and fail-closed behaviorsequenceDiagram
actor User
participant ClaudeAdapter
participant T3HookRunner
participant FileSystem
participant HookCommand
User->>ClaudeAdapter: invoke full-access tool
ClaudeAdapter->>T3HookRunner: prepare(cwd)
T3HookRunner->>T3HookRunner: resolvePlanState(cwd)
T3HookRunner->>FileSystem: findConfigPath + read hooks.json
FileSystem-->>T3HookRunner: hooks.json content
T3HookRunner->>T3HookRunner: decodeHooksConfigJson
T3HookRunner->>T3HookRunner: decodeHooksConfigEventKeysJson
T3HookRunner->>T3HookRunner: logWarning on unsupported events
T3HookRunner-->>ClaudeAdapter: T3HookPlan(hasPreToolUseHooks, hasPreToolUseHooksNow)
loop each tool call
ClaudeAdapter->>T3HookRunner: plan.evaluatePreToolUse(input)
T3HookRunner->>T3HookRunner: resolvePlanState(cwd)
alt hooks.json unreadable/invalid
T3HookRunner->>T3HookRunner: logConfigFailure(T3HookConfigError)
T3HookRunner-->>ClaudeAdapter: decision ask, title "T3 hook config failed"
else hooks.json valid
T3HookRunner->>HookCommand: run hook command
HookCommand-->>T3HookRunner: HookCommandOutput (allow/ask/deny)
T3HookRunner-->>ClaudeAdapter: normalizedDecision
end
end
ClaudeAdapter-->>User: tool allowed / approval prompt / denied
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
readConfig, you decode the same JSON string twice (once viadecodeHooksConfigJsonand again viadecodeHooksConfigEventKeysJsonjust to get event keys); consider derivingunsupportedEventsdirectly fromconfig.hooksto avoid the extra parse and reduce complexity. hasPreToolUseHooksNowcurrently uses a broadEffect.catchthat logs a config failure and returnstruefor any error; narrowing this toT3HookConfigError(similar to theevaluatePreToolUsecatchTag) would avoid mislabeling unrelated failures as hook-config issues while preserving the fail-closed behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments- In `readConfig`, you decode the same JSON string twice (once via `decodeHooksConfigJson` and again via `decodeHooksConfigEventKeysJson` just to get event keys); consider deriving `unsupportedEvents` directly from `config.hooks` to avoid the extra parse and reduce complexity.
-`hasPreToolUseHooksNow` currently uses a broad `Effect.catch` that logs a config failure and returns `true` for any error; narrowing this to `T3HookConfigError` (similar to the `evaluatePreToolUse` catchTag) would avoid mislabeling unrelated failures as hook-config issues while preserving the fail-closed behavior.
## Individual Comments### Comment 1
<locationpath="apps/server/src/hooks/T3HookRunner.ts"line_range="384" />
<code_context>
return { decision: "allow" } satisfies T3HookDecision;
});
+ const resolvePlanState = Effect.fn("T3HookRunner.resolvePlanState")(function* (cwd: string) {
+ const configPathOption = yield* findConfigPath(cwd);
+ if (Option.isNone(configPathOption)) {
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the new hook plan flow by caching path-derived state, centralizing config-error handling, and deriving unsupported event keys from the already-decoded config instead of extra schemas.
- The extra indirection around config/state resolution and error handling is noticeable. You can keep the new live behaviors but simplify the flow and reduce repeated work.
### 1. Avoid re-running `findConfigPath` on every operation
You currently do:
```tsconst snapshot =yield*resolvePlanState(cwd);
// ...hasPreToolUseHooksNow: resolvePlanState(cwd).pipe(/* ... */),
evaluatePreToolUse: (input) =>Effect.gen(function* () {
const state =yield*resolvePlanState(cwd);
// ...
}).pipe(/* ... */),
````resolvePlanState` re-runs `findConfigPath(cwd)` every time, even though `cwd` is fixed per `prepare` call. You can keep live config re-reads but cache the `configPath` and `projectDirectory` once and re-use them:
```tsconst prepare:T3HookRunner["Service"]["prepare"] =Effect.fn("T3HookRunner.prepare")(function* (cwd) {
const configPathOption =yield*findConfigPath(cwd);
if (Option.isNone(configPathOption)) {
return {
configPath: undefined,
hasPreToolUseHooks: false,
hasPreToolUseHooksNow: Effect.succeed(false),
evaluatePreToolUse: () =>Effect.succeed({ decision: "allow"asconst }),
} satisfiesT3HookPlan;
}
const configPath =configPathOption.value;
const projectDirectory =path.dirname(path.dirname(configPath));
const readCurrentState =Effect.gen(function* () {
const config =yield*readConfig(configPath);
return {
configPath,
projectDirectory,
entries: config.hooks.PreToolUse?? ([] asReadonlyArray<HookMatcherConfig>),
};
});
const initialState =yield*readCurrentState;
const hasHooks =initialState.entries.length>0;
return {
configPath,
hasPreToolUseHooks: hasHooks,
hasPreToolUseHooksNow: readCurrentState.pipe(
Effect.map((state) =>state.entries.length>0),
Effect.catch((error) =>logConfigFailure(error).pipe(Effect.as(true))),
),
evaluatePreToolUse: (input) =>readCurrentState.pipe(
Effect.flatMap((state) =>state.entries.length===0?Effect.succeed({ decision: "allow" } satisfiesT3HookDecision)
:evaluateEntries({
entries: state.entries,
configPath: state.configPath,
projectDirectory: state.projectDirectory!,
payload: { ...input, cwd },
}),
),
Effect.catchTag(
"T3HookConfigError",
handleConfigFailure, // see helper below
),
),
} satisfiesT3HookPlan;
});
```
This keeps:
- Single `findConfigPath` per `prepare`.
- Live config re-reading via `readCurrentState`.
- Snapshot fields (`configPath`, `hasPreToolUseHooks`) plus dynamic ones, but with clearer boundaries.
### 2. Centralize error → decision mapping
Right now the mapping from `T3HookConfigError` to `T3HookDecision` is inlined inside `prepare`:
```tsEffect.catchTag("T3HookConfigError", (error) =>logConfigFailure(error).pipe(
Effect.as({
decision: "ask",
title: "T3 hook config failed",
reason: `T3 project hooks could not be loaded from ${error.configPath}.`,
} satisfiesT3HookDecision),
),
),
```
Encapsulating this makes both `hasPreToolUseHooksNow` and `evaluatePreToolUse` easier to read:
```tsconst handleConfigFailure = (error:T3HookConfigError):Effect.Effect<T3HookDecision> =>logConfigFailure(error).pipe(
Effect.as({
decision: "ask",
title: "T3 hook config failed",
reason: `T3 project hooks could not be loaded from ${error.configPath}.`,
} satisfiesT3HookDecision),
);
```
Then:
```tshasPreToolUseHooksNow: readCurrentState.pipe(
Effect.map((state) =>state.entries.length>0),
Effect.catch((error) =>errorinstanceofT3HookConfigError?handleConfigFailure(error).pipe(Effect.map(() =>true))
:logConfigFailure(errorasT3HookConfigError).pipe(Effect.as(true)),
),
),
evaluatePreToolUse: (input) =>readCurrentState.pipe(
// ...Effect.catchTag("T3HookConfigError", handleConfigFailure),
),
```
This keeps your UX behavior identical but removes duplicated decision construction and shortens the pipelines.
### 3. Simplify unsupported-event warning
You added a second schema + decoder just to get event keys:
```tsconst HooksConfigEventKeys =Schema.Struct({
hooks: Schema.Record(Schema.String, Schema.Unknown),
});
const decodeHooksConfigEventKeysJson =Schema.decodeUnknownEffect(
fromLenientJson(HooksConfigEventKeys),
);
const declaredEvents =yield*decodeHooksConfigEventKeysJson(raw).pipe(
Effect.map((decoded) =>Object.keys(decoded.hooks)),
Effect.orElseSucceed(() => [] asReadonlyArray<string>),
);
```
Since you already successfully decoded `config` above, you can derive event keys directly from it and drop the extra schema/decoder:
```tsconst config =yield*decodeHooksConfigJson(raw);
// ...const declaredEvents =Object.keys(config.hooks);
const unsupportedEvents =declaredEvents
.filter((event) =>!SUPPORTED_HOOK_EVENTS.includes(eventas (typeofSUPPORTED_HOOK_EVENTS)[number]))
.sort();
if (unsupportedEvents.length>0) {
if (!warnedUnsupportedEvents.has(configPath)) {
warnedUnsupportedEvents.add(configPath);
yield*Effect.logWarning("ignoring unsupported T3 hook events", {
path: configPath,
unsupportedEvents,
supportedEvents: SUPPORTED_HOOK_EVENTS,
});
}
}
```
If “warn once per config file” is sufficient, keying `warnedUnsupportedEvents` by `configPath` alone removes the need to build and track `warningKey` strings.
These changes keep all current behaviors (live config updates, fail-closed, warnings) but reduce indirection, duplicated logic, and statefulness.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Uh oh!
There was an error while loading. Please reload this page.
Re-resolve .t3code/hooks.json on every PreToolUse evaluation so mid-session edits, creation, and deletion take effect without a restart; recompute Codex approval routing per turn from the live config; fail closed (ask) with a logged warning when the config becomes unreadable; warn once per config on unsupported hook event keys instead of silently ignoring them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Defmon3force-pushed
the
feat/in-app-hook-confirmation
branch
from
August 13, 2026 13:05
6e304cb to
83d9fd8CompareThread transfer impact
This comment will update automatically after the next completed run. |
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds a Claude-hooks-style PreToolUse gate for Full access threads, configured per project via
.t3code/hooks.json, with approvals surfaced in the app — then solidifies it:.t3code/hooks.jsonis re-read before every hook check. Claude threads pick up create/edit/delete on the next tool call; Codex threads apply command/matcher edits on the next approval and recompute approval routing each turn.PostToolUse,Stop, …) warn once in the server log.allow/ask/denyvia stdout JSON or exit codes; Claude-compatiblehookSpecificOutputaccepted. Codex coverage maps command approvals →Bash, file changes →Edit.Verification
vp test runover the 4 touched test files: 127/127 passvp run --filter t3 typecheck: clean in changed filesgpt-5.6-solreviews over the rebased frozen scope; one low-severity warning-key collision was fixed and narrowly re-reviewed to zero surviving findings.plans/solidify-project-hooks.md; user docs updated indocs/user/permission-modes.md🤖 Generated with Claude Code
Summary by Sourcery
Make T3 project hooks a live, fail-closed config for full-access sessions and align Codex approval routing with hook presence.
New Features:
Enhancements:
Tests: