Commit 6db3a0a
authored
fix(engine): respect ctx cancel on stream retry, surface self-review revert failure (#32)
* docs: remove generic LLM boilerplate ai_passage.md
ai_passage.md was a 53-line, ~1000-word essay on the history and
ethics of AI in general — entirely unrelated to the hawk project,
no README/AGENTS.md/CHANGELOG.md reference to it. It looks like
LLM-generated filler committed in '99261ca Fix CI formatting and
toolchain hygiene' to satisfy a 'must have an essay' requirement
that no longer applies. Untrack and delete.
* fix(engine): respect ctx cancel on stream retry, surface self-review revert failure
Three related fixes in the hawk agent loop:
1. **stream.go Stream retry no longer blocks ctx cancellation.**
Previously: `time.Sleep(time.Duration(streamAttempt+1) * time.Second)` — a
user-initiated cancel during a reasoning-only retry's backoff was ignored for
up to 3s. Now uses a select with ctx.Done() so cancel is observed immediately,
the in-flight stream is closed, and the loop exits with a structured error event.
2. **stream_tool_exec.go Self-review-before-apply now returns a hard error
when the revert itself fails.** Previously: if the LLM said 'this diff is bad'
and we tried to revert (os.Remove or os.WriteFile) and that syscall failed, the
code only logged s.log.Warn and proceeded with the rejected diff still on disk.
Now: capture the revert error, log it at Error level, and surface a hard tool
error ('Self-review rejected the change AND the revert failed: <err>. Manual
intervention required.') so the LLM can't continue building on top of code it
just flagged as broken.
3. **internal/tool/tool.go now exports ReadOnlyTools + IsReadOnly() so the
safe-concurrent allowlist has a single source of truth.** The map was previously
duplicated in stream.go:716 and stream_tool_exec.go:29 with identical content
— drift waiting to happen. Both call sites now go through tool.IsReadOnly(name),
which canonicalises aliases (read/file_read, ls, etc.) before lookup. A new
test (TestIsReadOnly, TestReadOnlyToolsSetContainsExpectedNames) locks the
contract so a future removal of e.g. 'Read' from the allowlist fails CI in
internal/tool/ rather than silently breaking concurrency classification in
internal/engine/.1 parent 5aa252b commit 6db3a0a
0 file changed
0 commit comments