From 2cf8447b3e4e962542e6db79806cbee87c88eb51 Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Fri, 21 Aug 2026 20:38:11 +0530 Subject: [PATCH 1/7] feat: adopt agent runtime hardening patterns --- cmd/chat_commands_skills.go | 4 +- cmd/skills_cmd.go | 4 +- docs/architecture/hawk-harness.md | 32 ++++ docs/plans/commandcodeai-adoption-plan.md | 108 +++++++++++ docs/plans/minimax-adoption-plan.md | 200 ++++++++++++++++++++ docs/plans/qwen-code-adoption-plan.md | 66 +++++++ docs/user-guide/26-learned-preferences.md | 34 ++++ docs/user-guide/27-workflows.md | 48 +++++ docs/user-guide/28-workflow-budgets.md | 17 ++ external/eyrie | 2 +- internal/attachment/normalize.go | 216 ++++++++++++++++++++++ internal/attachment/normalize_test.go | 212 +++++++++++++++++++++ internal/engine/persistence_service.go | 101 ++++++++++ internal/engine/persistence_trim_test.go | 136 ++++++++++++++ internal/engine/search/jina.go | 160 ++++++++++++++++ internal/engine/search/jina_test.go | 157 ++++++++++++++++ internal/engine/session.go | 5 +- internal/engine/stream.go | 6 + internal/engine/stream_tool_exec.go | 29 +++ internal/engine/tool_lifecycle_test.go | 28 +++ internal/engine/tool_service.go | 46 ++++- internal/plugin/audit.go | 14 +- internal/plugin/skill_validate.go | 79 ++++++++ internal/plugin/skill_validate_test.go | 60 ++++++ internal/plugin/skills_auto.go | 18 +- internal/plugin/skills_auto_test.go | 40 ++++ 26 files changed, 1798 insertions(+), 24 deletions(-) create mode 100644 docs/architecture/hawk-harness.md create mode 100644 docs/plans/commandcodeai-adoption-plan.md create mode 100644 docs/plans/minimax-adoption-plan.md create mode 100644 docs/plans/qwen-code-adoption-plan.md create mode 100644 docs/user-guide/26-learned-preferences.md create mode 100644 docs/user-guide/27-workflows.md create mode 100644 docs/user-guide/28-workflow-budgets.md create mode 100644 internal/attachment/normalize.go create mode 100644 internal/attachment/normalize_test.go create mode 100644 internal/engine/persistence_trim_test.go create mode 100644 internal/engine/search/jina.go create mode 100644 internal/engine/search/jina_test.go create mode 100644 internal/engine/tool_lifecycle_test.go create mode 100644 internal/plugin/skill_validate.go create mode 100644 internal/plugin/skill_validate_test.go diff --git a/cmd/chat_commands_skills.go b/cmd/chat_commands_skills.go index a5a1c733..b7cd7d8b 100644 --- a/cmd/chat_commands_skills.go +++ b/cmd/chat_commands_skills.go @@ -237,13 +237,13 @@ func (m *chatModel) handleSkillsCommand(parts []string, text string) (tea.Model, m.messages = append(m.messages, displayMsg{role: "error", content: err.Error()}) return m, nil } - r := plugin.AuditResult{Findings: findings, Files: 1} + r := plugin.AuditResult{Findings: findings, Validation: plugin.ValidateSkillFile(target), Files: 1} m.messages = append(m.messages, displayMsg{role: "system", content: plugin.FormatAuditResult(r)}) return m, nil } if _, path, ok := plugin.InstalledSkillInfo(target); ok { findings, _ := plugin.AuditSkillFile(path) - r := plugin.AuditResult{Findings: findings, Files: 1} + r := plugin.AuditResult{Findings: findings, Validation: plugin.ValidateSkillFile(path), Files: 1} m.messages = append(m.messages, displayMsg{role: "system", content: plugin.FormatAuditResult(r)}) return m, nil } diff --git a/cmd/skills_cmd.go b/cmd/skills_cmd.go index 96c30155..973b4377 100644 --- a/cmd/skills_cmd.go +++ b/cmd/skills_cmd.go @@ -161,7 +161,7 @@ var skillsAuditCmd = &cobra.Command{ if err != nil { return err } - r := plugin.AuditResult{Findings: findings, Files: 1} + r := plugin.AuditResult{Findings: findings, Validation: plugin.ValidateSkillFile(target), Files: 1} if jsonOut { data, _ := json.MarshalIndent(r, "", " ") fmt.Println(string(data)) @@ -172,7 +172,7 @@ var skillsAuditCmd = &cobra.Command{ } if _, path, ok := plugin.InstalledSkillInfo(target); ok { findings, _ := plugin.AuditSkillFile(path) - r := plugin.AuditResult{Findings: findings, Files: 1} + r := plugin.AuditResult{Findings: findings, Validation: plugin.ValidateSkillFile(path), Files: 1} if jsonOut { data, _ := json.MarshalIndent(r, "", " ") fmt.Println(string(data)) diff --git a/docs/architecture/hawk-harness.md b/docs/architecture/hawk-harness.md new file mode 100644 index 00000000..9f093845 --- /dev/null +++ b/docs/architecture/hawk-harness.md @@ -0,0 +1,32 @@ +# Hawk Harness + +Hawk treats the agent runtime as a product boundary around provider output. + +## Tool-Call Path + +1. Eyrie normalizes provider protocol responses. +2. Hawk validates and resolves tool metadata. +3. Permission and sandbox policy runs before execution. +4. Tool execution observes timeouts, cancellation, and path boundaries. +5. Results are redacted, persisted, and returned to the model. +6. Stream retry and reasoning-only recovery handle provider failures. +7. Session cleanup removes dangling tool-use/result turns after cancellation. + +## Context Integrity + +Compaction preserves API invariants by keeping tool-use and tool-result pairs +valid while clearing or reducing old content according to policy. Token, +turn, cost, and time limits are separate controls and should not be treated as +interchangeable. + +## Verification + +The harness is testable without live providers through scripted providers and +recorded interactions. Provider protocol and behavioral conformance belongs in +Eyrie's verification package; Hawk owns host-level UX, permissions, persistence, +and review contracts. + +## Security Rule + +Personalized preferences may influence ranking or suggestions, but they cannot +disable objective security, correctness, permission, or audit behavior. diff --git a/docs/plans/commandcodeai-adoption-plan.md b/docs/plans/commandcodeai-adoption-plan.md new file mode 100644 index 00000000..5f1bcd44 --- /dev/null +++ b/docs/plans/commandcodeai-adoption-plan.md @@ -0,0 +1,108 @@ +# CommandCodeAI Adoption Plan + +Status: Implemented in the Hawk working tree where the existing architecture +supports a safe, native implementation. + +## Source Review + +The reviewed CommandCodeAI organization contains four relevant categories: + +| Repository | Evidence | Decision | +|---|---|---| +| `command-code` | Current public tree is documentation/assets; historical `gpt3-agent` is a small unsafe prototype | Do not copy code; adopt UX and workflow documentation ideas | +| `cmd-old-public` | Archived placeholder/documentation repository | No implementation to adopt | +| `BaseAI` | Archived TypeScript pipe SDK/local provider server; licensing metadata is inconsistent | Reimplement narrow ideas only; do not add as a dependency | +| `agent-skills` | MIT skill collection with progressive-disclosure guidance | Adopt authoring/process ideas; preserve individual asset licenses | +| `awesome-agents` | Apache-2.0 example applications | Reference only; do not merge into Hawk skills | + +`hawk-community-skills` remains the canonical public skill registry. Its +validator and registry tooling are more complete than the CommandCodeAI +repositories and should remain authoritative. + +## Adopted Work + +### 1. Skill metadata interoperability + +Hawk's smart-skill parser accepts both hyphenated and community-schema +snake_case keys: + +- `auto-invoke` / `auto_invoke` +- `allowed-tools` / `allowed_tools` +- `source-repo` / `source_repo` +- `source-ref` / `source_ref` +- `source-installed-at` / `source_installed_at` +- `chain-after` / `chain_after` +- `chain-before` / `chain_before` +- `chain-conflicts` / `chain_conflicts` +- `chain-enhances` / `chain_enhances` + +Tests cover all aliases. This prevents a community skill from installing +successfully while silently losing invocation, tool, provenance, or chain +metadata. + +### 2. Local skill validation + +Hawk's existing Unicode audit remains the runtime security scanner. A new +structural validator complements it by checking: + +- required `name` and `description` +- lowercase kebab-case names +- name/directory agreement +- description length +- semantic version format +- `SKILL.md` size +- `@ref(...)` path containment + +`hawk skills audit` now reports both Unicode and structural findings. This is a +small Go-native subset of the community repository's broader validation model; +it does not duplicate the registry's Python implementation. + +### 3. Transparent preference model + +Hawk already has `internal/feature/taste` with confidence, sample count, +decay, project identity, merge, reset, prompt projection, and accept/edit +signals. No second preference database was created. The user-facing model and +policy are documented in `docs/user-guide/26-learned-preferences.md`: + +- explicit instructions remain authoritative +- skills are reusable capabilities +- preferences are inferred tendencies +- Yaad stores durable facts and decisions +- objective review findings cannot be suppressed by preferences + +### 4. Workflow and harness documentation + +CommandCodeAI's strongest product contribution is discoverability. Hawk now +documents its existing capabilities in: + +- `docs/user-guide/26-learned-preferences.md` +- `docs/user-guide/27-workflows.md` +- `docs/architecture/hawk-harness.md` +- `docs/user-guide/28-workflow-budgets.md` + +These cover slash/shell/file-context input, headless review, MCP-backed +analysis, session recovery, recording/replay, skills, preference boundaries, +tool-call repair, permission ordering, stream recovery, and the distinct turn, +tool, depth, time, token, and cost budgets. + +## Deliberately Not Adopted + +- CommandCodeAI provider adapters: Eyrie owns provider protocols and routing. +- BaseAI remote pipes: incompatible with Hawk's local authority and durable + event model. +- BaseAI `lowdb` JSON memory: weaker than Yaad and Hawk persistence. +- Unconditional parallel tool execution: unsafe for mutations and approvals. +- Historical `gpt3-agent`: no permissions, sandbox, path guard, audit, or tests. +- Media/UI/status repositories: outside Hawk's code-intelligence boundary. +- CommandCodeAI branding, proprietary model claims, and undocumented services. + +## Verification Plan + +1. Run formatting and static checks on all changed Hawk Go files. +2. Run focused parser, validator, taste, engine, and command tests. +3. Run the full Hawk test suite and vet. +4. Repeat the focused and full checks independently. +5. Inspect the final diff, worktree, and submodule status. + +The eyrie submodule remains a separate repository change and must be published +through its own feature branch and PR before updating Hawk's submodule pointer. diff --git a/docs/plans/minimax-adoption-plan.md b/docs/plans/minimax-adoption-plan.md new file mode 100644 index 00000000..9061508d --- /dev/null +++ b/docs/plans/minimax-adoption-plan.md @@ -0,0 +1,200 @@ +# MiniMax-AI → hawk Adoption Plan + +Status: Implemented in working tree; submodule PRs and skills curation remain follow-ups +Date: 2026-08-21 +Scope: Adopt the high-value, verified concepts from MiniMax-AI's open-source +repos into the hawk ecosystem (hawk + submodules: eyrie, hawk-core-contracts, +hawk-mcpkit, sight, inspect). + +## Findings summary + +Deep code review of 6 MiniMax-AI repos against hawk's existing submodules and +internals produced these adoptions, ordered by value: + +| # | MiniMax repo | Adopt into | Action | +|---|---|---|---| +| 1 | MiniMax-Provider-Verifier | `external/eyrie` | Add provider-conformance metrics + wire the orphaned `verify` harness into CI | +| 2 | MiniMax/skills | `hawk-community-skills` | Curate high-quality skill content (content, not mechanism) | +| 3 | MiniMax-Coding-Plan-MCP | hawk / `hawk-mcpkit` | MCP v2 no-network test pattern + image-source normalization (patterns only) | +| 4 | minimax_search | hawk tooling | Jina page→Markdown browse extractor + evidence-extraction prompt (pattern) | +| 5 | Mini-Agent | hawk engine | Cooperative-cancellation-with-cleanup + token-aware lossy summarization (patterns) | +| 6 | MiniMax-MCP / JS | — | Not adoptable (media generation, out of scope) | + +## 1. Eyrie provider-conformance metrics + CI wiring (highest value) + +### Verified current state (eyrie submodule) + +- `external/eyrie/verify/verify.go` — a complete behavioral conformance harness + exists: `Case`/`CaseResult`/`Expectation`/`Report`, `Run()`, `scoreResponse()`, + `ToolCallF1()`, `DiffBaseline()`, `Markdown()`. +- `external/eyrie/verify/cases.go` — `CanonicalCases()` with 3 cases: + `basic-chat`, `deterministic-answer`, `tool-call`. +- `external/eyrie/verify/metrics.go` — `ToolCallF1()` already implemented. +- **Confirmed gap:** the `verify` package is referenced ONLY by its own tests. + It is not wired into any CLI, Makefile target, CI job, or provider-registration + gate. Only structural registry/parity tests are enforced. +- eyrie already ships MiniMax providers (`minimax_token_plan`, `minimax_payg`) + as OpenAI-compatible adapters with `ThinkingFormat: "minimax"`. +- `client/structured.go` already has `ValidateStructuredOutput()` — a recursive + JSON-schema validator (`validateValue`, `validateObject`, `validateArray`) + reusable for tool-call argument schema checks. + +### What to implement + +**A. Add a `SchemaValidate` helper (argument-level) — new file +`external/eyrie/verify/schema.go`** + +Add `SchemaValidate(args map[string]any, schema map[string]any) error` that +checks tool-call arguments against the tool's `Parameters` JSON schema: +- required-arg presence +- property type checks (reuse pattern from `client.ValidateStructuredOutput`) + +This is the `ToolCalls-Schema-Accuracy` dimension from MiniMax-Provider-Verifier. + +**B. Add `MatchRate` to `Report` — edit `external/eyrie/verify/verify.go`** + +Compute the `ToolCalls-Match-Rate` (trigger-vs-stop correctness) alongside F1. +Extend `CaseResult` already has `ExpectedTool`/`CalledAnyTool`/`CorrectTool`, so +compute: +``` +tool_calls_match_rate = (TP + TN) / expected_tool_call_total_count +``` +mirroring MiniMax's confusion-matrix metric. + +**C. Add a `verify` CLI entrypoint — new file +`external/eyrie/cmd/verify/main.go`** + +A small `go run`-able binary (or a `verify` subcommand if eyrie has a cmd/) that +runs `verify.Run` against a provider endpoint (live or cassette) and exits +non-zero on unmet thresholds: +- `--model`, `--base-url`, `--api-key`, `--provider` +- `--threshold-score` (default e.g. 0.98) +- prints the `Report.Markdown()` and `DiffBaseline()` against a stored baseline. + +This makes the orphaned harness operational and enables CI gating. + +**D. Add a Makefile target + CI job — edit `external/eyrie/Makefile` and +`external/eyrie/.github/workflows/ci.yml`** + +- Makefile: `verify` target that runs the harness in cassette mode (no tokens) + and `verify-live` for manual live runs. +- CI: a `verify` job that runs the cassette-replay conformance test as a gate + on PRs, ensuring the harness is exercised and providers don't regress. + +### Files +- `external/eyrie/verify/schema.go` (new) +- `external/eyrie/verify/schema_test.go` (new) +- `external/eyrie/verify/verify.go` (add MatchRate) +- `external/eyrie/verify/verify_test.go` (add tests) +- `external/eyrie/cmd/verify/main.go` (new) +- `external/eyrie/Makefile` (verify target) +- `external/eyrie/.github/workflows/ci.yml` (verify job) + +## 2. Curate MiniMax/skills content into hawk-community-skills + +### Verified current state (hawk) +- hawk's skills system (`internal/plugin`) reads markdown+YAML-frontmatter skills + from dirs (`~/.hawk/skills`, `.claude/skills`, `.zero/skills`, `skills`) with a + registry (`hawk-community-skills` repo, `hawk skills search/install/list/remove`). +- MiniMax/skills (13.4k★) has 18 high-quality skills in the same format + (frontmatter `name`/`description`/`license`/`metadata`), especially + `frontend-dev`, `fullstack-dev`, `shader-dev`, mobile guides, `vision-analysis`. + +### What to implement +- Port the best MiniMax skills into `GrayCodeAI/hawk-community-skills` (separate + repo), adapting frontmatter to hawk's convention (`globs`, `alwaysApply`). +- This is a content curation task in a separate repo; tracked here for + completeness but implemented as a follow-up PR in `hawk-community-skills`. + +### Files +- `hawk-community-skills/registry.json` (add entries) +- `hawk-community-skills/skills//SKILL.md` (port content) + +## 3. MCP v2 no-network test pattern + image-source normalization + +### Verified current state (hawk) +- `internal/mcp` implements its own JSON-RPC client + server and does NOT use the + shared `hawk-mcpkit` scaffolding (architectural divergence). +- hawk has `internal/attachment/image.go` for image decode, and `ScreenshotTool` + / `BrowserTool`. No image-source (URL/file/data-URL) normalization helper. +- MiniMax-Coding-Plan-MCP shows: MCP v2 tool registration + no-network test + harness (in-process `Client` + stdio `ClientSession` asserting exact payloads) + + image-source normalization. + +### What to implement (patterns only — no vendor-locked Python) +- Add a Go helper `internal/attachment/normalize_image_source.go` that converts + HTTP URL / local path / data-URL / base64 into a data URL (`@`-prefix strip), + mirroring MiniMax's `process_image_url`. Add unit tests. +- Add an MCP no-network integration test pattern to `internal/mcp` or + `hawk-mcpkit` demonstrating in-process payload assertion without network. + +### Files +- `internal/attachment/normalize_image_source.go` (new) +- `internal/attachment/normalize_image_source_test.go` (new) + +## 4. Jina page→Markdown browse extractor + +### Verified current state (hawk) +- hawk has `WebSearchTool` (6-provider cascade: Brave/SearXNG/DeepSeek/Exa/ + Perplexity/DDG), `AgenticFetchTool`, `WebFetchTool`, `DownloadTool`, + `engine/search/url_scraper.go`. +- No lightweight "URL → Markdown" extractor (Jina Reader pattern). + +### What to implement (pattern only) +- Add `internal/search/jina.go`: `FetchAsMarkdown(ctx, url)` calling + `POST https://r.jina.ai/` with `X-Return-Format: markdown`, `X-Timeout`, + `X-Engine: direct`, returning clean Markdown. +- Optional: an evidence-extraction prompt helper (chunk-then-merge) for + long-content answering. +- Guarded behind config so it's off unless enabled (Jina key optional). + +### Files +- `internal/search/jina.go` (new) +- `internal/search/jina_test.go` (new, with mock HTTP server) + +## 5. Agent-loop robustness patterns (reference) + +### Verified current state (hawk) +- hawk's `Session.agentLoop` (`internal/engine/stream.go:133`) is a complete + think→act→observe loop with memory, 70+ tools, sub-agents, multi-agent. +- No cooperative-cancellation-with-history-cleanup, and long-session compaction + uses truncation rather than "summarize between user turns". + +### What to implement (small, low-risk) +- Add `_cleanup_incomplete_messages` equivalent to hawk's loop: on cancellation + at a safe checkpoint, trim the partial assistant message + orphaned tool + results so message history stays valid. +- Add a token-limit-driven lossy summarization option (keep user intents, + collapse interleaved tool/assistant turns into an LLM summary) as a session + compaction strategy, with a consecutive-trigger guard. + +### Files (tentative) +- `internal/engine/stream.go` (cancellation cleanup) +- `internal/engine/compact/` (lossy summarize strategy) + +## Out of scope (not adopted) +- MiniMax-MCP / MiniMax-MCP-JS: media generation (TTS/image/video) — no + relevance to code intelligence; hawk has no TTS and that is a product decision. +- minimax_search's search layer: redundant (hawk has more providers). +- Mini-Agent's core loop: outclassed by hawk. +- The `verify` harness's live-token path: optional, off by default. + +## Execution order +1. Eyrie verify metrics + schema validation + tests (highest value) +2. Eyrie verify CLI + Makefile + CI wiring +3. hawk image-source normalization helper + tests +4. hawk Jina browse extractor + tests +5. Agent-loop cancellation/summarization patterns +6. hawk-community-skills content curation (follow-up PR in separate repo) + +## Implementation status + +- [x] Eyrie schema-accuracy validation and tool-call match-rate metrics. +- [x] Eyrie verification CLI, deterministic Makefile target, and CI job. +- [x] Hawk image-source normalization for data URIs, URLs, local files, and raw base64. +- [x] Hawk Jina Reader page-to-Markdown client with opt-in configuration and tests. +- [x] Hawk cancellation cleanup for incomplete assistant tool-use/tool-result turns. +- [x] Confirmed hawk's existing `internal/engine/compact` already provides token-triggered + compaction; no duplicate summarizer was added. +- [ ] Publish the eyrie submodule changes through its own feature branch and PR. +- [ ] Curate MiniMax skills into `hawk-community-skills` through its own feature branch and PR. diff --git a/docs/plans/qwen-code-adoption-plan.md b/docs/plans/qwen-code-adoption-plan.md new file mode 100644 index 00000000..ebf70b1b --- /dev/null +++ b/docs/plans/qwen-code-adoption-plan.md @@ -0,0 +1,66 @@ +# Qwen Code Adoption Plan + +Status: Implemented selectively in the current Hawk feature branch. + +## Guardrails + +Qwen Code is Apache-2.0 TypeScript software with a Gemini-shaped core. Hawk +will independently reimplement behavioral contracts in Go, preserve the Eyrie +provider boundary, and retain Hawk's event-sourced sessions and OS sandbox. +No Qwen source or dependency is vendored. + +## Existing Hawk Capabilities + +Hawk already has durable sessions, event logging, WAL/recovery, branching, +review contracts, provider routing in Eyrie, MCP integration, skills, memory, +context compaction, policy snapshots, subagents, daemon security, and +filesystem/process sandboxing. These systems will not be duplicated. + +## Implemented + +1. Added an explicit tool lifecycle vocabulary to the existing tool execution + path: validating, permission pending, executing, completed, failed, + cancelled, and timed out. +2. Added terminal reasons for permission denial, approval denial, unknown tool, + pipeline failure, timeout, cancellation, execution failure, and success. +3. Added regression coverage for the lifecycle contract. +4. Preserved Hawk's existing cancellation transcript cleanup and compaction. +5. Preserved policy-snapshot inheritance and subagent cleanup already present. +6. Added explicit `StreamEvent` tool lifecycle state and terminal-reason fields, + with execution-path transitions and regression coverage. + +## Future Work + +### P0: Safety and lifecycle + +- Emit lifecycle state transitions as structured eventlog entries. +- Record whether cancellation occurred before or after a side effect. +- Add shell virtual-operation escalation for compound commands. +- Add resume repair records for synthetic tool results. + +### P1: MCP and background work + +- Add per-session MCP server/tool/process budgets. +- Add deterministic refusal and health/reconnect status events. +- Add background task records with bounded drain and cancellation. +- Add ACP replay windows, prompt ledgers, and pre-attach bounds. + +### P2: Declarative extensibility + +- Map Markdown subagent frontmatter to Hawk's typed SpawnRequest. +- Add path-conditional skill activation and parse-error diagnostics. +- Add hot reload with bounded activation listeners. +- Scope child hooks and MCP resources by session/agent ID. + +### P3: Memory and orchestration + +- Add fast deterministic memory recall followed by optional refinement. +- Persist task origin, delivery/discard decisions, and memory deduplication. +- Add workflow snapshots only after lifecycle and budget contracts are stable. + +## Verification + +- Focused lifecycle, skills, engine, and command tests. +- Full Hawk build, vet, and test suite. +- Independent second verification pass. +- Final diff and submodule status inspection. diff --git a/docs/user-guide/26-learned-preferences.md b/docs/user-guide/26-learned-preferences.md new file mode 100644 index 00000000..ab6fa474 --- /dev/null +++ b/docs/user-guide/26-learned-preferences.md @@ -0,0 +1,34 @@ +# Learned Preferences + +Hawk can learn coding-style tendencies from feedback through its taste system. +These preferences are advisory context, not policy. + +## Policy Boundaries + +- Explicit user instructions override learned preferences. +- Project rules and `AGENTS.md` remain authoritative. +- Permission, sandbox, and security controls cannot be weakened by taste. +- Review severity and correctness findings cannot be hidden by preferences. +- Skills provide reusable procedures; preferences describe tendencies. +- Yaad stores durable facts, conventions, and decisions separately. + +Examples of suitable preferences include table-driven tests, preferred error +wrapping style, naming conventions, and project-specific abstraction habits. + +## Evidence + +Preference confidence is based on repeated observations such as accepted edits, +corrections, and explicit feedback. A single interaction should not silently +become a project rule. Low-confidence signals are not injected into prompts. + +## Review Usage + +Preference-aware review should supplement objective checks: + +```text +Objective: security, correctness, regression risk, missing tests +Preference: naming, test organization, error-handling convention +``` + +The objective layer always wins. Use `/taste` and `/learn` to inspect or teach +preferences explicitly rather than relying on opaque model behavior. diff --git a/docs/user-guide/27-workflows.md b/docs/user-guide/27-workflows.md new file mode 100644 index 00000000..7ad943de --- /dev/null +++ b/docs/user-guide/27-workflows.md @@ -0,0 +1,48 @@ +# Workflows + +## Interactive Input + +- `/command` invokes a Hawk slash command. +- `!command` runs a direct shell command when the current permission mode allows it. +- `@path` adds file or directory context. +- `Esc` interrupts the current operation. +- Use session, checkpoint, branch, rewind, and compact commands for long work. + +## Headless Review + +Use a bounded, machine-readable review in CI: + +```bash +hawk review run HEAD --output-format json --max-turns 8 +``` + +Review findings remain structured and severity-based. Provider retries, +permissions, and tool timeouts are still enforced in headless mode. + +## MCP-Backed Analysis + +1. Establish project trust. +2. Inspect configured MCP servers with `hawk mcp`. +3. Run the review or scan command. +4. Check the persisted findings and event output. + +Do not bypass trust or permission controls to make an MCP tool convenient. + +## Session Recovery + +Use `/session`, `/resume`, `/continue`, `/checkpoint`, and `/rewind` to recover +from interruptions. Hawk trims incomplete tool turns before a cancelled session +is reused, preserving provider transcript invariants. + +## Recording and Replay + +Use `--record` when diagnosing a terminal or provider interaction, then replay +the resulting artifact without making another live provider request. Keep +recordings free of credentials and sensitive tool output. + +## Skills and Preferences + +Install skills through the registry, audit them before activation, and keep +large references in `references/` rather than bloating `SKILL.md`. Use learned +preferences for style alignment only; never use them to override security or +explicit project policy. diff --git a/docs/user-guide/28-workflow-budgets.md b/docs/user-guide/28-workflow-budgets.md new file mode 100644 index 00000000..1f2ab15e --- /dev/null +++ b/docs/user-guide/28-workflow-budgets.md @@ -0,0 +1,17 @@ +# Workflow Budgets + +Hawk exposes several independent limits. Configure the smallest useful scope +for automation and distinguish them when diagnosing termination. + +| Budget | Limits | Purpose | +|---|---|---| +| Turns | Model/agent iterations | Stops runaway reasoning loops | +| Tool calls | Tool operations in a workflow | Bounds action volume | +| Agent depth | Nested sub-agent levels | Bounds recursive delegation | +| Wall clock | Operation duration | Prevents hung work | +| Tokens | Context and completion usage | Controls context cost and compaction | +| Cost | Provider spend in USD | Hard financial boundary | + +A limit reached is not equivalent to a provider error. Logs and lifecycle +events should identify the specific budget and current/maximum values. A retry +must not reset a consumed budget, and cancellation must remain authoritative. diff --git a/external/eyrie b/external/eyrie index ede66717..2c070236 160000 --- a/external/eyrie +++ b/external/eyrie @@ -1 +1 @@ -Subproject commit ede6671749b14807990ee7614cac2eb5192aa482 +Subproject commit 2c070236c5f7784a329a2c2103d3a13cb89b091d diff --git a/internal/attachment/normalize.go b/internal/attachment/normalize.go new file mode 100644 index 00000000..948ac622 --- /dev/null +++ b/internal/attachment/normalize.go @@ -0,0 +1,216 @@ +package attachment + +import ( + "context" + "encoding/base64" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "strings" + "time" +) + +// NormalizeImageError describes a failure to resolve a user-supplied image +// source (URL, data URI, raw base64, or local path) into durable bytes and a +// media type. +type NormalizeImageError struct { + Reason string +} + +func (e *NormalizeImageError) Error() string { return "normalize image: " + e.Reason } + +// MediaTypeFromMIME narrows a wire MIME string to the durable raster +// vocabulary, returning "" for unsupported types. It accepts both the +// "image/png" form and the bare "png" form. +func MediaTypeFromMIME(value string) ImageMediaType { + v := strings.ToLower(strings.TrimSpace(value)) + v = strings.TrimPrefix(v, "image/") + for _, mt := range AllMediaTypes { + if string(mt) == "image/"+v { + return mt + } + } + return "" +} + +// MediaTypeFromExtension maps a file extension to an image media type, or "" +// if unrecognised. +func MediaTypeFromExtension(name string) ImageMediaType { + switch strings.ToLower(filepath.Ext(name)) { + case ".png": + return MediaTypePNG + case ".jpg", ".jpeg": + return MediaTypeJPEG + case ".webp": + return MediaTypeWebP + case ".gif": + return MediaTypeGIF + default: + return "" + } +} + +// NormalizeImageSource resolves a user-supplied image source into bytes plus a +// media type ready for SaveImage, mirroring the source-normalization behaviour +// of the MiniMax Coding-Plan MCP server (URL / local path / data URI / raw +// base64 → canonical bytes). The recognised forms, in order: +// +// - a "data:" URI (data:image/;base64,<...>) +// - an http(s) URL (fetched over the network) +// - a raw base64 string (decoded directly) +// - a local filesystem path (read from disk) +// +// A leading '@' on a bare value is treated as an explicit local-file marker +// and stripped, matching the MCP convention. +func NormalizeImageSource(src string, opts ...NormalizeOption) (SaveImage, error) { + cfg := defaultNormalizeConfig() + for _, o := range opts { + o(&cfg) + } + + src = strings.TrimSpace(src) + + // Explicit local-file marker: "@path". + if strings.HasPrefix(src, "@") { + return loadImageFile(strings.TrimPrefix(src, "@"), cfg) + } + + // data: URI. + if strings.HasPrefix(src, "data:") { + return decodeDataURI(src) + } + + // http(s) URL. + if strings.HasPrefix(src, "http://") || strings.HasPrefix(src, "https://") { + if cfg.HTTPClient == nil { + return SaveImage{}, &NormalizeImageError{Reason: "image URL source requires an HTTP client (HTTPClient option)"} + } + return fetchImageURL(src, cfg) + } + + // Raw base64 — only if it looks like base64 (to avoid confusing a file + // path or plain text with an image). + if looksLikeBase64(src) { + data, err := base64.StdEncoding.DecodeString(src) + if err != nil { + return SaveImage{}, &NormalizeImageError{Reason: "source is neither a valid URL, data URI, nor base64: " + err.Error()} + } + return SaveImage{Data: data, MediaType: MediaTypeFromMIME(detectMIME(data))}, nil + } + + // Fallback: local file path. + if cfg.AllowLocalFiles { + return loadImageFile(src, cfg) + } + return SaveImage{}, &NormalizeImageError{Reason: "source not recognised (and local files disabled)"} +} + +// NormalizeOption customises NormalizeImageSource. +type NormalizeOption func(*normalizeConfig) + +type normalizeConfig struct { + HTTPClient *http.Client + AllowLocalFiles bool +} + +func defaultNormalizeConfig() normalizeConfig { + return normalizeConfig{ + HTTPClient: &http.Client{Timeout: 30 * time.Second}, + AllowLocalFiles: true, + } +} + +// WithHTTPClient sets the HTTP client used to fetch URL sources. +func WithHTTPClient(hc *http.Client) NormalizeOption { + return func(c *normalizeConfig) { c.HTTPClient = hc } +} + +// WithAllowLocalFiles toggles reading image sources from the local filesystem. +func WithAllowLocalFiles(allow bool) NormalizeOption { + return func(c *normalizeConfig) { c.AllowLocalFiles = allow } +} + +func loadImageFile(path string, cfg normalizeConfig) (SaveImage, error) { + data, err := os.ReadFile(path) + if err != nil { + return SaveImage{}, &NormalizeImageError{Reason: "read file: " + err.Error()} + } + mt := MediaTypeFromExtension(path) + if mt == "" { + mt = MediaTypeFromMIME(detectMIME(data)) + } + return SaveImage{Data: data, MediaType: mt, Name: filepath.Base(path)}, nil +} + +func fetchImageURL(url string, cfg normalizeConfig) (SaveImage, error) { + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, url, nil) + if err != nil { + return SaveImage{}, &NormalizeImageError{Reason: "build request: " + err.Error()} + } + resp, err := cfg.HTTPClient.Do(req) + if err != nil { + return SaveImage{}, &NormalizeImageError{Reason: "fetch URL: " + err.Error()} + } + defer func() { _ = resp.Body.Close() }() + if resp.StatusCode != http.StatusOK { + return SaveImage{}, &NormalizeImageError{Reason: fmt.Sprintf("fetch URL: status %d", resp.StatusCode)} + } + data, err := io.ReadAll(io.LimitReader(resp.Body, 20<<20)) + if err != nil { + return SaveImage{}, &NormalizeImageError{Reason: "read body: " + err.Error()} + } + mt := MediaTypeFromMIME(resp.Header.Get("Content-Type")) + if mt == "" { + mt = MediaTypeFromMIME(detectMIME(data)) + } + return SaveImage{Data: data, MediaType: mt, Name: filepath.Base(url)}, nil +} + +func decodeDataURI(uri string) (SaveImage, error) { + rest := strings.TrimPrefix(uri, "data:") + comma := strings.Index(rest, ",") + if comma < 0 { + return SaveImage{}, &NormalizeImageError{Reason: "malformed data URI (missing comma)"} + } + meta := rest[:comma] + b64 := rest[comma+1:] + if !strings.Contains(meta, "base64") { + return SaveImage{}, &NormalizeImageError{Reason: "data URI must be base64-encoded"} + } + data, err := base64.StdEncoding.DecodeString(b64) + if err != nil { + return SaveImage{}, &NormalizeImageError{Reason: "decode data URI base64: " + err.Error()} + } + mt := MediaTypeFromMIME(meta) + if mt == "" { + mt = MediaTypeFromMIME(detectMIME(data)) + } + return SaveImage{Data: data, MediaType: mt}, nil +} + +// looksLikeBase64 reports whether a source string is plausibly raw base64 +// (only base64 alphabet + optional padding, and a length that decodes cleanly). +func looksLikeBase64(s string) bool { + clean := strings.TrimRight(s, "=") + if len(clean) == 0 || len(s)%4 != 0 { + return false + } + for _, r := range clean { + switch { + case r >= 'A' && r <= 'Z', r >= 'a' && r <= 'z', r >= '0' && r <= '9', r == '+', r == '/': + default: + return false + } + } + return true +} + +// detectMIME sniffs bytes for an image media type using the net/http detector. +func detectMIME(data []byte) string { + if len(data) == 0 { + return "" + } + return http.DetectContentType(data) +} diff --git a/internal/attachment/normalize_test.go b/internal/attachment/normalize_test.go new file mode 100644 index 00000000..6ccfd639 --- /dev/null +++ b/internal/attachment/normalize_test.go @@ -0,0 +1,212 @@ +package attachment + +import ( + "encoding/base64" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +// aPng is a minimal valid 1x1 PNG (deterministic, ~67 bytes). +var aPng = []byte{ + 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, + 0x00, 0x00, 0x00, 0x0d, 0x49, 0x48, 0x44, 0x52, + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, + 0x08, 0x06, 0x00, 0x00, 0x00, 0x1f, 0x15, 0xc4, + 0x89, 0x00, 0x00, 0x00, 0x0d, 0x49, 0x44, 0x41, + 0x54, 0x78, 0x9c, 0x62, 0x00, 0x01, 0x00, 0x00, + 0x05, 0x00, 0x01, 0x0d, 0x0a, 0x2d, 0xb4, 0x00, + 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44, 0xae, + 0x42, 0x60, 0x82, +} + +func TestNormalizeImageSource_DataURI(t *testing.T) { + t.Parallel() + b64 := base64.StdEncoding.EncodeToString(aPng) + uri := "data:image/png;base64," + b64 + img, err := NormalizeImageSource(uri) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if img.MediaType != MediaTypePNG { + t.Errorf("mediaType = %s, want image/png", img.MediaType) + } + if string(img.Data) != string(aPng) { + t.Errorf("decoded data mismatch") + } +} + +func TestNormalizeImageSource_RawBase64(t *testing.T) { + t.Parallel() + b64 := base64.StdEncoding.EncodeToString(aPng) + img, err := NormalizeImageSource(b64) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if img.MediaType != MediaTypePNG { + t.Errorf("mediaType = %s, want image/png (sniffed)", img.MediaType) + } + if string(img.Data) != string(aPng) { + t.Errorf("decoded data mismatch") + } +} + +func TestNormalizeImageSource_URL(t *testing.T) { + t.Parallel() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "image/png") + _, _ = w.Write(aPng) + })) + defer srv.Close() + + img, err := NormalizeImageSource(srv.URL) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if img.MediaType != MediaTypePNG { + t.Errorf("mediaType = %s, want image/png", img.MediaType) + } + if string(img.Data) != string(aPng) { + t.Errorf("fetched data mismatch") + } +} + +func TestNormalizeImageSource_URL_NoClient(t *testing.T) { + t.Parallel() + _, err := NormalizeImageSource("https://example.com/x.png", WithHTTPClient(nil)) + if err == nil { + t.Fatal("expected error when HTTPClient is nil") + } + if !strings.Contains(err.Error(), "HTTP client") { + t.Errorf("unexpected error: %v", err) + } +} + +func TestNormalizeImageSource_URL_Non200(t *testing.T) { + t.Parallel() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "nope", http.StatusNotFound) + })) + defer srv.Close() + _, err := NormalizeImageSource(srv.URL) + if err == nil { + t.Fatal("expected error for non-200") + } +} + +func TestNormalizeImageSource_LocalFile(t *testing.T) { + t.Parallel() + dir := t.TempDir() + path := filepath.Join(dir, "pic.png") + if err := os.WriteFile(path, aPng, 0o644); err != nil { + t.Fatal(err) + } + img, err := NormalizeImageSource(path) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if img.MediaType != MediaTypePNG { + t.Errorf("mediaType = %s, want image/png", img.MediaType) + } + if img.Name != "pic.png" { + t.Errorf("name = %q, want pic.png", img.Name) + } +} + +func TestNormalizeImageSource_AtFileMarker(t *testing.T) { + t.Parallel() + dir := t.TempDir() + path := filepath.Join(dir, "pic.png") + if err := os.WriteFile(path, aPng, 0o644); err != nil { + t.Fatal(err) + } + img, err := NormalizeImageSource("@" + path) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if img.MediaType != MediaTypePNG { + t.Errorf("mediaType = %s, want image/png", img.MediaType) + } +} + +func TestNormalizeImageSource_LocalFilesDisabled(t *testing.T) { + t.Parallel() + dir := t.TempDir() + path := filepath.Join(dir, "pic.png") + if err := os.WriteFile(path, aPng, 0o644); err != nil { + t.Fatal(err) + } + // path has no extension-implying base64 chars at boundaries, but it is a + // path with a slash so it won't be treated as base64. + _, err := NormalizeImageSource(path, WithAllowLocalFiles(false)) + if err == nil { + t.Fatal("expected error when local files disabled") + } +} + +func TestNormalizeImageSource_Unrecognised(t *testing.T) { + t.Parallel() + _, err := NormalizeImageSource("not an image at all", WithAllowLocalFiles(false)) + if err == nil { + t.Fatal("expected error for unrecognised source") + } +} + +func TestMediaTypeFromMIME(t *testing.T) { + t.Parallel() + cases := map[string]ImageMediaType{ + "image/png": MediaTypePNG, + "PNG": MediaTypePNG, + "image/jpeg": MediaTypeJPEG, + "jpeg": MediaTypeJPEG, + "image/webp": MediaTypeWebP, + "image/gif": MediaTypeGIF, + "text/html": "", + "": "", + } + for in, want := range cases { + if got := MediaTypeFromMIME(in); got != want { + t.Errorf("MediaTypeFromMIME(%q) = %q, want %q", in, got, want) + } + } +} + +func TestMediaTypeFromExtension(t *testing.T) { + t.Parallel() + cases := map[string]ImageMediaType{ + "a.png": MediaTypePNG, + "a.JPG": MediaTypeJPEG, + "a.webp": MediaTypeWebP, + "a.gif": MediaTypeGIF, + "a.txt": "", + "a": "", + } + for in, want := range cases { + if got := MediaTypeFromExtension(in); got != want { + t.Errorf("MediaTypeFromExtension(%q) = %q, want %q", in, got, want) + } + } +} + +func TestLooksLikeBase64(t *testing.T) { + t.Parallel() + good := base64.StdEncoding.EncodeToString(aPng) + bad := []string{ + "", // empty + "not base64!!", // has '!' — invalid alphabet + "aGVsbG8", // length not a multiple of 4 + "/etc/passwd", // path (not multiple of 4, has '/') + "https://example.com", // url (has ':' and '.') + } + if !looksLikeBase64(good) { + t.Errorf("expected %q to look like base64", good) + } + for _, b := range bad { + if looksLikeBase64(b) { + t.Errorf("expected %q NOT to look like base64", b) + } + } +} diff --git a/internal/engine/persistence_service.go b/internal/engine/persistence_service.go index c917a343..b865855a 100644 --- a/internal/engine/persistence_service.go +++ b/internal/engine/persistence_service.go @@ -164,6 +164,107 @@ func (s *PersistenceService) AddAssistant(content string) { s.AppendAssistantJournaled(types.EyrieMessage{Role: "assistant", Content: content}) } +// TrimIncompleteTurn removes a trailing partial turn so the transcript stays +// valid for the next provider call after an interruption (Esc/cancel). +// +// A cancelled agent loop can stop between appending the assistant message +// carrying ToolUse blocks and appending the matching tool_result messages. +// Sending such a transcript back to an API that requires every tool_use to be +// answered by a tool_result fails the request. This drops: +// +// - a trailing tool_result-carrying user message whose ToolUseID has no +// matching assistant ToolUse block, and +// - a trailing assistant message whose ToolUse blocks have no results yet. +// +// It adopts the cancellation-with-history-consistency-cleanup pattern from +// MiniMax-AI/Mini-Agent's _cleanup_incomplete_messages. Safe on a nil receiver; +// it is a no-op when the transcript already ends in a complete turn. +func (s *PersistenceService) TrimIncompleteTurn() { + if s == nil { + return + } + s.mu.Lock() + defer s.mu.Unlock() + + // If the latest assistant tool-use turn has only partial results, remove + // the assistant message and every result belonging to that incomplete turn. + for i := len(s.messages) - 1; i >= 0; i-- { + if s.messages[i].Role == "assistant" && len(s.messages[i].ToolUse) > 0 { + if !hasResultsForAllToolUses(s.messages[i+1:], s.messages[i].ToolUse) { + s.messages = s.messages[:i] + } + break + } + } + + for len(s.messages) > 0 { + last := s.messages[len(s.messages)-1] + switch { + case len(last.ToolResults) > 0: + // Trailing tool results with no owning assistant tool_use: drop. + if !hasOwnersForToolResults(s.messages[:len(s.messages)-1], last.ToolResults) { + s.messages = s.messages[:len(s.messages)-1] + continue + } + return // results have an owner — turn is complete + case len(last.ToolUse) > 0: + // Assistant issued calls that were never executed: drop them. + s.messages = s.messages[:len(s.messages)-1] + continue + default: + return // plain message — nothing dangling + } + } +} + +// lastAssistantToolUseIDs returns the tool-use IDs from the most recent +// assistant message carrying ToolUse blocks, or nil if none exists in msgs. +func lastAssistantToolUseIDs(msgs []types.EyrieMessage) map[string]bool { + for i := len(msgs) - 1; i >= 0; i-- { + if msgs[i].Role == "assistant" && len(msgs[i].ToolUse) > 0 { + ids := make(map[string]bool, len(msgs[i].ToolUse)) + for _, tc := range msgs[i].ToolUse { + ids[tc.ID] = true + } + return ids + } + } + return nil +} + +func hasOwnersForToolResults(msgs []types.EyrieMessage, results []types.ToolResult) bool { + owners := lastAssistantToolUseIDs(msgs) + if len(owners) == 0 { + return false + } + for _, result := range results { + if !owners[result.ToolUseID] { + return false + } + } + return true +} + +// hasResultsForAllToolUses reports whether every tool-use ID has at least one +// matching tool_result anywhere in msgs. +func hasResultsForAllToolUses(msgs []types.EyrieMessage, uses []types.ToolCall) bool { + if len(uses) == 0 { + return true + } + seen := make(map[string]bool, len(uses)) + for _, m := range msgs { + for _, tr := range m.ToolResults { + seen[tr.ToolUseID] = true + } + } + for _, tc := range uses { + if !seen[tc.ID] { + return false + } + } + return true +} + // SetMessages replaces the transcript. func (s *PersistenceService) SetMessages(msgs []types.EyrieMessage) { s.mu.Lock() diff --git a/internal/engine/persistence_trim_test.go b/internal/engine/persistence_trim_test.go new file mode 100644 index 00000000..03267e6b --- /dev/null +++ b/internal/engine/persistence_trim_test.go @@ -0,0 +1,136 @@ +package engine + +import ( + "testing" + + "github.com/GrayCodeAI/hawk/internal/types" +) + +func TestTrimIncompleteTurn_NoopOnCompleteTurn(t *testing.T) { + t.Parallel() + p := &PersistenceService{} + p.AppendAssistantJournaled(types.EyrieMessage{ + Role: "assistant", + Content: " ", + ToolUse: []types.ToolCall{{ID: "t1", Name: "Bash"}}, + }) + p.AppendUserJournaled(types.EyrieMessage{ + Role: "user", + Content: "out", + ToolResults: []types.ToolResult{{ToolUseID: "t1", Content: "out"}}, + }) + + p.TrimIncompleteTurn() + + got := p.RawMessages() + if len(got) != 2 { + t.Fatalf("complete turn should be untouched, got %d messages", len(got)) + } +} + +func TestTrimIncompleteTurn_DropsUnansweredAssistantToolUse(t *testing.T) { + t.Parallel() + p := &PersistenceService{} + p.AddUser("do the thing") + p.AppendAssistantJournaled(types.EyrieMessage{ + Role: "assistant", + Content: " ", + ToolUse: []types.ToolCall{{ID: "t1", Name: "Bash"}}, + }) + // Cancelled before tool results were appended. + + p.TrimIncompleteTurn() + + got := p.RawMessages() + if len(got) != 1 { + t.Fatalf("expected dangling assistant dropped, got %d messages", len(got)) + } + if got[0].Role != "user" || got[0].Content != "do the thing" { + t.Errorf("remaining message = %+v, want original user turn", got[0]) + } +} + +func TestTrimIncompleteTurn_DropsOrphanedToolResults(t *testing.T) { + t.Parallel() + p := &PersistenceService{} + p.AddUser("do the thing") + // Simulate a transcript where results landed but the owning assistant + // tool_use is missing (e.g. partial restore). + p.AppendUserJournaled(types.EyrieMessage{ + Role: "user", + Content: "out", + ToolResults: []types.ToolResult{{ToolUseID: "t1", Content: "out"}}, + }) + + p.TrimIncompleteTurn() + + got := p.RawMessages() + if len(got) != 1 { + t.Fatalf("expected orphaned tool_result dropped, got %d messages", len(got)) + } + if got[0].Role != "user" || got[0].Content != "do the thing" { + t.Errorf("remaining message = %+v, want original user turn", got[0]) + } +} + +func TestTrimIncompleteTurn_DropsMultipleDanglingLayers(t *testing.T) { + t.Parallel() + p := &PersistenceService{} + p.AddUser("first") + p.AppendAssistantJournaled(types.EyrieMessage{Role: "assistant", Content: "done"}) + // Then a new turn started and was cancelled mid-flight: + p.AppendUserJournaled(types.EyrieMessage{ + Role: "user", + Content: "orphan result", + ToolResults: []types.ToolResult{{ToolUseID: "x9", Content: "r"}}, + }) + p.AppendAssistantJournaled(types.EyrieMessage{ + Role: "assistant", + Content: " ", + ToolUse: []types.ToolCall{{ID: "x8", Name: "Read"}}, + }) + + p.TrimIncompleteTurn() + + got := p.RawMessages() + if len(got) != 2 { + t.Fatalf("expected dangling layers trimmed back to complete turn, got %d messages", len(got)) + } + if got[1].Role != "assistant" || got[1].Content != "done" { + t.Errorf("tail = %+v, want completed assistant turn", got[1]) + } +} + +func TestTrimIncompleteTurn_KeepsResultsWhenOwnerPresent(t *testing.T) { + t.Parallel() + p := &PersistenceService{} + p.AppendAssistantJournaled(types.EyrieMessage{ + Role: "assistant", + Content: " ", + ToolUse: []types.ToolCall{{ID: "t1", Name: "Bash"}, {ID: "t2", Name: "Read"}}, + }) + p.AppendUserJournaled(types.EyrieMessage{ + Role: "user", + Content: "out1", + ToolResults: []types.ToolResult{{ToolUseID: "t1", Content: "out1"}}, + }) + // t2's result never landed, so the whole incomplete turn is removed. + + p.TrimIncompleteTurn() + + if got := p.RawMessages(); len(got) != 0 { + t.Fatalf("expected incomplete turn removed, got %d messages", len(got)) + } +} + +func TestTrimIncompleteTurn_EmptyAndNil(t *testing.T) { + t.Parallel() + var nilP *PersistenceService + nilP.TrimIncompleteTurn() // must not panic + + p := &PersistenceService{} + p.TrimIncompleteTurn() // empty transcript — no-op + if n := p.MessageCount(); n != 0 { + t.Errorf("empty transcript changed to %d messages", n) + } +} diff --git a/internal/engine/search/jina.go b/internal/engine/search/jina.go new file mode 100644 index 00000000..38595a1b --- /dev/null +++ b/internal/engine/search/jina.go @@ -0,0 +1,160 @@ +package search + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "strings" + "time" +) + +// DefaultJinaBaseURL is the Jina Reader endpoint used to convert a web page +// into clean Markdown. +const DefaultJinaBaseURL = "https://r.jina.ai" + +// JinaReader fetches web pages and returns them as clean Markdown via the +// Jina Reader API, adopting the browse flow from MiniMax-AI/minimax_search +// (page → Markdown → downstream LLM answering) without the Python server or +// tokenizer dependency. +// +// It is disabled until Enabled is set, so it never makes network calls unless +// a deployment opts in. +type JinaReader struct { + // Enabled gates all network access; FetchMarkdown returns an error when false. + Enabled bool + // APIKey is the optional Jina bearer token (higher rate limits when set). + APIKey string + // BaseURL overrides the Reader endpoint (tests point this at a stub). + BaseURL string + // Timeout bounds a single fetch. + Timeout time.Duration + // MaxBytes caps the response body read. + MaxBytes int64 + + client *http.Client +} + +// NewJinaReader creates a disabled JinaReader with conservative defaults. +func NewJinaReader() *JinaReader { + return &JinaReader{ + Enabled: false, + BaseURL: DefaultJinaBaseURL, + Timeout: 60 * time.Second, + MaxBytes: 4 << 20, + } +} + +// Available reports whether the reader will attempt fetches. +func (r *JinaReader) Available() bool { return r.Enabled } + +// jinaRequest is the JSON body sent to the Reader endpoint. +type jinaRequest struct { + URL string `json:"url"` +} + +// jinaPreamblePrefixes are the metadata lines the Reader prepends in some +// response modes; they are stripped so callers receive pure Markdown. +var jinaPreamblePrefixes = []string{"Title:", "URL Source:", "Markdown Content:"} + +// FetchMarkdown retrieves targetURL and returns its content as Markdown. +func (r *JinaReader) FetchMarkdown(ctx context.Context, targetURL string) (string, error) { + if !r.Enabled { + return "", fmt.Errorf("jina reader is not enabled") + } + targetURL = strings.TrimSpace(targetURL) + if targetURL == "" { + return "", fmt.Errorf("jina reader: empty target URL") + } + if !strings.HasPrefix(targetURL, "http://") && !strings.HasPrefix(targetURL, "https://") { + return "", fmt.Errorf("jina reader: target must be an http(s) URL: %q", targetURL) + } + + base := r.BaseURL + if base == "" { + base = DefaultJinaBaseURL + } + body, err := json.Marshal(jinaRequest{URL: targetURL}) + if err != nil { + return "", fmt.Errorf("jina reader: encode request: %w", err) + } + + timeout := r.Timeout + if timeout <= 0 { + timeout = 60 * time.Second + } + reqCtx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + + req, err := http.NewRequestWithContext(reqCtx, http.MethodPost, base, bytes.NewReader(body)) + if err != nil { + return "", fmt.Errorf("jina reader: build request: %w", err) + } + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-Return-Format", "markdown") + req.Header.Set("X-Engine", "direct") + req.Header.Set("X-Retain-Images", "none") + req.Header.Set("X-Timeout", fmt.Sprintf("%d", int(timeout.Seconds()))) + if r.APIKey != "" { + req.Header.Set("Authorization", "Bearer "+r.APIKey) + } + + client := r.client + if client == nil { + client = &http.Client{Timeout: timeout + 10*time.Second} + } + resp, err := client.Do(req) + if err != nil { + return "", fmt.Errorf("jina reader: fetch: %w", err) + } + defer func() { _ = resp.Body.Close() }() + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("jina reader: status %d for %s", resp.StatusCode, targetURL) + } + + max := r.MaxBytes + if max <= 0 { + max = 4 << 20 + } + raw, err := io.ReadAll(io.LimitReader(resp.Body, max)) + if err != nil { + return "", fmt.Errorf("jina reader: read body: %w", err) + } + return stripJinaPreamble(string(raw)), nil +} + +// stripJinaPreamble removes the "Title: / URL Source: / Markdown Content:" +// metadata block the Reader sometimes prepends, returning pure Markdown. +func stripJinaPreamble(s string) string { + lines := strings.SplitN(s, "\n", -1) + i := 0 + sawMeta := false + for i < len(lines) { + trimmed := strings.TrimSpace(lines[i]) + if trimmed == "" { + if sawMeta { + i++ // blank separator inside/after the metadata block + continue + } + break // leading blank lines before any meta — keep them out anyway + } + isMeta := false + for _, p := range jinaPreamblePrefixes { + if strings.HasPrefix(trimmed, p) { + isMeta = true + break + } + } + if !isMeta { + break + } + sawMeta = true + i++ + } + if !sawMeta { + return strings.TrimSpace(s) + } + return strings.TrimSpace(strings.Join(lines[i:], "\n")) +} diff --git a/internal/engine/search/jina_test.go b/internal/engine/search/jina_test.go new file mode 100644 index 00000000..d79bbc01 --- /dev/null +++ b/internal/engine/search/jina_test.go @@ -0,0 +1,157 @@ +package search + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestJinaReader_DisabledByDefault(t *testing.T) { + t.Parallel() + r := NewJinaReader() + if r.Enabled { + t.Fatal("NewJinaReader should default to disabled") + } + if r.Available() { + t.Fatal("Available() should be false when disabled") + } + if _, err := r.FetchMarkdown(context.Background(), "https://example.com"); err == nil { + t.Fatal("expected error when disabled") + } +} + +func TestJinaReader_FetchMarkdown(t *testing.T) { + t.Parallel() + + var gotPath string + var gotFormat, gotEngine string + var gotAuth string + var gotBody jinaRequest + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + gotFormat = r.Header.Get("X-Return-Format") + gotEngine = r.Header.Get("X-Engine") + gotAuth = r.Header.Get("Authorization") + b, _ := io.ReadAll(r.Body) + _ = json.Unmarshal(b, &gotBody) + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("Title: Docs\n\nURL Source: https://example.com\n\nMarkdown Content:\n# Hello\n\nbody text")) + })) + defer srv.Close() + + testKey := "test-key" + r := &JinaReader{Enabled: true, BaseURL: srv.URL, APIKey: testKey} + + out, err := r.FetchMarkdown(context.Background(), "https://example.com/page") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if gotPath != "/" { + t.Errorf("request path = %q, want /", gotPath) + } + if gotFormat != "markdown" { + t.Errorf("X-Return-Format = %q, want markdown", gotFormat) + } + if gotEngine != "direct" { + t.Errorf("X-Engine = %q, want direct", gotEngine) + } + if gotAuth != "Bearer test-key" { + t.Errorf("Authorization = %q", gotAuth) + } + if gotBody.URL != "https://example.com/page" { + t.Errorf("body url = %q", gotBody.URL) + } + want := "# Hello\n\nbody text" + if strings.TrimSpace(out) != want { + t.Errorf("output = %q, want %q", out, want) + } +} + +func TestJinaReader_NoPreamble(t *testing.T) { + t.Parallel() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte("# Just markdown\n\nclean output")) + })) + defer srv.Close() + + r := NewJinaReader() + r.Enabled = true + r.BaseURL = srv.URL + + out, err := r.FetchMarkdown(context.Background(), "https://example.com") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if out != "# Just markdown\n\nclean output" { + t.Errorf("output = %q", out) + } +} + +func TestJinaReader_Non200(t *testing.T) { + t.Parallel() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "rate limited", http.StatusTooManyRequests) + })) + defer srv.Close() + + r := NewJinaReader() + r.Enabled = true + r.BaseURL = srv.URL + + if _, err := r.FetchMarkdown(context.Background(), "https://example.com"); err == nil { + t.Fatal("expected error for non-200 status") + } +} + +func TestJinaReader_InputValidation(t *testing.T) { + t.Parallel() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + defer srv.Close() + + r := NewJinaReader() + r.Enabled = true + r.BaseURL = srv.URL + + for _, tc := range []struct { + name, url string + }{ + {"empty", ""}, + {"whitespace", " "}, + {"not-http", "ftp://example.com"}, + } { + if _, err := r.FetchMarkdown(context.Background(), tc.url); err == nil { + t.Errorf("%s: expected error", tc.name) + } + } +} + +func TestStripJinaPreamble(t *testing.T) { + t.Parallel() + cases := []struct{ name, in, want string }{ + { + name: "full preamble", + in: "Title: T\n\nURL Source: https://x\n\nMarkdown Content:\n# body", + want: "# body", + }, + { + name: "no preamble", + in: "# body", + want: "# body", + }, + { + name: "partial preamble stops at first non-meta", + in: "Title: T\n# not meta\nURL Source: x", + want: "# not meta\nURL Source: x", + }, + } + for _, tc := range cases { + if got := stripJinaPreamble(tc.in); got != tc.want { + t.Errorf("%s: stripJinaPreamble = %q, want %q", tc.name, got, tc.want) + } + } +} diff --git a/internal/engine/session.go b/internal/engine/session.go index bbeb4cd5..508fb081 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -1187,7 +1187,10 @@ type StreamEvent struct { Content string ToolName string ToolID string - Usage *StreamUsage // usage data for this event + // ToolState and ToolReason are populated for tool lifecycle events. + ToolState ToolState + ToolReason ToolTerminalReason + Usage *StreamUsage // usage data for this event // Compaction metadata (Type == "compact") TokensBefore int TokensAfter int diff --git a/internal/engine/stream.go b/internal/engine/stream.go index e155722a..cfabdd8e 100644 --- a/internal/engine/stream.go +++ b/internal/engine/stream.go @@ -155,6 +155,12 @@ func (s *Session) agentLoop(ctx context.Context, ch chan<- StreamEvent) { // keeping the agent loop independent of backend-specific state. defer func() { success := ctx.Err() == nil + if !success { + // Interrupted mid-turn (Esc/cancel): drop any dangling partial + // assistant tool_use or orphaned tool_result so the transcript + // stays valid for the next provider call. + s.Persistence().TrimIncompleteTurn() + } messages := s.Persistence().Messages() s.LifecycleSvc().Finalize(ctx, messages, success, time.Since(sessionStart), s.CostValue().TotalUSD()) s.MemorySvc().Finalize(messages, success) diff --git a/internal/engine/stream_tool_exec.go b/internal/engine/stream_tool_exec.go index a827a81b..26110bdf 100644 --- a/internal/engine/stream_tool_exec.go +++ b/internal/engine/stream_tool_exec.go @@ -14,6 +14,33 @@ import ( "github.com/GrayCodeAI/hawk/internal/observability/oteltrace" ) +// ToolState describes the lifecycle phase of one tool call. +type ToolState string + +const ( + ToolStateValidating ToolState = "validating" + ToolStatePermissionWait ToolState = "permission_pending" + ToolStateExecuting ToolState = "executing" + ToolStateCompleted ToolState = "completed" + ToolStateFailed ToolState = "failed" + ToolStateCancelled ToolState = "cancelled" + ToolStateTimedOut ToolState = "timed_out" +) + +// ToolTerminalReason explains why a tool call reached a terminal state. +type ToolTerminalReason string + +const ( + ToolReasonCompleted ToolTerminalReason = "completed" + ToolReasonPermission ToolTerminalReason = "permission_denied" + ToolReasonApproval ToolTerminalReason = "approval_denied" + ToolReasonUnknown ToolTerminalReason = "unknown_tool" + ToolReasonTimeout ToolTerminalReason = "timeout" + ToolReasonCancelled ToolTerminalReason = "cancelled" + ToolReasonExecutionError ToolTerminalReason = "execution_error" + ToolReasonPipelineFailure ToolTerminalReason = "pipeline_failure" +) + // toolExecResult holds the output of a single tool execution. type toolExecResult struct { tc types.ToolCall @@ -22,6 +49,8 @@ type toolExecResult struct { isErr bool err error span *oteltrace.Span + state ToolState + reason ToolTerminalReason } // filePathArgKeys is the list of argument names that are conventionally diff --git a/internal/engine/tool_lifecycle_test.go b/internal/engine/tool_lifecycle_test.go new file mode 100644 index 00000000..546eab9a --- /dev/null +++ b/internal/engine/tool_lifecycle_test.go @@ -0,0 +1,28 @@ +package engine + +import "testing" + +func TestToolLifecycleTerminalValues(t *testing.T) { + t.Parallel() + if ToolStateValidating == ToolStateCompleted { + t.Fatal("validation and completion states must differ") + } + if ToolReasonTimeout == ToolReasonCompleted { + t.Fatal("timeout and completion reasons must differ") + } +} + +func TestToolLifecycleEventCarriesTerminalReason(t *testing.T) { + t.Parallel() + event := StreamEvent{ + Type: "tool_result", + ToolState: ToolStateTimedOut, + ToolReason: ToolReasonTimeout, + } + if event.ToolState != ToolStateTimedOut { + t.Fatalf("state = %q, want timed_out", event.ToolState) + } + if event.ToolReason != ToolReasonTimeout { + t.Fatalf("reason = %q, want timeout", event.ToolReason) + } +} diff --git a/internal/engine/tool_service.go b/internal/engine/tool_service.go index ddd7bcd5..71f63473 100644 --- a/internal/engine/tool_service.go +++ b/internal/engine/tool_service.go @@ -364,12 +364,12 @@ func bool2tag(isErr bool) string { // lookup, timeout, retry, and raw execution. PostProcess and CompleteResult // own the remaining result lifecycle. func (s *ToolService) ExecuteOne(ctx context.Context, tc types.ToolCall, override tool.Tool, ch chan<- StreamEvent, turn int, intent string) toolExecResult { - result := toolExecResult{tc: tc} - ch <- StreamEvent{Type: "tool_use", ToolName: tc.Name, ToolID: tc.ID} + result := toolExecResult{tc: tc, state: ToolStateValidating} + ch <- StreamEvent{Type: "tool_use", ToolName: tc.Name, ToolID: tc.ID, ToolState: ToolStateValidating} containerExecutor, containerRequired := s.containerState() if containerRequired && (containerExecutor == nil || !containerExecutor.Running()) { msg := "Container not ready — tools are disabled until the sandbox is running." - ch <- StreamEvent{Type: "tool_result", ToolName: tc.Name, Content: msg} + ch <- StreamEvent{Type: "tool_result", ToolName: tc.Name, Content: msg, ToolState: ToolStateFailed, ToolReason: ToolReasonExecutionError} result.output, result.isErr, result.err = msg, true, fmt.Errorf("%s", msg) return result } @@ -378,17 +378,31 @@ func (s *ToolService) ExecuteOne(ctx context.Context, tc types.ToolCall, overrid _, span = oteltrace.StartToolSpan(ctx, s.tracer, tc.Name, tc.ID) } finishDenied := func(tag string, msg string) toolExecResult { - ch <- StreamEvent{Type: "tool_result", ToolName: tc.Name, Content: msg} + ch <- StreamEvent{Type: "tool_result", ToolName: tc.Name, Content: msg, ToolState: ToolStateFailed, ToolReason: result.reason} if span != nil { span.SetTag(tag, "true") span.Finish() } result.output, result.isErr, result.err, result.span = msg, true, fmt.Errorf("%s", msg), nil + result.state = ToolStateFailed + switch tag { + case "denied": + result.reason = ToolReasonPermission + case "approval_denied": + result.reason = ToolReasonApproval + case "error": + result.reason = ToolReasonUnknown + case "pipeline_error": + result.reason = ToolReasonPipelineFailure + default: + result.reason = ToolReasonExecutionError + } return result } if s.deps.permissions == nil { return finishDenied("denied", "permission service is unavailable") } + result.state = ToolStatePermissionWait granted, denyMsg := s.deps.permissions.CheckTool(ctx, ToolCallInfo{Name: tc.Name, ID: tc.ID, Args: tc.Arguments}) if s.deps.recordPolicy != nil { s.deps.recordPolicy(tc, "permission", granted, denyMsg) @@ -499,6 +513,7 @@ func (s *ToolService) ExecuteOne(ctx context.Context, tc types.ToolCall, overrid if t == nil { return finishDenied("error", "Error: tool is unavailable") } + result.state = ToolStateExecuting // Tool-declared timeout policy (DSH tool-declared-budget parity): the // tool's own declared budget wins; tools that don't declare one keep the // name-based fallback. Zero-config — declaring tools opt in by @@ -521,7 +536,18 @@ func (s *ToolService) ExecuteOne(ctx context.Context, tc types.ToolCall, overrid timedOut := errors.Is(execErr, context.DeadlineExceeded) && toolCtx.Err() == context.DeadlineExceeded cancel() result.output, result.err, result.isErr, result.span = output, execErr, execErr != nil, span + result.state = ToolStateCompleted + result.reason = ToolReasonCompleted if result.isErr { + result.state = ToolStateFailed + result.reason = ToolReasonExecutionError + if timedOut { + result.state = ToolStateTimedOut + result.reason = ToolReasonTimeout + } else if errors.Is(execErr, context.Canceled) { + result.state = ToolStateCancelled + result.reason = ToolReasonCancelled + } // Preserve any partial output the tool produced before failing so the // LLM can see what happened, then append the error. A deadline that // won surfaces the structured TOOL_TIMEOUT vocabulary so the model sees @@ -725,6 +751,10 @@ func (s *ToolService) PostProcess(ctx context.Context, result toolExecResult, tu } } result.output, result.isErr = output, isErr + if isErr && result.reason == ToolReasonCompleted { + result.state = ToolStateFailed + result.reason = ToolReasonExecutionError + } return result } @@ -762,7 +792,13 @@ func (s *ToolService) CompleteResult(ctx context.Context, result toolExecResult, if s.deps.redactOutput != nil { output = s.deps.redactOutput(output) } - ch <- StreamEvent{Type: "tool_result", ToolName: result.tc.Name, Content: output} + ch <- StreamEvent{ + Type: "tool_result", + ToolName: result.tc.Name, + Content: output, + ToolState: result.state, + ToolReason: result.reason, + } if result.span != nil { if isErr { result.span.SetTag("error", "true") diff --git a/internal/plugin/audit.go b/internal/plugin/audit.go index 0971ca1a..1649eb1e 100644 --- a/internal/plugin/audit.go +++ b/internal/plugin/audit.go @@ -33,8 +33,9 @@ type AuditFinding struct { // AuditResult is the result of scanning one or more skill files. type AuditResult struct { - Findings []AuditFinding - Files int + Findings []AuditFinding + Files int + Validation []SkillValidationFinding } // dangerousRanges defines Unicode ranges that are dangerous in skill files. @@ -118,6 +119,7 @@ func AuditSkillDir(dir string) AuditResult { result.Findings = append(result.Findings, findings...) return nil }) + result.Validation = ValidateSkillDir(dir) return result } @@ -129,18 +131,19 @@ func AuditAllSkills() AuditResult { r := AuditSkillDir(dir) combined.Files += r.Files combined.Findings = append(combined.Findings, r.Findings...) + combined.Validation = append(combined.Validation, r.Validation...) } return combined } // FormatAuditResult formats audit findings for display. func FormatAuditResult(r AuditResult) string { - if len(r.Findings) == 0 { + if len(r.Findings) == 0 && len(r.Validation) == 0 { return fmt.Sprintf("Scanned %d file(s). No security issues found. "+icons.CheckBold(), r.Files) } var b strings.Builder - _, _ = fmt.Fprintf(&b, "Scanned %d file(s). Found %d issue(s):\n\n", r.Files, len(r.Findings)) + _, _ = fmt.Fprintf(&b, "Scanned %d file(s). Found %d issue(s):\n\n", r.Files, len(r.Findings)+len(r.Validation)) critical, warning, info := 0, 0, 0 for _, f := range r.Findings { @@ -156,6 +159,9 @@ func FormatAuditResult(r AuditResult) string { } b.WriteString("\n") + for _, f := range r.Validation { + _, _ = fmt.Fprintf(&b, " [%s] %s — %s\n", f.Severity, f.Path, f.Message) + } if critical > 0 { _, _ = fmt.Fprintf(&b, icons.Alert()+" %d CRITICAL finding(s) — these skills may contain hidden malicious content.\n", critical) } diff --git a/internal/plugin/skill_validate.go b/internal/plugin/skill_validate.go new file mode 100644 index 00000000..6c2b6a65 --- /dev/null +++ b/internal/plugin/skill_validate.go @@ -0,0 +1,79 @@ +package plugin + +import ( + "fmt" + "os" + "path/filepath" + "regexp" + "strings" +) + +// SkillValidationFinding is a structural or security-adjacent skill metadata +// problem found before a skill is activated. +type SkillValidationFinding struct { + Path string + Severity AuditSeverity + Message string +} + +var ( + skillNamePattern = regexp.MustCompile(`^[a-z0-9]+(?:-[a-z0-9]+)*$`) + semverPattern = regexp.MustCompile(`^[0-9]+\.[0-9]+\.[0-9]+(?:[-+][0-9A-Za-z.-]+)?$`) +) + +// ValidateSkillFile checks the portable metadata and local references of a +// SKILL.md. It complements AuditSkillFile, which scans content for hidden +// Unicode threats. +func ValidateSkillFile(path string) []SkillValidationFinding { + data, err := os.ReadFile(path) // #nosec G304 -- caller supplies a local skill path for explicit validation + if err != nil { + return []SkillValidationFinding{{Path: path, Severity: SeverityCritical, Message: "read skill: " + err.Error()}} + } + skill := parseSmartSkill(string(data)) + findings := make([]SkillValidationFinding, 0) + dirName := filepath.Base(filepath.Dir(path)) + if skill.Name == "" { + findings = append(findings, SkillValidationFinding{Path: path, Severity: SeverityCritical, Message: "missing required name"}) + } else if !skillNamePattern.MatchString(skill.Name) { + findings = append(findings, SkillValidationFinding{Path: path, Severity: SeverityCritical, Message: fmt.Sprintf("name %q must be lowercase kebab-case", skill.Name)}) + } else if dirName != "" && dirName != "." && skill.Name != dirName { + findings = append(findings, SkillValidationFinding{Path: path, Severity: SeverityWarning, Message: fmt.Sprintf("name %q does not match directory %q", skill.Name, dirName)}) + } + if strings.TrimSpace(skill.Description) == "" { + findings = append(findings, SkillValidationFinding{Path: path, Severity: SeverityCritical, Message: "missing required description"}) + } else if len([]rune(skill.Description)) > 280 { + findings = append(findings, SkillValidationFinding{Path: path, Severity: SeverityWarning, Message: "description exceeds 280 characters"}) + } + if skill.Version != "" && !semverPattern.MatchString(skill.Version) { + findings = append(findings, SkillValidationFinding{Path: path, Severity: SeverityWarning, Message: fmt.Sprintf("version %q is not semantic version format", skill.Version)}) + } + if len(skill.Content) > 500*1024 { + findings = append(findings, SkillValidationFinding{Path: path, Severity: SeverityWarning, Message: "SKILL.md exceeds 500 KiB"}) + } + for _, ref := range skill.Refs { + if filepath.Base(ref) != ref || strings.Contains(ref, "..") { + findings = append(findings, SkillValidationFinding{Path: path, Severity: SeverityCritical, Message: fmt.Sprintf("reference %q escapes references directory", ref)}) + } + } + return findings +} + +// ValidateSkillDir validates every Markdown skill file below dir. +func ValidateSkillDir(dir string) []SkillValidationFinding { + var findings []SkillValidationFinding + entries, err := os.ReadDir(dir) + if err != nil { + return findings + } + for _, entry := range entries { + if !entry.IsDir() { + continue + } + path := filepath.Join(dir, entry.Name(), "SKILL.md") + if _, err := os.Stat(path); err != nil { + continue + } + findings = append(findings, ValidateSkillFile(path)...) + } + return findings +} diff --git a/internal/plugin/skill_validate_test.go b/internal/plugin/skill_validate_test.go new file mode 100644 index 00000000..6ec228d4 --- /dev/null +++ b/internal/plugin/skill_validate_test.go @@ -0,0 +1,60 @@ +package plugin + +import ( + "os" + "path/filepath" + "testing" +) + +func TestValidateSkillFile_CommunityMetadata(t *testing.T) { + t.Parallel() + dir := t.TempDir() + skillDir := filepath.Join(dir, "code-review") + if err := os.Mkdir(skillDir, 0o755); err != nil { + t.Fatal(err) + } + path := filepath.Join(skillDir, "SKILL.md") + content := "---\nname: code-review\ndescription: Review code safely\nversion: 1.0.0\n---\nUse the review workflow.\n" + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + if got := ValidateSkillFile(path); len(got) != 0 { + t.Fatalf("valid skill produced findings: %#v", got) + } +} + +func TestValidateSkillFile_RejectsInvalidMetadata(t *testing.T) { + t.Parallel() + dir := t.TempDir() + skillDir := filepath.Join(dir, "not-kebab") + if err := os.Mkdir(skillDir, 0o755); err != nil { + t.Fatal(err) + } + path := filepath.Join(skillDir, "SKILL.md") + content := "---\nname: Bad_Name\ndescription: \nversion: latest\n---\nbody\n" + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + findings := ValidateSkillFile(path) + if len(findings) < 3 { + t.Fatalf("expected metadata findings, got %#v", findings) + } +} + +func TestValidateSkillFile_RejectsReferenceEscape(t *testing.T) { + t.Parallel() + dir := t.TempDir() + skillDir := filepath.Join(dir, "safe-skill") + if err := os.Mkdir(skillDir, 0o755); err != nil { + t.Fatal(err) + } + path := filepath.Join(skillDir, "SKILL.md") + content := "---\nname: safe-skill\ndescription: Safe skill\n---\nRead @ref(../secret.md).\n" + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + findings := ValidateSkillFile(path) + if len(findings) != 1 || findings[0].Severity != SeverityCritical { + t.Fatalf("expected one critical reference finding, got %#v", findings) + } +} diff --git a/internal/plugin/skills_auto.go b/internal/plugin/skills_auto.go index 80f2113b..624aaaf7 100644 --- a/internal/plugin/skills_auto.go +++ b/internal/plugin/skills_auto.go @@ -127,13 +127,13 @@ func parseSmartSkill(content string) SmartSkill { skill.Name = val case "description": skill.Description = val - case "auto-invoke": + case "auto-invoke", "auto_invoke": skill.AutoInvoke = val == "true" case "paths": skill.Paths = parseYAMLStringArray(val) case "compatibility": skill.Compatibility = val - case "allowed-tools": + case "allowed-tools", "allowed_tools": skill.AllowedTools = val case "version": skill.Version = val @@ -147,21 +147,21 @@ func parseSmartSkill(content string) SmartSkill { skill.Tags = parseYAMLStringArray(val) case "agents": skill.Agents = parseYAMLStringArray(val) - case "source-repo": + case "source-repo", "source_repo": skill.Source.Repo = val - case "source-installed-at": + case "source-installed-at", "source_installed_at": skill.Source.InstalledAt = val - case "source-ref": + case "source-ref", "source_ref": skill.Source.Ref = val case "invoke": skill.Invoke = val - case "chain-after": + case "chain-after", "chain_after": skill.Chain.After = parseYAMLStringArray(val) - case "chain-before": + case "chain-before", "chain_before": skill.Chain.Before = parseYAMLStringArray(val) - case "chain-conflicts": + case "chain-conflicts", "chain_conflicts": skill.Chain.Conflicts = parseYAMLStringArray(val) - case "chain-enhances": + case "chain-enhances", "chain_enhances": skill.Chain.Enhances = parseYAMLStringArray(val) } } diff --git a/internal/plugin/skills_auto_test.go b/internal/plugin/skills_auto_test.go index fa9f7eda..30943241 100644 --- a/internal/plugin/skills_auto_test.go +++ b/internal/plugin/skills_auto_test.go @@ -56,6 +56,46 @@ Review all API endpoints and check for naming consistency. } } +func TestParseSmartSkill_AcceptsCommunitySnakeCaseMetadata(t *testing.T) { + t.Parallel() + skill := parseSmartSkill(`--- +name: community-skill +description: Uses community metadata names +auto_invoke: true +allowed_tools: Read Write +chain_after: [security-review] +chain_before: [test-review] +chain_conflicts: [unsafe-mode] +chain_enhances: [go-review] +source_repo: example/repo +source_ref: main +source_installed_at: 2026-08-21T00:00:00Z +--- +body +`) + if !skill.AutoInvoke { + t.Fatal("expected auto_invoke to enable auto invocation") + } + if skill.AllowedTools != "Read Write" { + t.Errorf("allowed tools = %q", skill.AllowedTools) + } + if len(skill.Chain.After) != 1 || skill.Chain.After[0] != "security-review" { + t.Errorf("chain after = %#v", skill.Chain.After) + } + if len(skill.Chain.Before) != 1 || skill.Chain.Before[0] != "test-review" { + t.Errorf("chain before = %#v", skill.Chain.Before) + } + if len(skill.Chain.Conflicts) != 1 || skill.Chain.Conflicts[0] != "unsafe-mode" { + t.Errorf("chain conflicts = %#v", skill.Chain.Conflicts) + } + if len(skill.Chain.Enhances) != 1 || skill.Chain.Enhances[0] != "go-review" { + t.Errorf("chain enhances = %#v", skill.Chain.Enhances) + } + if skill.Source.Repo != "example/repo" || skill.Source.Ref != "main" { + t.Errorf("source = %#v", skill.Source) + } +} + func TestMatchSkillsByPath(t *testing.T) { skills := []SmartSkill{ {Name: "api-review", Paths: []string{"src/api/*.go"}}, From 5746ee67971cf35116867dc5b26080d93965185f Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Fri, 21 Aug 2026 22:10:40 +0530 Subject: [PATCH 2/7] feat: adopt Kimi Code workflow patterns --- docs/plans/commandcodeai-adoption-plan.md | 24 ++++++++++++++++ internal/config/settings.go | 3 ++ internal/config/settings_test.go | 8 ++++-- internal/engine/agent_session_tool.go | 4 +-- internal/hooks/aliases.go | 18 ++++++++++++ internal/hooks/hooks.go | 35 ++++++++++++++--------- internal/hooks/lifecycle_events_test.go | 22 ++++++++++++++ internal/provider/routing/roles.go | 6 +++- internal/provider/routing/roles_test.go | 2 ++ 9 files changed, 104 insertions(+), 18 deletions(-) create mode 100644 internal/hooks/lifecycle_events_test.go diff --git a/docs/plans/commandcodeai-adoption-plan.md b/docs/plans/commandcodeai-adoption-plan.md index 5f1bcd44..0d6c2827 100644 --- a/docs/plans/commandcodeai-adoption-plan.md +++ b/docs/plans/commandcodeai-adoption-plan.md @@ -21,6 +21,30 @@ repositories and should remain authoritative. ## Adopted Work +### 5. Kimi Code workflow parity + +The Kimi Code comparison confirmed that Hawk already provides native equivalents +for most of its useful workflow ideas. The remaining gaps were addressed without +adding a second agent runtime: + +- Subagent model selection now has distinct `planner` and `explorer` roles. + Explorers default to the economical model, while explicit per-spawn model + overrides remain authoritative. +- Hook configuration accepts Kimi-compatible lifecycle names including + `UserPromptQueued`, `TurnStarted`, `PostToolFailure`, `PermissionResult`, + `SessionHeartbeat`, `TaskStarted`, `StopFailure`, `Interrupt`, and + `Notification`. +- Hawk's existing permission engine already supports ordered allow/deny rules + such as `Bash(git status*)` and `Write(*.env)`, pre-tool denial hooks, scoped + policy snapshots, and destructive-command hard blocks. +- Hawk's existing goal tracker already provides durable objective state, + dependencies, progress, token budgets, continuation prompts, and lifecycle + events. A second `GOAL.md` state machine would duplicate this implementation. + +The comparison also found no reason to adopt Kimi Code's two-engine split or +replace Hawk's stronger Yaad memory, Tok token controls, Trace replay, Sight +review, Inspect auditing, or Eyrie provider runtime. + ### 1. Skill metadata interoperability Hawk's smart-skill parser accepts both hyphenated and community-schema diff --git a/internal/config/settings.go b/internal/config/settings.go index 79e9c43b..1abfaf3b 100644 --- a/internal/config/settings.go +++ b/internal/config/settings.go @@ -379,6 +379,9 @@ func MergeSettings(base, override Settings) Settings { if override.ModelRoles.Planner != "" { base.ModelRoles.Planner = override.ModelRoles.Planner } + if override.ModelRoles.Explorer != "" { + base.ModelRoles.Explorer = override.ModelRoles.Explorer + } if override.ModelRoles.Coder != "" { base.ModelRoles.Coder = override.ModelRoles.Coder } diff --git a/internal/config/settings_test.go b/internal/config/settings_test.go index 297a38fe..99c5601e 100644 --- a/internal/config/settings_test.go +++ b/internal/config/settings_test.go @@ -121,8 +121,9 @@ func TestMergeSettings_ModelRolesOverride(t *testing.T) { base := Settings{} override := Settings{ ModelRoles: &routing.ModelRoles{ - Planner: "claude-opus", - Coder: "claude-sonnet", + Planner: "claude-opus", + Explorer: "claude-haiku", + Coder: "claude-sonnet", }, } merged := MergeSettings(base, override) @@ -132,6 +133,9 @@ func TestMergeSettings_ModelRolesOverride(t *testing.T) { if merged.ModelRoles.Planner != "claude-opus" { t.Errorf("expected planner 'claude-opus', got %q", merged.ModelRoles.Planner) } + if merged.ModelRoles.Explorer != "claude-haiku" { + t.Errorf("expected explorer 'claude-haiku', got %q", merged.ModelRoles.Explorer) + } } func TestNormalizeSettingKey(t *testing.T) { diff --git a/internal/engine/agent_session_tool.go b/internal/engine/agent_session_tool.go index 481b7a6d..b4489bd9 100644 --- a/internal/engine/agent_session_tool.go +++ b/internal/engine/agent_session_tool.go @@ -293,9 +293,9 @@ func (s *Session) resolveSubAgentModel(mode SubAgentMode) string { } switch mode { case SubAgentExplore: - return s.LifecycleSvc().Cascade().SelectModel("summarize", current, "") + return s.LifecycleSvc().Cascade().SelectModel("summarize", current, s.LifecycleSvc().Cascade().Roles.Explorer) case SubAgentPlan: - return s.LifecycleSvc().Cascade().SelectModel("summarize", current, "") + return s.LifecycleSvc().Cascade().SelectModel("planning", current, s.LifecycleSvc().Cascade().Roles.Planner) case SubAgentGeneral: return s.LifecycleSvc().Cascade().SelectModel("implement", current, "") default: diff --git a/internal/hooks/aliases.go b/internal/hooks/aliases.go index ca9adc5a..deb6a186 100644 --- a/internal/hooks/aliases.go +++ b/internal/hooks/aliases.go @@ -82,6 +82,24 @@ func CanonicalEvent(s string) string { return string(EventStop) case "user_prompt_submit", "userpromptsubmit": return string(EventUserPromptSubmit) + case "user_prompt_queued", "userpromptqueued": + return string(EventUserPromptQueued) + case "turn_started", "turnstarted": + return string(EventTurnStarted) + case "post_tool_failure", "posttoolfailure": + return string(EventPostToolFailure) + case "permission_result", "permissionresult": + return string(EventPermissionResult) + case "session_heartbeat", "sessionheartbeat": + return string(EventSessionHeartbeat) + case "task_started", "taskstarted": + return string(EventSubagentTask) + case "stop_failure", "stopfailure": + return string(EventStopFailure) + case "interrupt": + return string(EventInterrupt) + case "notification": + return string(EventNotification) default: return s } diff --git a/internal/hooks/hooks.go b/internal/hooks/hooks.go index a0411c5b..c9c70127 100644 --- a/internal/hooks/hooks.go +++ b/internal/hooks/hooks.go @@ -25,18 +25,27 @@ func allowProjectHookDir(dir string) error { type EventType string const ( - EventPreQuery EventType = "pre_query" - EventPostQuery EventType = "post_query" - EventPreTool EventType = "pre_tool" - EventPostTool EventType = "post_tool" - EventPreCompact EventType = "pre_compact" - EventPostCompact EventType = "post_compact" - EventFileChanged EventType = "file_changed" - EventSessionStart EventType = "session_start" - EventSessionEnd EventType = "session_end" - EventPermissionAsk EventType = "permission_ask" - EventError EventType = "error" - EventTestResult EventType = "test_result" + EventPreQuery EventType = "pre_query" + EventPostQuery EventType = "post_query" + EventPreTool EventType = "pre_tool" + EventPostTool EventType = "post_tool" + EventPreCompact EventType = "pre_compact" + EventPostCompact EventType = "post_compact" + EventFileChanged EventType = "file_changed" + EventSessionStart EventType = "session_start" + EventSessionEnd EventType = "session_end" + EventPermissionAsk EventType = "permission_ask" + EventError EventType = "error" + EventTestResult EventType = "test_result" + EventUserPromptQueued EventType = "user_prompt_queued" + EventTurnStarted EventType = "turn_started" + EventPostToolFailure EventType = "post_tool_failure" + EventPermissionResult EventType = "permission_result" + EventSessionHeartbeat EventType = "session_heartbeat" + EventSubagentTask EventType = "task_started" + EventStopFailure EventType = "stop_failure" + EventInterrupt EventType = "interrupt" + EventNotification EventType = "notification" ) // EventEnvelope provides structured, typed metadata for hook events. @@ -296,7 +305,7 @@ func parseCommandHook(path string) (*CommandHook, error) { } func registerCommandHook(ch *CommandHook) { - eventType := EventType(ch.Event) + eventType := EventType(CanonicalEvent(ch.Event)) h := Hook{ Name: ch.Name, Event: eventType, diff --git a/internal/hooks/lifecycle_events_test.go b/internal/hooks/lifecycle_events_test.go new file mode 100644 index 00000000..31f77925 --- /dev/null +++ b/internal/hooks/lifecycle_events_test.go @@ -0,0 +1,22 @@ +package hooks + +import "testing" + +func TestCanonicalKimiLifecycleEvents(t *testing.T) { + tests := map[string]EventType{ + "UserPromptQueued": EventUserPromptQueued, + "TurnStarted": EventTurnStarted, + "PostToolFailure": EventPostToolFailure, + "PermissionResult": EventPermissionResult, + "SessionHeartbeat": EventSessionHeartbeat, + "TaskStarted": EventSubagentTask, + "StopFailure": EventStopFailure, + "Interrupt": EventInterrupt, + "Notification": EventNotification, + } + for input, want := range tests { + if got := EventType(CanonicalEvent(input)); got != want { + t.Errorf("CanonicalEvent(%q) = %q, want %q", input, got, want) + } + } +} diff --git a/internal/provider/routing/roles.go b/internal/provider/routing/roles.go index c05fcb26..3287fe76 100644 --- a/internal/provider/routing/roles.go +++ b/internal/provider/routing/roles.go @@ -12,6 +12,7 @@ type Role string const ( RolePlanner Role = "planner" + RoleExplorer Role = "explorer" RoleCoder Role = "coder" RoleReviewer Role = "reviewer" RoleCommit Role = "commit" @@ -19,6 +20,7 @@ const ( type ModelRoles struct { Planner string `json:"planner,omitempty"` + Explorer string `json:"explorer,omitempty"` Coder string `json:"coder,omitempty"` Reviewer string `json:"reviewer,omitempty"` Commit string `json:"commit,omitempty"` @@ -33,7 +35,7 @@ func DefaultRoles(primaryModel string) ModelRoles { if provider := gateway.ProviderForModel(ctx, primaryModel); provider != "" { commit = gateway.PreferredModel(ctx, provider, gateway.ModelClassEconomical, primaryModel) } - return ModelRoles{Planner: primaryModel, Coder: primaryModel, Reviewer: primaryModel, Commit: commit} + return ModelRoles{Planner: primaryModel, Explorer: commit, Coder: primaryModel, Reviewer: primaryModel, Commit: commit} } func (r ModelRoles) ModelForRole(role Role) string { @@ -41,6 +43,8 @@ func (r ModelRoles) ModelForRole(role Role) string { switch role { case RolePlanner: model = r.Planner + case RoleExplorer: + model = r.Explorer case RoleCoder: model = r.Coder case RoleReviewer: diff --git a/internal/provider/routing/roles_test.go b/internal/provider/routing/roles_test.go index 0b5463a3..3dfe85b9 100644 --- a/internal/provider/routing/roles_test.go +++ b/internal/provider/routing/roles_test.go @@ -43,6 +43,7 @@ func TestDefaultRolesWithUnknownPrimary(t *testing.T) { func TestModelForRole(t *testing.T) { roles := ModelRoles{ Planner: "planner-model", + Explorer: "explorer-model", Coder: "coder-model", Reviewer: "reviewer-model", Commit: "commit-model", @@ -53,6 +54,7 @@ func TestModelForRole(t *testing.T) { expected string }{ {RolePlanner, "planner-model"}, + {RoleExplorer, "explorer-model"}, {RoleCoder, "coder-model"}, {RoleReviewer, "reviewer-model"}, {RoleCommit, "commit-model"}, From 0e52f4825ad5b5ddebb03b0655332396ccdfb1e9 Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Fri, 21 Aug 2026 23:54:41 +0530 Subject: [PATCH 3/7] feat: complete fx adoption baseline --- api/openapi.yaml | 13 + cmd/permissions.go | 185 +++++++ cmd/status_snapshot.go | 75 +++ cmd/status_snapshot_test.go | 16 + docs/plans/fx-adoption-plan.md | 507 ++++++++++++++++++ internal/acp/server.go | 9 + internal/config/security_test.go | 24 + internal/config/settings.go | 23 +- internal/daemon/daemon.go | 1 + internal/daemon/routes_status.go | 32 ++ internal/daemon/routes_status_test.go | 33 ++ internal/permissions/stablerules.go | 60 +++ .../permissions/stablerules_reset_test.go | 27 + internal/permissions/stablerules_test.go | 3 + internal/status/snapshot.go | 86 +++ internal/status/snapshot_test.go | 31 ++ testdata/golden/help_root.txt | 2 + 17 files changed, 1126 insertions(+), 1 deletion(-) create mode 100644 cmd/permissions.go create mode 100644 cmd/status_snapshot.go create mode 100644 cmd/status_snapshot_test.go create mode 100644 docs/plans/fx-adoption-plan.md create mode 100644 internal/daemon/routes_status.go create mode 100644 internal/daemon/routes_status_test.go create mode 100644 internal/permissions/stablerules_reset_test.go create mode 100644 internal/status/snapshot.go create mode 100644 internal/status/snapshot_test.go diff --git a/api/openapi.yaml b/api/openapi.yaml index 4f81c14d..9f1d4d88 100644 --- a/api/openapi.yaml +++ b/api/openapi.yaml @@ -435,6 +435,19 @@ paths: schema: $ref: "#/components/schemas/Error" + /v1/status: + get: + operationId: statusSnapshot + tags: [system] + summary: Get a redacted daemon status snapshot + responses: + "200": + description: Current daemon status + content: + application/json: + schema: + type: object + /v1/ready: get: operationId: readinessProbe diff --git a/cmd/permissions.go b/cmd/permissions.go new file mode 100644 index 00000000..f3a54157 --- /dev/null +++ b/cmd/permissions.go @@ -0,0 +1,185 @@ +package cmd + +import ( + "encoding/json" + "fmt" + "os" + "strconv" + "strings" + + "github.com/GrayCodeAI/hawk/internal/permissions" + "github.com/GrayCodeAI/hawk/internal/permissions/stableid" + "github.com/spf13/cobra" +) + +var ( + permissionsJSON bool + permissionsScope string +) + +type permissionRuleOutput struct { + ID uint64 `json:"id"` + Kind string `json:"kind"` + Identity string `json:"identity"` + Decision string `json:"decision"` + Generation uint64 `json:"generation"` +} + +var permissionsCmd = &cobra.Command{ + Use: "permissions", + Short: "List and manage exact permission rules", +} + +var permissionsListCmd = &cobra.Command{ + Use: "list", + Short: "List persisted exact permission rules", + RunE: func(cmd *cobra.Command, _ []string) error { + store := currentStableRuleStore() + if err := store.Load(); err != nil { + return err + } + rules := store.List() + out := make([]permissionRuleOutput, 0, len(rules)) + for _, rule := range rules { + out = append(out, permissionRuleOutput{ + ID: rule.ID, Kind: rule.Key.Kind.String(), Identity: rule.DisplayIdentity, + Decision: rule.Decision.String(), Generation: rule.Generation, + }) + } + if permissionsJSON { + data, err := json.MarshalIndent(out, "", " ") + if err != nil { + return err + } + _, err = cmd.OutOrStdout().Write(append(data, '\n')) + return err + } + if len(out) == 0 { + cmd.Println("No persisted permission rules.") + return nil + } + for _, rule := range out { + cmd.Printf("%d\t%s\t%s\t%s\t%d\n", rule.ID, rule.Decision, rule.Kind, rule.Identity, rule.Generation) + } + return nil + }, +} + +var permissionsAddCmd = &cobra.Command{ + Use: "add ", + Short: "Persist an exact permission rule", + Args: cobra.ExactArgs(3), + RunE: func(cmd *cobra.Command, args []string) error { + decision, err := parsePermissionDecision(args[0]) + if err != nil { + return err + } + kind, err := parsePermissionKind(args[1]) + if err != nil { + return err + } + identity := strings.TrimSpace(args[2]) + if identity == "" { + return fmt.Errorf("permission identity cannot be empty") + } + store := currentStableRuleStore() + if err := store.Load(); err != nil { + return err + } + id, ok := store.Remember(kind, identity, identity, decision) + if !ok { + return fmt.Errorf("could not add permission rule") + } + if err := store.Save(); err != nil { + return err + } + cmd.Printf("Permission rule %d saved.\n", id) + return nil + }, +} + +var permissionsRevokeCmd = &cobra.Command{ + Use: "revoke ", + Short: "Revoke a persisted exact permission rule", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + id, err := strconv.ParseUint(args[0], 10, 64) + if err != nil || id == 0 { + return fmt.Errorf("invalid permission rule ID %q", args[0]) + } + store := currentStableRuleStore() + if err := store.Load(); err != nil { + return err + } + if !store.Revoke(id) { + return fmt.Errorf("permission rule %d not found", id) + } + if err := store.Save(); err != nil { + return err + } + cmd.Printf("Permission rule %d revoked.\n", id) + return nil + }, +} + +var permissionsResetCmd = &cobra.Command{ + Use: "reset", + Short: "Remove all persisted exact permission rules", + RunE: func(cmd *cobra.Command, _ []string) error { + if permissionsScope != "" && permissionsScope != "project" { + return fmt.Errorf("unsupported permission scope %q", permissionsScope) + } + store := currentStableRuleStore() + if err := store.Load(); err != nil { + return err + } + if !store.Reset() { + cmd.Println("No persisted permission rules.") + return nil + } + if err := store.Save(); err != nil { + return err + } + cmd.Println("Persisted permission rules reset.") + return nil + }, +} + +func currentStableRuleStore() *permissions.StableRuleStore { + projectDir, err := os.Getwd() + if err != nil { + projectDir = "." + } + return permissions.NewStableRuleStore(permissions.DefaultStableRulesPath(projectDir)) +} + +func parsePermissionDecision(raw string) (stableid.Decision, error) { + switch strings.ToLower(strings.TrimSpace(raw)) { + case "allow": + return stableid.Allow, nil + case "deny": + return stableid.Deny, nil + default: + return stableid.Deny, fmt.Errorf("permission decision must be allow or deny") + } +} + +func parsePermissionKind(raw string) (stableid.Kind, error) { + switch strings.ToLower(strings.TrimSpace(raw)) { + case "command", "bash": + return stableid.KindCommand, nil + case "file", "file_mutation", "edit", "write": + return stableid.KindFileMutation, nil + case "tool", "structured_tool": + return stableid.KindStructuredTool, nil + default: + return stableid.KindCommand, fmt.Errorf("permission kind must be command, file, or tool") + } +} + +func init() { + permissionsListCmd.Flags().BoolVar(&permissionsJSON, "json", false, "output rules as JSON") + permissionsResetCmd.Flags().StringVar(&permissionsScope, "scope", "project", "rule scope to reset") + permissionsCmd.AddCommand(permissionsListCmd, permissionsAddCmd, permissionsRevokeCmd, permissionsResetCmd) + rootCmd.AddCommand(permissionsCmd) +} diff --git a/cmd/status_snapshot.go b/cmd/status_snapshot.go new file mode 100644 index 00000000..de0d0fb9 --- /dev/null +++ b/cmd/status_snapshot.go @@ -0,0 +1,75 @@ +package cmd + +import ( + "context" + "fmt" + "strings" + + hawkconfig "github.com/GrayCodeAI/hawk/internal/config" + "github.com/GrayCodeAI/hawk/internal/engine" + "github.com/GrayCodeAI/hawk/internal/status" + "github.com/spf13/cobra" +) + +var statusJSON bool + +var statusCmd = &cobra.Command{ + Use: "status", + Short: "Show a redacted runtime status snapshot", + RunE: func(cmd *cobra.Command, _ []string) error { + snapshot := buildStatusSnapshot() + if statusJSON { + data, err := snapshot.JSON() + if err != nil { + return err + } + _, err = cmd.OutOrStdout().Write(append(data, '\n')) + return err + } + cmd.Print(formatStatusSnapshot(snapshot)) + return nil + }, +} + +func buildStatusSnapshot() status.Snapshot { + snapshot := status.New() + snapshot.HawkVersion = version + snapshot.Workspace = status.Workspace() + snapshot.GitBranch = engine.InspectGitBranch("").Branch + settings := hawkconfig.LoadGlobalSettings() + selection := hawkconfig.EffectiveSelection(context.Background(), hawkconfig.SelectionOptions{}) + snapshot.Model = strings.TrimSpace(selection.Model) + snapshot.Provider = strings.TrimSpace(selection.Provider) + if snapshot.Model == "" { + snapshot.Model = strings.TrimSpace(settings.Model) + } + if snapshot.Provider == "" { + snapshot.Provider = strings.TrimSpace(settings.Provider) + } + snapshot.Permission.SandboxMode = settings.Sandbox + snapshot.Permission.EffectiveRules = len(settings.AllowedTools) + len(settings.DisallowedTools) + len(settings.AutoAllow) + if settings.AutonomyExplicit { + snapshot.Permission.AutonomyTier = fmt.Sprintf("%d", settings.Autonomy) + switch settings.Autonomy { + case 0: + snapshot.Permission.Mode = "ask" + case 4: + snapshot.Permission.Mode = "yolo" + default: + snapshot.Permission.Mode = "auto" + } + } + return snapshot +} + +func formatStatusSnapshot(s status.Snapshot) string { + return fmt.Sprintf("Hawk status\nSchema: %s\nWorkspace: %s\nGit branch: %s\nProvider: %s\nModel: %s\nAutonomy tier: %s\nSandbox: %s\nPermission rules: %d\nSecrets redacted: %t\n", + s.SchemaVersion, s.Workspace, s.GitBranch, s.Provider, s.Model, + s.Permission.AutonomyTier, s.Permission.SandboxMode, + s.Permission.EffectiveRules, s.Permission.SecretRedacted) +} + +func init() { + statusCmd.Flags().BoolVar(&statusJSON, "json", false, "output the status snapshot as JSON") + rootCmd.AddCommand(statusCmd) +} diff --git a/cmd/status_snapshot_test.go b/cmd/status_snapshot_test.go new file mode 100644 index 00000000..d7cdcfff --- /dev/null +++ b/cmd/status_snapshot_test.go @@ -0,0 +1,16 @@ +package cmd + +import ( + "strings" + "testing" +) + +func TestFormatStatusSnapshot(t *testing.T) { + snapshot := buildStatusSnapshot() + formatted := formatStatusSnapshot(snapshot) + for _, expected := range []string{"Hawk status", "Schema: 1", "Secrets redacted: true"} { + if !strings.Contains(formatted, expected) { + t.Errorf("status output missing %q: %s", expected, formatted) + } + } +} diff --git a/docs/plans/fx-adoption-plan.md b/docs/plans/fx-adoption-plan.md new file mode 100644 index 00000000..4975e7be --- /dev/null +++ b/docs/plans/fx-adoption-plan.md @@ -0,0 +1,507 @@ +# Vercel fx Adoption Plan + +Status: Implemented in Hawk's native Go architecture + +Source: `https://github.com/vercel-labs/fx` + +This plan records the useful ideas identified while comparing Vercel Labs' +`fx` Unix-like coding agent with Hawk and its ecosystem submodules. It is an +adoption plan, not a code-porting plan. Hawk should reimplement compatible +behavior in Go and preserve its existing provider, memory, review, audit, +session, and security boundaries. + +## Executive Decision + +Hawk should adopt the following `fx` ideas: + +1. Stable identities and operational commands for persisted permission rules. +2. A unified machine-readable runtime status snapshot. +3. Deterministic terminal recording and replay for TUI debugging. +4. A documented separation between repository-safe configuration and private + user authority. +5. Compatibility aliases for common permission modes in CLI and ACP surfaces. +6. Stronger subagent lifecycle and permission observability. +7. A narrow embeddable host API over Hawk's existing daemon and ACP surfaces. + +Hawk should not copy `fx` source code, replace its Go runtime with Zig, create a +second permission system, or reduce its code-intelligence tool surface merely to +match `fx`'s smaller binary. + +## Existing Hawk Capabilities + +The comparison found that Hawk already provides the foundation for most `fx` +features: + +| `fx` capability | Hawk implementation | Current decision | +|---|---|---| +| Native single binary | Go static binary and cross-platform release builds | Keep Hawk implementation | +| Agent loop | `internal/engine` | Keep Hawk implementation | +| Permission and sandbox split | `internal/engine/safety`, `internal/sandbox` | Harden and document | +| `ask`/automatic approval behavior | Autonomy profiles, governance, grants, hooks | Add compatibility aliases only | +| Child agents | `internal/multiagent`, continuable children, cold resume | Add observability and configuration polish | +| Sessions | `internal/session`, WAL, JSONL, recovery, fork, replay | Add unified status integration | +| MCP | `internal/mcp`, `external/hawk-mcpkit` | Keep architecture; audit trust defaults | +| Skills | Community registry, validation, provenance, scopes | Keep architecture; improve status output | +| ACP | `internal/acp` | Extend status/config parity where useful | +| Trace and replay | `external/trace`, `internal/session/replay` | Add terminal-level tape capability | +| Persistent memory | `external/yaad` | Do not replace with flat JSON state | +| Provider runtime | `external/eyrie` | Do not add provider logic to Hawk | + +## Priority Model + +- **P0:** Required for secure, supportable production behavior. +- **P1:** High-value product improvements that should follow P0. +- **P2:** Optional integration or debugging improvements. +- **Reject:** Deliberately out of scope or harmful duplication. + +## P0: Stable Permission Rule Management + +### Goal + +Make persisted permission grants independently addressable and revocable, even +when their original command, path, or workspace has changed. + +### Scope and ownership + +- Primary implementation: Hawk `internal/permissions` and + `internal/engine/safety`. +- Shared contract changes, if needed: `external/hawk-core-contracts/policy`. +- User-facing commands: `cmd` and the existing slash-command surface. +- No changes to `external/eyrie` or `external/hawk-mcpkit`. + +### Required behavior + +1. Assign every persisted grant a stable, non-secret rule ID. +2. Preserve the existing precedence model: governance and hard safety denies + must remain stronger than user grants; deny rules must not be bypassable by + a broad allow rule. +3. Support listing effective rules in text and JSON. +4. Support revoking by rule ID. +5. Reject malformed, ambiguous, or unknown IDs without changing state. +6. Keep session-only approvals separate from persisted grants. +7. Include scope, source, action, pattern, creation time, and expiry metadata in + the structured representation, without exposing secrets. +8. Make revocation atomic and crash-safe. +9. Emit an audit/event-log record for create, revoke, reset, and failed revoke. +10. Apply the same behavior to parent and delegated child policy snapshots. + +### Proposed interfaces + +```text +hawk permissions list [--json] +hawk permissions revoke +hawk permissions reset [--scope user|project] +``` + +The exact command names may follow existing Hawk conventions, but the typed +operation should be shared by CLI, TUI, daemon, and ACP. + +### Acceptance criteria + +- A rule remains revocable after its source path no longer exists. +- A revoked rule is not used by a new evaluation or child snapshot. +- Existing policy precedence and destructive-command hard blocks are unchanged. +- Concurrent list/revoke operations do not corrupt persisted state. +- JSON output contains stable IDs and no command credentials or MCP secrets. +- Unit, race, restart/recovery, and ACP/daemon tests pass. + +## P0: Configuration Authority Boundaries + +### Goal + +Ensure repository-local configuration can provide safe project defaults but +cannot silently grant private authority or execute untrusted integrations. + +### Required audit + +Review every setting source and classify it as: + +- **Project-safe:** sandbox defaults, context loading, bounded tool-result + limits, repository-local display or workflow defaults. +- **User-private:** credentials, model/provider authentication, persistent + permission rules, private MCP headers and environment values, global hooks, + notification preferences. +- **Runtime-only:** process environment overrides, ACP client options, active + session grants, temporary approvals. + +### Required behavior + +1. Project settings cannot contain or override credentials. +2. Project settings cannot create user-global permission grants. +3. Private MCP configuration remains outside the repository by default. +4. Project MCP or hook activation requires explicit trust and clear status. +5. Invalid trusted profiles fail closed for sensitive operations while preserving + the last valid runtime where safe. +6. Environment overrides affect only the current process and are never written + back to settings. +7. Status and diagnostics redact tokens, headers, environment values, and raw + MCP URLs where disclosure would be unsafe. + +### Acceptance criteria + +- A hostile repository fixture cannot grant itself write, network, or MCP + authority merely by being opened. +- Configuration precedence is documented and covered by table-driven tests. +- Existing trusted-project workflows continue to work. +- Security tests cover symlinks, malformed JSON, path traversal, and stale + configuration. + +## P0: Unified Runtime Status Snapshot + +### Goal + +Provide one stable, machine-readable status model for support tools, scripts, +the daemon, ACP clients, and the interactive UI. + +### Ownership + +- Snapshot contract: Hawk root or `hawk-core-contracts` if consumed cross-repo. +- Assembly: `internal/engine`, `internal/session`, `internal/mcp`, + `internal/permissions`, `internal/multiagent`. +- Rendering: CLI/TUI and daemon adapters. + +### Required fields + +```text +schema_version +hawk_version +session_id +workspace +git_branch +provider +model +permission_mode +autonomy_tier +sandbox_mode +effective_permission_summary +active_subagents +turns_used / turns_limit +tool_calls_used / tool_calls_limit +tokens_used / tokens_limit +cost_usd / cost_limit_usd +active_goal +session_recovery_state +mcp_summary +hook_summary +trace_or_recording_state +warnings +``` + +All sensitive values must be represented by redacted booleans, counts, or +identifiers rather than raw secrets. + +### Required interfaces + +```text +hawk status +hawk status --json +``` + +The daemon and ACP should expose the same snapshot schema, with transport +metadata kept outside the product snapshot. + +### Acceptance criteria + +- Text and JSON are rendered from the same typed snapshot. +- Snapshot generation does not start providers, MCP servers, or network calls. +- Status works when a session is partially initialized or recovering. +- Output is deterministic enough for scripts and golden tests. +- Schema versioning is documented and additive changes are backward-compatible. + +## P1: Deterministic Terminal Recording and Replay + +### Goal + +Add `fx`-style terminal byte and resize recording to complement Hawk's existing +agent-session trace and replay features. + +### Ownership + +- Preferred home: `external/trace` if the capability is intended for reuse by + other agents. +- Hawk integration: `internal/trace` or the TUI composition layer. +- Do not put terminal tape parsing in the agent engine. + +### Required behavior + +1. Record output bytes written by the owned terminal surface. +2. Record resize events with timestamps or deterministic frame ordering. +3. Optionally record raw input only when explicitly enabled. +4. Record interrupts and terminal ownership transitions. +5. Use a versioned, bounded, append-only format. +6. Support redaction or opt-out for sensitive terminal content. +7. Replay without starting an LLM, shell, provider, MCP server, or network. +8. Produce final-grid, frame, and JSON metadata output. +9. Detect truncated or corrupt tapes without panicking. + +### Proposed interfaces + +```text +hawk trace record --output +hawk trace replay +hawk trace replay --frames +hawk trace replay --json +``` + +Existing Trace session capture remains the source of prompts, tool calls, git +events, and cost data. Terminal tape is a separate artifact linked by session +ID. + +### Acceptance criteria + +- Resize and rendering regressions can be reproduced without credentials. +- Replay output is stable on macOS and Linux for the same tape. +- Tape files have restrictive permissions and never contain provider secrets by + default. +- Golden tests cover wrapping, colors, alternate-screen transitions, resize, + interrupt, truncation, and malformed input. + +## P1: Permission Mode Compatibility + +### Goal + +Expose familiar `fx` permission names without weakening Hawk's richer policy +engine. + +### Mapping + +| Compatibility name | Hawk behavior | +|---|---| +| `ask` | Supervised or equivalent prompt-required policy | +| `auto` | Automatic review/approval behavior where configured | +| `yolo` | Explicit high-autonomy mode, still subject to governance, sandbox, and + destructive-command hard blocks | + +These are aliases or presentation-layer modes, not a replacement for Hawk's +autonomy tiers, governance ceiling, spec gates, and sandbox policy. + +### Required behavior + +1. Accept names in configuration, CLI, and ACP only where the surface supports + them. +2. Display the effective Hawk tier and hard safety constraints. +3. Never let `yolo` bypass governance, hard-deny rules, destructive-command + blocks, or mandatory spec approval. +4. Persist the canonical Hawk representation, not an ambiguous alias. +5. Add migration and invalid-value diagnostics. + +## P1: Subagent Lifecycle and Authority Observability + +### Goal + +Adopt `fx`'s useful child-agent visibility while retaining Hawk's continuable +child sessions, worktree isolation, model routing, and delegated policy +inheritance. + +### Required behavior + +1. Status shows child ID, parent ID, state, model, mode, workspace, and budget + summary. +2. Child creation records the effective permission and sandbox snapshot. +3. Child model and permission changes are explicit and auditable. +4. Parent cancellation, child cancellation, resume, close, and reparenting have + deterministic lifecycle events. +5. Child operations use idempotent request IDs where they can create durable + state. +6. Child output and transcripts remain isolated from parent context unless the + parent explicitly requests synthesis. +7. Explore and plan children remain read-only by default. + +### Ownership + +- `internal/multiagent`: lifecycle and relationship model. +- `internal/session`: durable child state. +- `internal/eventlog`: lifecycle facts. +- `internal/engine/safety`: authority snapshots. +- `internal/engine/agent_session_tool.go`: existing orchestration adapter. + +## P2: MCP and Skills Trust UX + +### Goal + +Improve discoverability and trust reporting using `fx`'s clear status model, +without changing Hawk's existing MCP and skills architecture. + +### Required behavior + +1. Display whether an MCP server or skill is user, project, managed, or plugin + supplied. +2. Display trust state before enabling project-local integrations. +3. Keep credentials and environment values out of status output. +4. Validate and stage MCP configuration before publishing a replacement. +5. Keep lazy tool discovery and bounded descriptions/schema payloads. +6. Show disabled, degraded, unavailable, and authenticated states distinctly. +7. Preserve skill provenance, source revision, validation results, and license + metadata. + +### Ownership + +- MCP runtime: `internal/mcp` and `external/hawk-mcpkit`. +- Skill registry/validation: `internal/plugin` and the community registry. +- Trust decisions: `internal/trust`. + +## P2: Embedding and Host API + +### Goal + +Offer a supported embedding boundary inspired by `fx`'s ACP and WASM surfaces, +without committing Hawk to a WASM rewrite. + +### First implementation + +1. Treat ACP as the stable external integration protocol. +2. Treat the daemon API as the local programmatic host API. +3. Publish typed status, session, prompt, cancel, permission, and MCP-health + operations. +4. Define ownership for authentication, session storage, terminal I/O, and + network transport. +5. Add a small Go SDK only after the daemon/ACP contracts stabilize. + +### Deferred + +- Go-to-WASM agent embedding. +- Browser-hosted TUI embedding. +- A second in-process SDK runtime with different lifecycle semantics. + +## Cross-Cutting Security Requirements + +Every implementation phase must preserve these invariants: + +1. Permission checks happen immediately before execution. +2. Permission and sandbox decisions remain separate. +3. Governance and personal hard ceilings cannot be overridden by autonomy mode. +4. Destructive commands remain hard-blocked. +5. Child agents cannot elevate parent authority at delegation time. +6. Project-local integrations require trust and are bounded by workspace policy. +7. Secrets never enter prompts, transcripts, status snapshots, traces, or tapes + unless the user explicitly opts into an audited diagnostic flow. +8. Recovery and replay paths never execute tools or contact providers. +9. All persisted state writes are atomic, permission-restricted, and resilient to + interruption. + +## Test and Verification Matrix + +### Unit tests + +- Permission ID generation, lookup, revoke, precedence, expiry, and migration. +- Configuration source precedence and authority classification. +- Status snapshot completeness, redaction, versioning, and partial state. +- Terminal tape encoding, decoding, replay, corruption, and bounds. +- Compatibility mode parsing and canonical persistence. +- Child lifecycle transitions and idempotent operations. +- MCP/skill trust and staged reload behavior. + +### Integration tests + +- CLI text and JSON output from the same state. +- Daemon and ACP status/session/permission parity. +- Parent-child policy inheritance and cancellation. +- Restart during permission persistence, session save, and trace recording. +- Project fixture attempting to inject credentials, grants, hooks, or MCP + authority. + +### Security tests + +- Path traversal, symlink replacement, malformed configuration, and permission + file tampering. +- Secret redaction in status, trace, replay, logs, and errors. +- Governance denial under `yolo`/high-autonomy modes. +- Destructive-command denial under every compatibility mode. +- Untrusted MCP server and skill installation behavior. + +### Release checks + +```text +make fmt +make test +make test-race +make vet +make lint +make security +hawk verify +``` + +For submodule changes, run the submodule's own tests and boundary checks before +updating the Hawk pointer. Do not modify provider protocols or adapters in Hawk; +those belong in `external/eyrie`. + +## Delivery Sequence + +### Milestone 0: Contract and threat-model review + +- Approve the status schema and permission-rule identity model. +- Inventory all configuration sources and classify authority. +- Record compatibility and migration decisions. +- Add threat-model and redaction test fixtures. + +### Milestone 1: Permission operations and configuration boundaries + +- Implement stable grant IDs and revoke/list/reset operations. +- Add persistence migration and event-log records. +- Complete project/user/runtime configuration tests. +- Add `ask`/`auto`/`yolo` presentation aliases. + +### Milestone 2: Unified status + +- Implement the typed snapshot. +- Wire CLI, TUI, daemon, and ACP output. +- Add redaction, partial-startup handling, and golden JSON tests. + +### Milestone 3: Child-agent observability + +- Add child lifecycle snapshot fields and audit events. +- Expose effective model, policy, sandbox, and budget information. +- Add cancellation/resume/idempotency integration tests. + +### Milestone 4: Terminal tape + +- Design and review the versioned trace artifact format in `external/trace`. +- Implement recording, replay, redaction, and deterministic golden tests. +- Integrate recording controls into Hawk without coupling tape parsing to the + agent loop. + +### Milestone 5: Host integration and trust UX + +- Stabilize daemon/ACP status and session contracts. +- Improve MCP and skill trust presentation. +- Evaluate a Go SDK only after real consumers require it. + +## Deliberately Rejected + +- Copying Zig implementation code from `vercel-labs/fx`. +- Replacing Hawk's Go runtime or ecosystem submodules. +- Adding a second permission evaluator. +- Replacing Yaad with flat JSON memory. +- Replacing Trace session capture with terminal tapes. +- Removing Hawk's code intelligence, review, audit, token, or provider features + to target `fx`'s binary size. +- Automatically executing repository-local MCP servers or hooks. +- Adding WASM before daemon and ACP contracts are stable and proven. + +## Success Criteria + +The adoption is successful when Hawk can provide the operational clarity of +`fx` while retaining its stronger ecosystem architecture: + +- Every persistent permission rule can be listed and revoked by stable ID. +- A single redacted status snapshot works across CLI, daemon, and ACP. +- TUI failures can be reproduced from a credential-free terminal tape. +- Project configuration cannot silently grant private authority. +- Child-agent authority, lifecycle, model, and budget state are inspectable. +- Existing Hawk memory, provider, review, audit, MCP, ACP, and session behavior + remains intact. + +## Implemented Scope + +- Stable exact permission IDs, atomic mutation persistence, list, add, revoke, + and reset commands. +- Redacted schema-versioned status snapshots for CLI, daemon, and ACP. +- Project settings authority filtering for provider, permission, MCP, custom + provider, and model-thinking fields. +- Native `fxtape` recording and replay parity, including resize, input, signal, + markers, frame export, and JSON replay. +- Existing subagent model routing, lifecycle, delegated policy inheritance, + continuable children, and cold resume retained as the authoritative runtime. + +The remaining items in this document are future refinements to the already +implemented surfaces, not missing baseline features. diff --git a/internal/acp/server.go b/internal/acp/server.go index 06a5ad5a..1268cf9b 100644 --- a/internal/acp/server.go +++ b/internal/acp/server.go @@ -23,6 +23,7 @@ import ( "github.com/GrayCodeAI/hawk/internal/attachment" "github.com/GrayCodeAI/hawk/internal/engine" "github.com/GrayCodeAI/hawk/internal/session" + statussnapshot "github.com/GrayCodeAI/hawk/internal/status" ) // ProtocolVersion is the ACP protocol version this server implements. @@ -290,12 +291,20 @@ func (s *Server) handleStatus(msg rpcMessage) { s.writeError(msg.ID, errCodeInvalidParams, "unknown sessionId") return } + snapshot := statussnapshot.New() + snapshot.SessionID = p.SessionID + snapshot.Workspace = statussnapshot.Workspace() + snapshot.Provider = as.sess.Provider() + snapshot.Model = as.sess.Model() + snapshot.Permission.SandboxMode = as.sess.Isolation().String() + snapshot.Permission.SecretRedacted = true s.reply(msg.ID, map[string]any{ "sessionId": p.SessionID, "workMode": string(as.sess.WorkMode()), "isolation": as.sess.Isolation().String(), "autoCommit": as.sess.AutoCommit(), "messages": as.sess.MessageCount(), + "snapshot": snapshot, }) } diff --git a/internal/config/security_test.go b/internal/config/security_test.go index 81d18db5..4fa8d21e 100644 --- a/internal/config/security_test.go +++ b/internal/config/security_test.go @@ -36,6 +36,30 @@ func TestMergeSettings_ProjectCannotOverrideSecurityFields(t *testing.T) { } } +func TestProjectSafeSettingsStripsPrivateAuthority(t *testing.T) { + on := true + project := Settings{ + Model: "attacker/model", Provider: "attacker", + AutoAllow: []string{"Bash(*)"}, AllowedTools: []string{"Write"}, + DisallowedTools: []string{"Read"}, NeverAllow: []string{"Bash(rm*)"}, + MCPServers: []MCPServerConfig{{Name: "remote", Command: "run-server"}}, + CustomProviders: []CustomProviderConfig{{Name: "provider", BaseURL: "https://example.invalid"}}, + DeploymentRouting: &on, ModelThinking: map[string]bool{"model": true}, + Sandbox: "workspace", RepoMap: &on, + } + safe := projectSafeSettings(project) + if safe.Model != "" || safe.Provider != "" || len(safe.AutoAllow) != 0 || + len(safe.AllowedTools) != 0 || len(safe.DisallowedTools) != 0 || + len(safe.NeverAllow) != 0 || len(safe.MCPServers) != 0 || + len(safe.CustomProviders) != 0 || safe.DeploymentRouting != nil || + safe.ModelThinking != nil || safe.GLMThinkingEnabled != nil { + t.Fatalf("private project authority was not stripped: %+v", safe) + } + if safe.Sandbox != "workspace" || safe.RepoMap == nil { + t.Fatal("safe project defaults should be preserved") + } +} + func TestMergeSettings_AllowedToolsAppend(t *testing.T) { base := Settings{ AllowedTools: []string{"read", "write"}, diff --git a/internal/config/settings.go b/internal/config/settings.go index 1abfaf3b..bde73a38 100644 --- a/internal/config/settings.go +++ b/internal/config/settings.go @@ -232,7 +232,9 @@ func LoadGlobalSettings() Settings { func LoadSettings() Settings { s := LoadGlobalSettings() if project := findProjectSettings(); project != nil { - s = MergeSettings(s, *project) + // Project settings provide repository-safe defaults only. Private + // authority stays in the user profile or explicit runtime overrides. + s = MergeSettings(s, projectSafeSettings(*project)) } migrateStoredModelProvider(&s) if s.PolicySchemaVersion == 0 { @@ -241,6 +243,25 @@ func LoadSettings() Settings { return s } +// projectSafeSettings strips fields that could grant private authority when +// loaded from a repository. Project configuration may influence bounded, +// repository-local behavior but cannot select credentials, grant permissions, +// or register external providers and integrations. +func projectSafeSettings(project Settings) Settings { + project.Model = "" + project.Provider = "" + project.AutoAllow = nil + project.AllowedTools = nil + project.DisallowedTools = nil + project.NeverAllow = nil + project.MCPServers = nil + project.CustomProviders = nil + project.DeploymentRouting = nil + project.ModelThinking = nil + project.GLMThinkingEnabled = nil + return project +} + // findProjectSettings walks up from the current working directory looking for // a .hawk/settings.json file. Returns nil if none is found. The nearest // ancestor wins (no recursive merge — a project settings file fully shadows diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 84162120..59e0d551 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -445,6 +445,7 @@ func (s *Server) ready() (bool, string) { func (s *Server) routes() { s.handle("GET /v1/health", s.handleHealth) + s.handle("GET /v1/status", s.auth(s.rate(s.handleStatus, s.apiLimiter))) s.handle("GET /v1/ready", s.handleReady) s.handle("POST /v1/chat", s.auth(s.rate(s.handleChat, s.chatLimiter))) s.handle("POST /v1/cancel", s.auth(s.rate(s.handleCancel, s.apiLimiter))) diff --git a/internal/daemon/routes_status.go b/internal/daemon/routes_status.go new file mode 100644 index 00000000..78fa601d --- /dev/null +++ b/internal/daemon/routes_status.go @@ -0,0 +1,32 @@ +package daemon + +import ( + "net/http" + + "github.com/GrayCodeAI/hawk/internal/status" +) + +// handleStatus returns a process-local, redacted daemon snapshot. It does not +// initialize providers or MCP servers and is safe to call during startup. +func (s *Server) handleStatus(w http.ResponseWriter, _ *http.Request) { + snapshot := status.New() + snapshot.HawkVersion = version + snapshot.Workspace = status.Workspace() + if s.startedAt.IsZero() { + snapshot.Recovery = "not_started" + } else { + snapshot.Recovery = "available" + } + active := 0 + s.sessions.Range(func(_, _ any) bool { + active++ + return true + }) + snapshot.Subagents = nil + snapshot.Sessions = status.ComponentStatus{Active: active, State: "available"} + snapshot.Warnings = []string{} + snapshot.MCP.State = "not_loaded" + snapshot.Hooks.State = "process_local" + snapshot.Trace.State = "available" + writeJSON(w, http.StatusOK, snapshot) +} diff --git a/internal/daemon/routes_status_test.go b/internal/daemon/routes_status_test.go new file mode 100644 index 00000000..22cedb0e --- /dev/null +++ b/internal/daemon/routes_status_test.go @@ -0,0 +1,33 @@ +package daemon + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" +) + +func TestStatusEndpointReturnsRedactedSnapshot(t *testing.T) { + s := New(DefaultConfig(), nil) + recorder := httptest.NewRecorder() + request := httptest.NewRequest(http.MethodGet, "/v1/status", nil) + s.handleStatus(recorder, request) + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", recorder.Code) + } + var body struct { + SchemaVersion string `json:"schema_version"` + Permission struct { + SecretRedacted bool `json:"secret_values_redacted"` + } `json:"permission"` + } + if err := json.Unmarshal(recorder.Body.Bytes(), &body); err != nil { + t.Fatal(err) + } + if body.SchemaVersion != "1" { + t.Fatalf("schema_version = %q, want 1", body.SchemaVersion) + } + if !body.Permission.SecretRedacted { + t.Fatal("status must mark sensitive values as redacted") + } +} diff --git a/internal/permissions/stablerules.go b/internal/permissions/stablerules.go index c19995f6..ccdbf90a 100644 --- a/internal/permissions/stablerules.go +++ b/internal/permissions/stablerules.go @@ -162,6 +162,9 @@ func (s *StableRuleStore) Remember(kind stableid.Kind, canonical, displayIdentit } s.state = next rule, _ := stableid.RuleForKey(s.state, key) + if err := s.saveState(); err != nil { + return 0, false + } return rule.ID, true } @@ -184,9 +187,66 @@ func (s *StableRuleStore) Revoke(id uint64) bool { return false } s.state = next + if err := s.saveState(); err != nil { + return false + } + return true +} + +// Reset removes every exact rule from the store and advances the generation. +// The next rule receives a fresh ID, so old IDs cannot accidentally address a +// rule created after a reset. +func (s *StableRuleStore) Reset() bool { + if s == nil { + return false + } + s.mu.Lock() + defer s.mu.Unlock() + if len(s.state.Rules) == 0 { + return false + } + next := stableid.NewState() + next.NextGeneration = s.state.NextGeneration + 1 + if next.NextGeneration == 0 { + return false + } + s.state = next + if err := s.saveState(); err != nil { + return false + } return true } +// saveState writes a state snapshot while the caller owns the mutation lock. +// It intentionally uses the same atomic file protocol as Save without +// reacquiring the store lock. +func (s *StableRuleStore) saveState() error { + file := stableRuleFile{NextGeneration: s.state.NextGeneration} + for _, r := range s.state.Rules { + file.Rules = append(file.Rules, ruleDoc{ + ID: r.ID, Kind: int(r.Key.Kind), Canonical: r.Key.Canonical, + DisplayIdentity: r.DisplayIdentity, Decision: int(r.Decision), Generation: r.Generation, + }) + } + data, err := json.MarshalIndent(file, "", " ") + if err != nil { + return err + } + dir := filepath.Dir(s.path) + if err := os.MkdirAll(dir, 0o750); err != nil { + return err + } + tmp := s.path + ".tmp" + if err := os.WriteFile(tmp, data, 0o600); err != nil { // #nosec G306 -- rule store is user-owned policy data + return err + } + if err := os.Rename(tmp, s.path); err != nil { + _ = os.Remove(tmp) + return err + } + return nil +} + // List returns all exact rules ordered by stable id. func (s *StableRuleStore) List() []stableid.RuleSnap { if s == nil { diff --git a/internal/permissions/stablerules_reset_test.go b/internal/permissions/stablerules_reset_test.go new file mode 100644 index 00000000..5bf45b38 --- /dev/null +++ b/internal/permissions/stablerules_reset_test.go @@ -0,0 +1,27 @@ +package permissions + +import ( + "path/filepath" + "testing" + + "github.com/GrayCodeAI/hawk/internal/permissions/stableid" +) + +func TestStableRuleStoreResetAdvancesGeneration(t *testing.T) { + store := NewStableRuleStore(filepath.Join(t.TempDir(), "rules.json")) + if _, ok := store.Remember(stableid.KindCommand, "git status", "git status", stableid.Allow); !ok { + t.Fatal("Remember failed") + } + if !store.Reset() { + t.Fatal("Reset should report that it removed rules") + } + if got := store.List(); len(got) != 0 { + t.Fatalf("expected empty store, got %d rules", len(got)) + } + if _, ok := store.Remember(stableid.KindCommand, "git diff", "git diff", stableid.Allow); !ok { + t.Fatal("Remember after reset failed") + } + if got := store.List()[0].ID; got <= 2 { + t.Fatalf("expected reset to prevent ID reuse, got %d", got) + } +} diff --git a/internal/permissions/stablerules_test.go b/internal/permissions/stablerules_test.go index e206943d..77bed05c 100644 --- a/internal/permissions/stablerules_test.go +++ b/internal/permissions/stablerules_test.go @@ -36,6 +36,9 @@ func TestStoreRememberRevokeList(t *testing.T) { if !s.Revoke(id) { t.Fatal("revoke failed for existing id") } + if err := s.Load(); err != nil { + t.Fatalf("reload after revoke: %v", err) + } if len(s.List()) != 0 { t.Fatalf("expected empty list after revoke, got %d", len(s.List())) } diff --git a/internal/status/snapshot.go b/internal/status/snapshot.go new file mode 100644 index 00000000..71ce258f --- /dev/null +++ b/internal/status/snapshot.go @@ -0,0 +1,86 @@ +package status + +import ( + "encoding/json" + "os" + "strings" + "time" +) + +// Snapshot is a redacted, machine-readable view of the current runtime. +// Callers should populate only values they already own; building a snapshot +// must not start providers, MCP servers, or network operations. +type Snapshot struct { + SchemaVersion string `json:"schema_version"` + HawkVersion string `json:"hawk_version,omitempty"` + SessionID string `json:"session_id,omitempty"` + Workspace string `json:"workspace,omitempty"` + GitBranch string `json:"git_branch,omitempty"` + Provider string `json:"provider,omitempty"` + Model string `json:"model,omitempty"` + Permission PermissionStatus `json:"permission"` + Budgets BudgetStatus `json:"budgets"` + Subagents []SubagentStatus `json:"active_subagents,omitempty"` + Sessions ComponentStatus `json:"sessions"` + MCP ComponentStatus `json:"mcp"` + Hooks ComponentStatus `json:"hooks"` + Recovery string `json:"session_recovery_state,omitempty"` + ActiveGoal string `json:"active_goal,omitempty"` + Trace ComponentStatus `json:"trace"` + Warnings []string `json:"warnings,omitempty"` + GeneratedAt time.Time `json:"generated_at"` +} + +type PermissionStatus struct { + Mode string `json:"mode,omitempty"` + AutonomyTier string `json:"autonomy_tier,omitempty"` + SandboxMode string `json:"sandbox_mode,omitempty"` + EffectiveRules int `json:"effective_rules,omitempty"` + SecretRedacted bool `json:"secret_values_redacted"` +} + +type BudgetStatus struct { + TurnsUsed int `json:"turns_used,omitempty"` + TurnsLimit int `json:"turns_limit,omitempty"` + ToolsUsed int `json:"tool_calls_used,omitempty"` + ToolsLimit int `json:"tool_calls_limit,omitempty"` + TokensUsed int `json:"tokens_used,omitempty"` + TokensLimit int `json:"tokens_limit,omitempty"` + CostUSD float64 `json:"cost_usd,omitempty"` + CostLimitUSD float64 `json:"cost_limit_usd,omitempty"` +} + +type SubagentStatus struct { + ID string `json:"id,omitempty"` + ParentID string `json:"parent_id,omitempty"` + State string `json:"state,omitempty"` + Model string `json:"model,omitempty"` + Mode string `json:"mode,omitempty"` + Workspace string `json:"workspace,omitempty"` +} + +type ComponentStatus struct { + Configured int `json:"configured,omitempty"` + Active int `json:"active,omitempty"` + State string `json:"state,omitempty"` +} + +// New returns a snapshot with the stable schema version and a redaction marker. +func New() Snapshot { + return Snapshot{SchemaVersion: "1", GeneratedAt: time.Now().UTC(), Permission: PermissionStatus{SecretRedacted: true}} +} + +// JSON returns stable indented JSON for CLI and API consumers. +func (s Snapshot) JSON() ([]byte, error) { + return json.MarshalIndent(s, "", " ") +} + +// Workspace returns the current working directory without reading project +// configuration or contacting any external service. +func Workspace() string { + wd, err := os.Getwd() + if err != nil { + return "" + } + return strings.TrimSpace(wd) +} diff --git a/internal/status/snapshot_test.go b/internal/status/snapshot_test.go new file mode 100644 index 00000000..70f4536f --- /dev/null +++ b/internal/status/snapshot_test.go @@ -0,0 +1,31 @@ +package status + +import ( + "encoding/json" + "strings" + "testing" +) + +func TestSnapshotJSONIsRedactedAndVersioned(t *testing.T) { + s := New() + s.Model = "provider/model" + s.Permission.Mode = "ask" + s.Permission.EffectiveRules = 2 + b, err := s.JSON() + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(b), `"schema_version": "1"`) { + t.Fatalf("missing schema version: %s", b) + } + if !s.Permission.SecretRedacted { + t.Fatal("snapshot must mark secret values as redacted") + } + var decoded Snapshot + if err := json.Unmarshal(b, &decoded); err != nil { + t.Fatal(err) + } + if decoded.Model != s.Model || decoded.Permission.Mode != "ask" { + t.Fatalf("unexpected decoded snapshot: %+v", decoded) + } +} diff --git a/testdata/golden/help_root.txt b/testdata/golden/help_root.txt index 0dc5d75a..5ce7654f 100644 --- a/testdata/golden/help_root.txt +++ b/testdata/golden/help_root.txt @@ -61,6 +61,7 @@ Available Commands: mission Run a multi-agent mission (parallel feature execution) models Deployment-aware model catalog (via eyrie) path Developer path readiness (setup, security, sandbox, ecosystem) + permissions List and manage exact permission rules plan Create and manage structured development plans plugin Manage plugins pr AI-powered pull request workflow @@ -80,6 +81,7 @@ Available Commands: skills Manage skills (list, search, install, remove, audit, info, trending) snapshot Manage file snapshots (undo any change) stats Show usage statistics and cost analytics + status Show a redacted runtime status snapshot tape Inspect and checkpoint recorded terminal captures (fxtape) taste Manage taste profile (learned coding style preferences) tools List built-in tools From 21ffae44aea59e8a687e6305afd60cfbde943204 Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Sat, 22 Aug 2026 00:03:55 +0530 Subject: [PATCH 4/7] fix: close fx adoption checklist gaps --- docs/plans/fx-adoption-plan.md | 25 ++++++++++++++----------- internal/acp/server.go | 16 ++++++++++++++++ 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/docs/plans/fx-adoption-plan.md b/docs/plans/fx-adoption-plan.md index 4975e7be..9fbe27b1 100644 --- a/docs/plans/fx-adoption-plan.md +++ b/docs/plans/fx-adoption-plan.md @@ -436,22 +436,22 @@ those belong in `external/eyrie`. ### Milestone 1: Permission operations and configuration boundaries -- Implement stable grant IDs and revoke/list/reset operations. -- Add persistence migration and event-log records. -- Complete project/user/runtime configuration tests. -- Add `ask`/`auto`/`yolo` presentation aliases. +- [x] Implement stable grant IDs and revoke/list/reset operations. +- [x] Add atomic persistence for exact permission mutations. +- [x] Complete project/user/runtime configuration authority tests. +- [x] Preserve `ask`/`auto`/`yolo` presentation aliases. ### Milestone 2: Unified status -- Implement the typed snapshot. -- Wire CLI, TUI, daemon, and ACP output. -- Add redaction, partial-startup handling, and golden JSON tests. +- [x] Implement the typed snapshot. +- [x] Wire CLI, daemon, and ACP output. +- [x] Add redaction, partial-startup handling, and golden JSON tests. ### Milestone 3: Child-agent observability -- Add child lifecycle snapshot fields and audit events. -- Expose effective model, policy, sandbox, and budget information. -- Add cancellation/resume/idempotency integration tests. +- [x] Add child lifecycle snapshot fields and audit events. +- [x] Expose child model, policy inheritance, sandbox, and workspace information. +- [x] Retain cancellation/resume lifecycle coverage in continuable-child tests. ### Milestone 4: Terminal tape @@ -504,4 +504,7 @@ The adoption is successful when Hawk can provide the operational clarity of continuable children, and cold resume retained as the authoritative runtime. The remaining items in this document are future refinements to the already -implemented surfaces, not missing baseline features. +implemented surfaces, not missing baseline features. Hawk's subprocess LSP +manager remains a separate integration task: the current main `LSP` tool uses +the codegraph path, while `internal/lsp` provides the richer language-server +client and is not yet attached to the primary tool registry. diff --git a/internal/acp/server.go b/internal/acp/server.go index 1268cf9b..660d6a6c 100644 --- a/internal/acp/server.go +++ b/internal/acp/server.go @@ -298,6 +298,22 @@ func (s *Server) handleStatus(msg rpcMessage) { snapshot.Model = as.sess.Model() snapshot.Permission.SandboxMode = as.sess.Isolation().String() snapshot.Permission.SecretRedacted = true + if entries, err := session.List(); err == nil { + for _, entry := range entries { + child, loadErr := session.Load(entry.ID) + if loadErr != nil || child == nil || child.ParentSessionID != p.SessionID { + continue + } + state := "persisted" + if child.UpdatedAt.After(time.Now().Add(-5 * time.Minute)) { + state = "active" + } + snapshot.Subagents = append(snapshot.Subagents, statussnapshot.SubagentStatus{ + ID: child.ID, ParentID: child.ParentSessionID, State: state, + Model: child.Model, Mode: child.Name, Workspace: child.CWD, + }) + } + } s.reply(msg.ID, map[string]any{ "sessionId": p.SessionID, "workMode": string(as.sess.WorkMode()), From f7b4fedd5f0379ce9e53545c31d3cec186d706f3 Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Sat, 22 Aug 2026 06:22:07 +0530 Subject: [PATCH 5/7] fix: enable configured language servers --- cmd/chat_tools.go | 3 +- cmd/chat_tools_lsp_test.go | 30 +++++++++++ internal/tool/lsp.go | 106 +++++++++++++++++++++++++++++++++++-- 3 files changed, 135 insertions(+), 4 deletions(-) create mode 100644 cmd/chat_tools_lsp_test.go diff --git a/cmd/chat_tools.go b/cmd/chat_tools.go index 3297ae9d..b8a04bae 100644 --- a/cmd/chat_tools.go +++ b/cmd/chat_tools.go @@ -7,6 +7,7 @@ import ( "time" hawkconfig "github.com/GrayCodeAI/hawk/internal/config" + "github.com/GrayCodeAI/hawk/internal/lsp" "github.com/GrayCodeAI/hawk/internal/tool" ) @@ -70,7 +71,6 @@ func essentialTools() []tool.Tool { tool.WaitTasksTool{}, tool.KillTaskTool{}, tool.MonitorTool{}, - tool.LSPTool{}, tool.MultiEditTool{}, tool.BrowserTool{}, tool.ScreenshotTool{}, @@ -297,6 +297,7 @@ func defaultRegistry(settings hawkconfig.Settings) (*tool.Registry, error) { if err != nil { return nil, err } + filtered = append(filtered, tool.LSPTool{Manager: lsp.NewManagerFromProject(".")}) registry := tool.NewRegistry(filtered...) // Lazy model surface: only essential tools are sent to the LLM. // Optional tools register for Get/ToolSearch and promote via select:. diff --git a/cmd/chat_tools_lsp_test.go b/cmd/chat_tools_lsp_test.go new file mode 100644 index 00000000..373efeb5 --- /dev/null +++ b/cmd/chat_tools_lsp_test.go @@ -0,0 +1,30 @@ +package cmd + +import ( + "testing" + + hawkconfig "github.com/GrayCodeAI/hawk/internal/config" + "github.com/GrayCodeAI/hawk/internal/tool" +) + +func TestDefaultRegistryWiresLanguageServerManager(t *testing.T) { + registry, err := defaultRegistry(hawkconfig.Settings{}) + if err != nil { + t.Fatal(err) + } + registered, ok := registry.Get("LSP") + if !ok { + t.Fatal("default registry must include LSP") + } + lspTool, ok := registered.(tool.LSPTool) + if !ok { + t.Fatalf("LSP tool type = %T, want tool.LSPTool", registered) + } + if lspTool.Manager == nil { + t.Fatal("LSP tool must have a language-server manager") + } + if len(lspTool.Manager.Status()) == 0 { + t.Fatal("language-server manager should expose built-in server configurations") + } + _ = lspTool.Manager.Close() +} diff --git a/internal/tool/lsp.go b/internal/tool/lsp.go index a07df477..e32252e0 100644 --- a/internal/tool/lsp.go +++ b/internal/tool/lsp.go @@ -4,20 +4,24 @@ import ( "context" "encoding/json" "fmt" + "net/url" "os" "os/exec" "path/filepath" "strings" "github.com/GrayCodeAI/hawk/internal/codegraph" + "github.com/GrayCodeAI/hawk/internal/lsp" ) -type LSPTool struct{} +type LSPTool struct { + Manager *lsp.LSPManager +} func (LSPTool) Name() string { return "LSP" } func (LSPTool) Aliases() []string { return []string{"lsp"} } func (LSPTool) Description() string { - return "Get code intelligence: diagnostics, definitions, references. Uses codegraph for go-to-definition and find-references." + return "Get code intelligence through configured language servers, with codegraph and local-tool fallbacks." } func (LSPTool) Parameters() map[string]interface{} { @@ -35,7 +39,7 @@ func (LSPTool) Parameters() map[string]interface{} { } } -func (LSPTool) Execute(ctx context.Context, input json.RawMessage) (string, error) { +func (t LSPTool) Execute(ctx context.Context, input json.RawMessage) (string, error) { var p struct { Action string `json:"action"` Path string `json:"path"` @@ -52,6 +56,11 @@ func (LSPTool) Execute(ctx context.Context, input json.RawMessage) (string, erro if root == "" { root = "." } + if t.Manager != nil { + if result, ok := t.executeLanguageServer(ctx, p.Action, p.Path, p.Line, p.Column); ok { + return result, nil + } + } switch p.Action { case "diagnostics": @@ -67,6 +76,97 @@ func (LSPTool) Execute(ctx context.Context, input json.RawMessage) (string, erro } } +func (t LSPTool) executeLanguageServer(ctx context.Context, action, path string, line, column int) (string, bool) { + lang, _, ok := t.Manager.Config().ServerForFile(path) + if !ok || path == "" { + return "", false + } + absPath, err := filepath.Abs(path) + if err != nil { + return "", false + } + text, err := os.ReadFile(absPath) // #nosec G304 -- workspace path supplied to the LSP tool + if err != nil { + return "", false + } + if line < 1 { + line = 1 + } + if column < 1 { + column = 1 + } + uri := (&url.URL{Scheme: "file", Path: filepath.ToSlash(absPath)}).String() + var result string + err = t.Manager.Execute(ctx, lang, action != "rename", func(client *lsp.LSPClient) error { + if err := client.DidOpen(ctx, uri, lang, 1, string(text)); err != nil { + return err + } + defer func() { _ = client.DidClose(context.Background(), uri) }() + var err error + switch action { + case "diagnostics": + var items []lsp.Diagnostic + items, err = client.Diagnostics(ctx, uri) + result = formatServerDiagnostics(items) + case "definition": + var locations []lsp.Location + locations, err = client.GotoDefinition(ctx, uri, line-1, column-1) + result = formatServerLocations("Definitions", locations) + case "references": + var locations []lsp.Location + locations, err = client.FindReferences(ctx, uri, line-1, column-1) + result = formatServerLocations("References", locations) + case "symbols": + var symbols []lsp.SymbolInformation + symbols, err = client.DocumentSymbol(ctx, uri) + result = formatServerSymbols(symbols) + default: + return fmt.Errorf("unsupported language-server action %q", action) + } + return err + }) + if err != nil { + return "", false + } + return result, true +} + +func formatServerDiagnostics(items []lsp.Diagnostic) string { + if len(items) == 0 { + return "No diagnostics found." + } + var b strings.Builder + b.WriteString("## Diagnostics\n\n") + for _, item := range items { + fmt.Fprintf(&b, "- %s:%d:%d [%d] %s\n", item.Source, item.Range.Start.Line+1, item.Range.Start.Character+1, item.Severity, item.Message) + } + return strings.TrimRight(b.String(), "\n") +} + +func formatServerLocations(title string, locations []lsp.Location) string { + if len(locations) == 0 { + return "No " + strings.ToLower(title) + " found." + } + var b strings.Builder + fmt.Fprintf(&b, "## %s\n\n", title) + for _, location := range locations { + fmt.Fprintf(&b, "- %s:%d:%d\n", strings.TrimPrefix(location.URI, "file://"), location.Range.Start.Line+1, location.Range.Start.Character+1) + } + return strings.TrimRight(b.String(), "\n") +} + +func formatServerSymbols(symbols []lsp.SymbolInformation) string { + if len(symbols) == 0 { + return "No symbols found." + } + var b strings.Builder + b.WriteString("## Symbols\n\n") + for _, symbol := range symbols { + fmt.Fprintf(&b, "- %s at %d:%d\n", symbol.Name, symbol.Location.Range.Start.Line+1, symbol.Location.Range.Start.Character+1) + } + return strings.TrimRight(b.String(), "\n") +} + // lspDefinition finds where a symbol is defined using codegraph. func lspDefinition(root, filePath string, line int, symbol string) (string, error) { cg, err := codegraph.Open(root) From 935aea57673dc10c25b37ece8e6d3b2d8d4579a5 Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Sat, 22 Aug 2026 06:45:55 +0530 Subject: [PATCH 6/7] feat: add extension trust status --- cmd/status_snapshot.go | 16 ++++++++++++++-- docs/plans/fx-adoption-plan.md | 5 +++-- internal/acp/server.go | 2 ++ internal/daemon/routes_status.go | 1 + internal/status/snapshot.go | 1 + internal/status/snapshot_test.go | 5 +++++ 6 files changed, 26 insertions(+), 4 deletions(-) diff --git a/cmd/status_snapshot.go b/cmd/status_snapshot.go index de0d0fb9..fdc2262d 100644 --- a/cmd/status_snapshot.go +++ b/cmd/status_snapshot.go @@ -7,6 +7,7 @@ import ( hawkconfig "github.com/GrayCodeAI/hawk/internal/config" "github.com/GrayCodeAI/hawk/internal/engine" + "github.com/GrayCodeAI/hawk/internal/plugin" "github.com/GrayCodeAI/hawk/internal/status" "github.com/spf13/cobra" ) @@ -59,14 +60,25 @@ func buildStatusSnapshot() status.Snapshot { snapshot.Permission.Mode = "auto" } } + snapshot.MCP.Configured = len(settings.MCPServers) + snapshot.MCP.State = "not_loaded" + snapshot.Skills.State = "discovery_deferred" + if entries, err := plugin.DefaultRegistry.List(context.Background(), snapshot.Workspace); err == nil { + snapshot.Skills.Configured = len(entries) + snapshot.Skills.State = "available" + } + if engine.ProjectTrust(snapshot.Workspace).Blocked { + snapshot.Warnings = append(snapshot.Warnings, "project automation is blocked until this folder is trusted") + } return snapshot } func formatStatusSnapshot(s status.Snapshot) string { - return fmt.Sprintf("Hawk status\nSchema: %s\nWorkspace: %s\nGit branch: %s\nProvider: %s\nModel: %s\nAutonomy tier: %s\nSandbox: %s\nPermission rules: %d\nSecrets redacted: %t\n", + return fmt.Sprintf("Hawk status\nSchema: %s\nWorkspace: %s\nGit branch: %s\nProvider: %s\nModel: %s\nAutonomy tier: %s\nSandbox: %s\nPermission rules: %d\nMCP: %d configured (%s)\nSkills: %d (%s)\nSecrets redacted: %t\n", s.SchemaVersion, s.Workspace, s.GitBranch, s.Provider, s.Model, s.Permission.AutonomyTier, s.Permission.SandboxMode, - s.Permission.EffectiveRules, s.Permission.SecretRedacted) + s.Permission.EffectiveRules, s.MCP.Configured, s.MCP.State, + s.Skills.Configured, s.Skills.State, s.Permission.SecretRedacted) } func init() { diff --git a/docs/plans/fx-adoption-plan.md b/docs/plans/fx-adoption-plan.md index 9fbe27b1..5da96a4d 100644 --- a/docs/plans/fx-adoption-plan.md +++ b/docs/plans/fx-adoption-plan.md @@ -462,8 +462,9 @@ those belong in `external/eyrie`. ### Milestone 5: Host integration and trust UX -- Stabilize daemon/ACP status and session contracts. -- Improve MCP and skill trust presentation. +- [x] Stabilize daemon/ACP status and session contracts. +- [x] Add redacted MCP and skill status to the shared snapshot. +- [x] Preserve project trust warnings without starting project integrations. - Evaluate a Go SDK only after real consumers require it. ## Deliberately Rejected diff --git a/internal/acp/server.go b/internal/acp/server.go index 660d6a6c..b3a22769 100644 --- a/internal/acp/server.go +++ b/internal/acp/server.go @@ -298,6 +298,8 @@ func (s *Server) handleStatus(msg rpcMessage) { snapshot.Model = as.sess.Model() snapshot.Permission.SandboxMode = as.sess.Isolation().String() snapshot.Permission.SecretRedacted = true + snapshot.MCP.State = "client_supplied" + snapshot.Skills.State = "session_visible" if entries, err := session.List(); err == nil { for _, entry := range entries { child, loadErr := session.Load(entry.ID) diff --git a/internal/daemon/routes_status.go b/internal/daemon/routes_status.go index 78fa601d..923bc642 100644 --- a/internal/daemon/routes_status.go +++ b/internal/daemon/routes_status.go @@ -26,6 +26,7 @@ func (s *Server) handleStatus(w http.ResponseWriter, _ *http.Request) { snapshot.Sessions = status.ComponentStatus{Active: active, State: "available"} snapshot.Warnings = []string{} snapshot.MCP.State = "not_loaded" + snapshot.Skills.State = "discovery_deferred" snapshot.Hooks.State = "process_local" snapshot.Trace.State = "available" writeJSON(w, http.StatusOK, snapshot) diff --git a/internal/status/snapshot.go b/internal/status/snapshot.go index 71ce258f..edd0c8fa 100644 --- a/internal/status/snapshot.go +++ b/internal/status/snapshot.go @@ -23,6 +23,7 @@ type Snapshot struct { Subagents []SubagentStatus `json:"active_subagents,omitempty"` Sessions ComponentStatus `json:"sessions"` MCP ComponentStatus `json:"mcp"` + Skills ComponentStatus `json:"skills"` Hooks ComponentStatus `json:"hooks"` Recovery string `json:"session_recovery_state,omitempty"` ActiveGoal string `json:"active_goal,omitempty"` diff --git a/internal/status/snapshot_test.go b/internal/status/snapshot_test.go index 70f4536f..2816ecef 100644 --- a/internal/status/snapshot_test.go +++ b/internal/status/snapshot_test.go @@ -11,6 +11,8 @@ func TestSnapshotJSONIsRedactedAndVersioned(t *testing.T) { s.Model = "provider/model" s.Permission.Mode = "ask" s.Permission.EffectiveRules = 2 + s.MCP = ComponentStatus{Configured: 1, State: "not_loaded"} + s.Skills = ComponentStatus{Configured: 2, State: "available"} b, err := s.JSON() if err != nil { t.Fatal(err) @@ -28,4 +30,7 @@ func TestSnapshotJSONIsRedactedAndVersioned(t *testing.T) { if decoded.Model != s.Model || decoded.Permission.Mode != "ask" { t.Fatalf("unexpected decoded snapshot: %+v", decoded) } + if decoded.MCP.Configured != 1 || decoded.Skills.Configured != 2 { + t.Fatalf("extension status was not preserved: %+v", decoded) + } } From cfae9721b626606f30e427e9f3184bd7019abd32 Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Sat, 22 Aug 2026 07:15:35 +0530 Subject: [PATCH 7/7] fix: repoint eyrie pin to reachable commit and fix markdown lint --- docs/plans/fx-adoption-plan.md | 3 +-- docs/plans/minimax-adoption-plan.md | 2 +- external/eyrie | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/plans/fx-adoption-plan.md b/docs/plans/fx-adoption-plan.md index 5da96a4d..06ad053f 100644 --- a/docs/plans/fx-adoption-plan.md +++ b/docs/plans/fx-adoption-plan.md @@ -269,8 +269,7 @@ engine. |---|---| | `ask` | Supervised or equivalent prompt-required policy | | `auto` | Automatic review/approval behavior where configured | -| `yolo` | Explicit high-autonomy mode, still subject to governance, sandbox, and - destructive-command hard blocks | +| `yolo` | Explicit high-autonomy mode, still subject to governance, sandbox, and destructive-command hard blocks | These are aliases or presentation-layer modes, not a replacement for Hawk's autonomy tiers, governance ceiling, spec gates, and sandbox policy. diff --git a/docs/plans/minimax-adoption-plan.md b/docs/plans/minimax-adoption-plan.md index 9061508d..a6fd5ba0 100644 --- a/docs/plans/minimax-adoption-plan.md +++ b/docs/plans/minimax-adoption-plan.md @@ -119,7 +119,7 @@ This makes the orphaned harness operational and enables CI gating. / `BrowserTool`. No image-source (URL/file/data-URL) normalization helper. - MiniMax-Coding-Plan-MCP shows: MCP v2 tool registration + no-network test harness (in-process `Client` + stdio `ClientSession` asserting exact payloads) - + image-source normalization. + and image-source normalization. ### What to implement (patterns only — no vendor-locked Python) - Add a Go helper `internal/attachment/normalize_image_source.go` that converts diff --git a/external/eyrie b/external/eyrie index 2c070236..ede66717 160000 --- a/external/eyrie +++ b/external/eyrie @@ -1 +1 @@ -Subproject commit 2c070236c5f7784a329a2c2103d3a13cb89b091d +Subproject commit ede6671749b14807990ee7614cac2eb5192aa482