fix: engine exec safety, safewrite adoption, flock session lock, docs truth - #201
Merged
Conversation
Experiment-loop rollback, auto-commit git calls, and the post-edit syntax validators all ran on context.Background() with ignored errors, so a hung git/go vet/npx invocation could block a session forever and rollback failures were invisible. - experiment loop: thread the request ctx into snapshot(); rollback uses a detached, time-bounded ctx (gitRollbackTimeout) because a revert must still complete after cancellation; log failed rollbacks and failed HEAD lookups instead of discarding the errors - auto-commit: bound every git call with autoCommitTimeout; log git status failures instead of treating them as 'no changes' - validators: bound go vet / python3 / node / npx tsc with validatorTimeout so a wedged toolchain cannot hang post-edit checks No commands, commit messages, or validation logic changed.
…pt eval - AssumptionTracker.VerifyCommandSucceeds ran caller-supplied strings through `sh -c`, bypassing the permission/safety stack. It had zero callers (verified repo-wide) — delete it rather than keep the surface. - SelfHealer.RunScript executed the script path through `sh -c <path>`, re-parsing the path as shell code (double evaluation). Invoke the path directly via /bin/sh; timeout, capture, and exit-code handling are unchanged and shebang-led scripts behave identically.
Global settings, checkpoint file contents, checkpoint restores, handovers, and named checkpoints went through plain os.WriteFile (or a hand-rolled tmp+rename), leaving them open to partial writes and symlink substitution at the destination. All five sites already wrote mode 0600, which is exactly what safewrite.WriteFile produces, so file modes are unchanged while every write becomes atomic (fsync + rename) and symlink-resistant: - internal/config/settings.go SaveGlobal - internal/session/checkpoint.go saveFileContents / restoreFileContents (restore now refuses to write through a symlinked destination and fails loudly instead) - internal/session/handover.go SaveHandover - internal/session/named_checkpoint.go SaveNamedCheckpoint
AcquireLock used stat → stale-if->5min → remove → O_EXCL create, so a live lock could be deleted whenever staleness was misjudged (slow holder, clock skew), letting two instances open the same session. Mutual exclusion now comes from an OS advisory lock (gofrs/flock, promoted to a direct dependency); a crashed holder's lock is reclaimed instantly because the kernel drops the flock at process death — no stale window at all. The lock file keeps PID + timestamps purely as diagnostics (lockStaleAfter now only logs a hung-holder hint on contention), and Release keeps the file with a released marker to avoid the unlock-then-unlink split-brain race. - gofrs/flock v0.13.0 promoted from indirect to direct require; go.work untouched - exported API shape preserved (AcquireLock/Release/Refresh/ SessionLockedError); existing lock tests unchanged and passing - new test: 8 concurrent acquirers, exactly one holder; new test for instant crash reclaim
Memory persistence failures were silently discarded in four stream-loop sites (assistant learnings, skills, conversation summaries, insights); they now log via slog like the existing background-remember path. The self-improve lesson store ignored the whole persist chain (mkdir / marshal / write) and silently swallowed corrupt loads — save now returns an error that Learn/Clear log, and a corrupt store logs and starts empty. The config panel dropped three SetGlobalSetting(provider) failures on the floor; the model selection flow now reports the failure through the panel's error-message mechanism instead. Provider precedence (manual pick > gateway > provider) is unchanged.
The lint, lint-fix, and setup targets installed golangci-lint@latest while CI pins v2.1.0, so local lint results could diverge from the gate. Pin the Makefile to the same version via a GOLANGCI_VERSION variable; the install mechanism (go install on first miss) is unchanged.
SECURITY.md and CONTRIBUTING.md still carried the polyglot template's ruff / mypy / pip-audit / pnpm-lock / pyproject.toml language; this is a pure-Go repo. They now describe the real tools (golangci-lint with gosec rules, go vet, govulncheck) and CONTRIBUTING gains the repo's own dev targets (make setup / boundaries / test-10x / smoke). SPEC_DRIVEN_PLAN.md, SPEC_DRIVEN_PHASE2_PLAN.md, and internal/engine/REFACTOR_PLAN.md move into docs/plans/ (the refactor plan as engine-refactor-plan.md); all code references to the old locations are updated.
external/trace rebranded its root command from 'entire' to 'trace' (branch fix/audit-sweep-2026-08); the golden help snapshot now reflects the mounted command list.
Patel230
marked this pull request as draft
August 16, 2026 03:15
Point all 8 ecosystem submodules to their merged main after the fix/audit-sweep-2026-08 merges: - hawk-core-contracts: strict parsers, FailOn fix, Finding validation - yaad: cascade deletes, DSN pragmas, chunking, backup fsync, key policy - hawk-mcpkit: SSE body cap parity, dead code removal, tool-search docs - inspect: FailOn contract wiring, findings store retry/drop - sight: FailOn contract wiring, dead graph/audit removal - eyrie: Concentrate timeout/retries, stream diagnostics, bodyclose - tok: estimator cache bounds, RestorationTracker cap - trace: entire→trace rebrand, hook binary resolution, settings migration
Patel230
marked this pull request as ready for review
August 16, 2026 04:20
- Update to ede667174 (main branch after PR #110 merge)
All 8 dependency PRs are merged. Per the merge order in the PR description, bump every submodule pointer to the merged commit: - eyrie, hawk-core-contracts, hawk-mcpkit, tok, trace, yaad: merged main - inspect, sight: fix-branch tips (include the required 're-pin hawk-core-contracts to merged main' commits for standalone module builds) go.mod requires updated to the matching pseudo-versions.
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 free
to 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.
Summary
Deep code-quality audit fixes for the main hawk repo:
VerifyCommandSucceeds(zero callers, ransh -cbypassing safety stack);RunScriptno longer double-evals throughsh -cinternal/safewritegofrs/flock; PID file kept for diagnostics onlydocs/plans/entire→tracerename✅ Submodule pins updated — ready for merge
All 8 dependency PRs are merged, and the submodule pointers have been bumped to the merged commits:
ede667116ebcfd4a5ea25b5b6584481d3d1643b66759b437b42bdda9go.modrequires updated to matching pseudo-versions;check-submodule-release-parity.shpasses for all 8 modules.Test plan
go build ./...clean against the submodule fix branchesmake boundaries— all guards passgo test ./...(130+ packages, 0 failures) + boundary guards + govulncheck + vet