fix: address PR review — day sort, path canonicalization, dead code cleanup

- Sort within-day observations chronologically (was specificity-ordered)
- Canonicalize relative paths to POSIX format before DB lookup
- Skip projects param when allProjects is empty (prevents cross-project leaks)
- Remove dead stderrMessage field and hook-command block (unused after permissionDecision switch)
- Type permissionDecision as 'allow' | 'deny' union instead of string
- Remove redundant non-null assertions in getObservationsByFilePath
- Add edit guidance to deny message (use sed via Bash with smart tools)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Alex Newman
2026-04-07 01:59:30 -07:00
parent ef1b427a2a
commit e3475180cd
5 changed files with 89 additions and 92 deletions
File diff suppressed because one or more lines are too long
+9 -3
View File
@@ -129,11 +129,14 @@ function formatFileTimeline(observations: ObservationRow[], filePath: string): s
`- **Already know enough?** The timeline below may be all you need (semantic priming).`,
`- **Need details?** get_observations([IDs]) — ~300 tokens each.`,
`- **Need current code?** smart_outline("${filePath}") for structure (~1-2k tokens), smart_unfold("${filePath}", "<symbol>") for a specific function (~400-2k tokens).`,
`- **Need to edit?** Use smart tools for line numbers, then sed via Bash (Edit requires Read, but you already have the context).`,
];
for (const [day, dayObservations] of sortedDays) {
// Sort within each day chronologically (deduplicateObservations reorders by specificity)
const chronological = [...dayObservations].sort((a, b) => a.created_at_epoch - b.created_at_epoch);
lines.push(`### ${day}`);
for (const obs of dayObservations) {
for (const obs of chronological) {
const title = obs.title || 'Untitled';
const icon = TYPE_ICONS[obs.type] || '\u2753';
const time = compactTime(formatTime(obs.created_at_epoch));
@@ -187,10 +190,13 @@ export const fileContextHandler: EventHandler = {
const context = getProjectContext(input.cwd);
// Observations store relative paths — convert absolute to relative using cwd
const cwd = input.cwd || process.cwd();
const relativePath = path.isAbsolute(filePath) ? path.relative(cwd, filePath) : filePath;
const absolutePath = path.isAbsolute(filePath) ? filePath : path.resolve(cwd, filePath);
const relativePath = path.relative(cwd, absolutePath).split(path.sep).join("/");
const queryParams = new URLSearchParams({ path: relativePath });
// Pass all project names (parent + worktree) for unified lookup
queryParams.set('projects', context.allProjects.join(','));
if (context.allProjects.length > 0) {
queryParams.set('projects', context.allProjects.join(','));
}
queryParams.set('limit', String(FETCH_LOOKAHEAD_LIMIT));
const response = await workerHttpRequest(`/api/observations/by-file?${queryParams.toString()}`, {
-8
View File
@@ -84,14 +84,6 @@ export async function hookCommand(platform: string, event: string, options: Hook
console.log(JSON.stringify(output));
const exitCode = result.exitCode ?? HOOK_EXIT_CODES.SUCCESS;
// If handler wants to send a blocking message via stderr (exit code 2 contract),
// restore stderr and write the message before exiting
if (result.stderrMessage && exitCode === HOOK_EXIT_CODES.BLOCKING_ERROR) {
process.stderr.write = originalStderrWrite;
process.stderr.write(result.stderrMessage);
}
if (!options.skipExit) {
process.exit(exitCode);
}
+1 -2
View File
@@ -20,12 +20,11 @@ export interface HookResult {
hookSpecificOutput?: {
hookEventName: string;
additionalContext: string;
permissionDecision?: string;
permissionDecision?: 'allow' | 'deny';
permissionDecisionReason?: string;
};
systemMessage?: string;
exitCode?: number;
stderrMessage?: string; // Written to stderr before exit (for exit code 2 blocking feedback)
}
export interface PlatformAdapter {
+2 -2
View File
@@ -122,8 +122,8 @@ export function getObservationsByFilePath(
options?: { projects?: string[]; limit?: number }
): ObservationRecord[] {
const rawLimit = options?.limit;
const limit = Number.isInteger(rawLimit) && rawLimit! > 0
? Math.min(rawLimit!, 100)
const limit = Number.isInteger(rawLimit) && (rawLimit as number) > 0
? Math.min(rawLimit as number, 100)
: 15;
const params: (string | number)[] = [filePath, filePath];