Skip to content

feat: add Ollama and llama.cpp local inference providers - #2

Merged
intel352 merged 14 commits into
masterfrom
feat/local-inference-providers
Mar 28, 2026
Merged

intel352 merged 14 commits into
masterfrom
feat/local-inference-providers

Conversation

@intel352

Copy link
Copy Markdown
Contributor

Summary

  • Add two new AI providers for local model inference: Ollama (SDK-based) and llama.cpp (OpenAI-compatible with optional process management)
  • Add <think> reasoning trace support via Thinking field on Response/StreamEvent, threaded through executor, transcript, and step output
  • Add step.model_pull for model acquisition from Ollama and HuggingFace Hub
  • Add llama-server auto-download from GitHub releases

Design

See: docs/plans/2026-03-27-local-inference-providers-design.md

Implementation Plan

See: docs/plans/2026-03-27-local-inference-providers-plan.md

Changes

Core Interface

  • provider/provider.go: Added Thinking field to Response and StreamEvent

Shared Helpers

  • provider/local.go: ParseThinking(), ThinkingStreamParser (streaming state machine), LocalAuthMode()

Ollama Provider

  • provider/ollama.go: Full provider with Chat, Stream, Pull, ListModels, Health via ollama/ollama/api SDK
  • provider/ollama_convert.go: Message and tool type conversion between provider and Ollama API types

llama.cpp Provider

  • provider/llama_cpp.go: External mode (any OpenAI-compatible server) + managed mode (process lifecycle)
  • provider/llama_cpp_download.go: Auto-download llama-server from GitHub releases with platform detection

HuggingFace Downloader

  • provider/huggingface.go: Generic file downloader with resume support (Range header)

step.model_pull

  • step_model_pull.go: Pipeline step for Ollama pull and HuggingFace file download

Executor Threading

  • executor/executor.go: Thinking traces in Result and transcript entries
  • step_agent_execute.go: Thinking in step output

Wiring

  • provider_registry.go, module_provider.go, plugin.go, provider/auth_modes.go, provider/models.go: Registered both providers + step.model_pull

Tests

All new code has tests. Full suite: 8 packages, 0 failures.

🤖 Generated with Claude Code

intel352 and others added 13 commits March 27, 2026 18:56
Design for two new providers enabling self-hosted model inference
with <think> reasoning trace support, step.model_pull for model
acquisition from Ollama and HuggingFace, and llama-server process
management with auto-download.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
12-task TDD plan covering: core interface changes, think-tag parser,
Ollama provider (SDK), llama.cpp provider (process mgmt + auto-download),
HuggingFace downloader, step.model_pull, executor thinking threading,
and registry/plugin wiring.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add Thinking string field to Response and StreamEvent structs in
provider/provider.go to support reasoning traces from local models.
Add "thinking" as a new StreamEvent type. Existing providers unaffected.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds the Ollama client-only package needed for the Ollama provider implementation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…amParser, LocalAuthMode)

Adds provider/local.go with:
- ParseThinking: extracts <think>...</think> blocks from model output
- ThinkingStreamParser: streaming state machine handling tags split across chunks
- LocalAuthMode: helper for no-API-key provider auth metadata

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add Thinking field to TranscriptEntry and Result
- Capture resp.Thinking into finalThinking in execute loop
- Include Thinking in assistant transcript Record calls
- Expose thinking in step output map when non-empty
- Add TestExecute_ThinkingInResult and TestExecute_ThinkingInTranscript

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds provider/ollama_convert.go with:
- toOllamaMessages: converts provider messages to Ollama API messages
- toOllamaTools: converts ToolDef to Ollama Tool via JSON schema marshal/unmarshal
- fromOllamaResponse: converts Ollama response to provider Response with ParseThinking applied
- fromOllamaStreamChunk: extracts text, tool calls, and done flag from streaming chunks

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds provider/ollama.go with OllamaProvider:
- Chat(): non-streaming via Ollama API with ParseThinking applied
- Stream(): streaming with ThinkingStreamParser for <think> tag splitting
- Pull(): model download with progress callback
- ListModels(): returns models from /api/tags
- Health(): heartbeat check via HEAD /

Tests mock the Ollama HTTP API to cover chat, think-tag extraction,
streaming, streaming with think tags, health, and list models.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds two missing tests per design doc Section 8:
- TestStreamParserNestedThinkTags: verifies nested <think> inside open block
  is treated as literal content (no recursion); first </think> closes the block
- TestStreamParserUnclosedTag: verifies parser stays in inThink state when
  chunk has <think> with no closing tag, emitting thinking events and
  continuing correctly on subsequent feeds

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
llama_cpp.go (Task 6):
- LlamaCppProvider implementing Provider with external/managed modes
- Chat() and Stream() reuse openai_convert.go helpers + ParseThinking/ThinkingStreamParser
- EnsureServer() manages llama-server lifecycle in managed mode
- Close() kills managed process

llama_cpp_download.go (Task 7):
- EnsureLlamaServer() downloads from ggerganov/llama.cpp GitHub releases
- Caches to ~/.cache/workflow/llama-server/<version>/
- Platform-aware asset selection (linux/darwin/windows, amd64/arm64)

huggingface.go (Task 8):
- DownloadHuggingFaceFile() generic HF Hub file downloader
- Resume support via Range header on .part temp file
- Progress callback, skip-if-exists, configurable base URL for tests

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- ModelPullStep with "ollama" and "huggingface" sources
- Ollama: checks ListModels first (returns "ready"), falls back to Pull() ("downloaded")
- HuggingFace: delegates to DownloadHuggingFaceFile() with file size in output
- Output: {status, model_path, size_bytes}
- Tests with mock Ollama API server and mock HF download server

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…l listing

- provider_registry.go: add "ollama" and "llama_cpp" factories
- module_provider.go: add "ollama"/"llama_cpp" cases, update error message
- plugin.go: register step.model_pull, export NewModelPullStepFactory(),
  add ollama/llama_cpp to provider Options schema
- provider/auth_modes.go: add ollama and llama_cpp local auth modes
- provider/models.go: add ollama/llama_cpp cases + listOllamaModels()

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Task 7 (llama.cpp auto-download):
- Add named types ghRelease and ghAsset (replace anonymous structs)
- Extract findAssetURL() and assetNameForPlatform() as testable functions
- Add llamaCppCacheDir package var for test isolation (drop HOME env hack)
- Add TestAssetNameForPlatform_AllPlatforms and TestParseLlamaCppRelease_FindsAsset
- Add TestParseLlamaCppRelease_NoMatchingAsset and TestEnsureLlamaServer_GitHubAPIFlow

Task 8 (HuggingFace downloader):
- Add TestDownloadHuggingFaceFile_Resume (206 Partial Content, Range header)
- Add TestDownloadHuggingFaceFile_ErrorStatus (404 → error)
- Add TestDownloadHuggingFaceFile_DefaultOutputDir (empty outputDir fallback)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds local inference support to the agent plugin by introducing two new providers (Ollama + llama.cpp), plus a shared <think> parsing pipeline so “thinking traces” can be captured and surfaced through executor results, transcripts, and step outputs.

Changes:

  • Added Ollama (SDK-based) and llama.cpp (OpenAI-compatible, optional managed process) providers, including model listing and health checks.
  • Added <think> extraction + streaming parsing via Thinking fields on Response/StreamEvent, threaded through executor + step output.
  • Introduced step.model_pull for acquiring models from Ollama or HuggingFace Hub, and llama-server auto-download logic.

Reviewed changes

Copilot reviewed 28 out of 29 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
step_model_pull.go Adds step.model_pull implementation for Ollama pull + HuggingFace download.
step_model_pull_test.go Tests for the new step.model_pull behavior.
step_agent_execute.go Surfaces result.Thinking in step output when present.
provider/provider.go Extends core provider response/event types with Thinking.
provider/thinking_field_test.go JSON round-trip + omit-empty tests for the new Thinking fields.
provider/local.go Adds <think> parsing + streaming state machine + local auth-mode helper.
provider/local_test.go Tests for think parsing, streaming parsing, and LocalAuthMode.
provider/ollama.go Implements Ollama provider (chat/stream/pull/list/health).
provider/ollama_convert.go Converts message/tool types between internal types and Ollama SDK types.
provider/ollama_test.go Ollama provider unit tests using an HTTP test server.
provider/ollama_convert_test.go Tests for Ollama conversion helpers and think extraction.
provider/llama_cpp.go Implements llama.cpp provider over OpenAI-compatible API + managed server lifecycle.
provider/llama_cpp_test.go llama.cpp provider tests (chat/stream/thinking/defaults).
provider/llama_cpp_download.go Auto-downloads llama-server from GitHub releases into a cache dir.
provider/llama_cpp_download_test.go Tests for platform asset selection + download/extract/cache flows.
provider/huggingface.go Adds a generic HF file downloader with resume support.
provider/huggingface_test.go Tests for download/exists/progress/resume/error/default dir behaviors.
provider/models.go Wires model listing for ollama and llama_cpp.
provider/auth_modes.go Adds local inference auth modes (Ollama + llama.cpp).
provider_registry.go Registers provider factories for ollama and llama_cpp.
module_provider.go Wires new providers into the agent.provider module factory.
plugin.go Registers step.model_pull and adds new provider options in schema.
executor/interfaces.go Extends transcript entry schema with Thinking.
executor/executor.go Threads Thinking into transcript entries + final Result.
executor/executor_test.go Tests that executor result/transcript includes Thinking.
go.mod Adds new indirect dependencies for Ollama SDK and related packages.
go.sum Dependency checksums updated accordingly.
docs/plans/2026-03-27-local-inference-providers-design.md Design doc for local providers + thinking traces + model pull.
docs/plans/2026-03-27-local-inference-providers-plan.md Implementation plan captured in-repo.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread provider/llama_cpp.go
Comment on lines +89 to +99
params := openaisdk.ChatCompletionNewParams{
Messages: toOpenAIMessages(messages),
MaxTokens: openaisdk.Int(int64(p.config.MaxTokens)),
}
if len(tools) > 0 {
params.Tools = toOpenAITools(tools)
}
// llama-server requires a model field but ignores it; use a placeholder.
params.Model = shared.ChatModel("local")

resp, err := p.client.Chat.Completions.New(ctx, params)

Copilot AI Mar 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chat hardcodes params.Model to "local". For external mode (BaseURL pointing at an OpenAI-compatible server), many servers require a real model identifier and will reject unknown values. Consider adding a Model field to LlamaCppConfig (separate from ModelPath) and use it for params.Model when provided (or at least allow configuring the value instead of hardcoding).

Copilot uses AI. Check for mistakes.
Comment thread module_provider.go
Comment on lines +216 to +220
case "llama_cpp":
p = provider.NewLlamaCppProvider(provider.LlamaCppConfig{
BaseURL: baseURL,
MaxTokens: maxTokens,
})

Copilot AI Mar 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new llama_cpp provider wiring doesn't pass the module's model config into LlamaCppConfig at all. As a result, users can't use the managed mode described in the PR (where model is a local .gguf path), and external servers also can't receive a configurable model name. Please map the module config model to LlamaCppConfig.ModelPath for managed mode (and/or add a separate model_name for external mode).

Copilot uses AI. Check for mistakes.
Comment thread provider/llama_cpp_download.go Outdated
const llamaCppPinnedVersion = "b3447"

var (
llamaServerGitHubAPI = "https://api.github.com/repos/ggerganov/llama.cpp/releases/latest"

Copilot AI Mar 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

llamaCppPinnedVersion is described as a pinned version, but resolveLlamaServerURL fetches .../releases/latest and uses whatever TagName is returned. This means the downloaded binary isn't actually pinned, yet it is cached under a directory named after the pinned version. Either pin the GitHub API request to the specific tag/version, or rename/remove the pinned-version concept and cache by the resolved release tag to avoid confusion and mismatched cache contents.

Suggested change
llamaServerGitHubAPI = "https://api.github.com/repos/ggerganov/llama.cpp/releases/latest"
llamaServerGitHubAPI = "https://api.github.com/repos/ggerganov/llama.cpp/releases/tags/" + llamaCppPinnedVersion

Copilot uses AI. Check for mistakes.
Comment thread provider/llama_cpp.go Outdated
Comment on lines +206 to +211
// Close kills the managed llama-server process if one was started.
func (p *LlamaCppProvider) Close() error {
if p.cmd != nil && p.cmd.Process != nil {
return p.cmd.Process.Kill()
}
return nil

Copilot AI Mar 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Close() kills the managed llama-server process but never calls Wait(). On Unix, this can leave a zombie process until the parent exits. Please ensure the child is reaped (e.g., Kill then Wait, or start a goroutine after Start to always Wait and capture the exit status).

Copilot uses AI. Check for mistakes.
Comment thread provider/huggingface.go
Comment on lines +35 to +38
repoSlug := strings.ReplaceAll(repo, "/", "--")
destDir := filepath.Join(outputDir, repoSlug)
destPath := filepath.Join(destDir, filename)

Copilot AI Mar 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filename is joined directly into destPath without sanitization. A crafted filename containing path separators (e.g. "../../...") can escape destDir and overwrite arbitrary files. Please reject filenames containing path separators / .. segments (or use filepath.Base + a strict allowlist) before building destPath and writing the .part/final file.

Copilot uses AI. Check for mistakes.
Comment thread step_model_pull.go
Comment on lines +115 to +119
Output: map[string]any{
"status": "downloaded",
"model_path": path,
"size_bytes": int(sizeBytes),
},

Copilot AI Mar 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

size_bytes is derived from fi.Size() (int64) but then cast to int. This can overflow on 32-bit platforms and is inconsistent with other step outputs that commonly use int64 (e.g. latency_ms). Consider keeping size_bytes as int64 in the output map (and updating the test accordingly).

Copilot uses AI. Check for mistakes.
Comment thread provider/ollama.go Outdated
if cfg.HTTPClient == nil {
cfg.HTTPClient = http.DefaultClient
}
base, _ := url.Parse(cfg.BaseURL)

Copilot AI Mar 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NewOllamaProvider ignores the error from url.Parse(cfg.BaseURL). If an invalid BaseURL is configured, this can lead to a nil/invalid client base URL and hard-to-diagnose failures later. Please handle the parse error explicitly (e.g., fall back to the default URL, or store the error and return it from Chat/Stream/ListModels/Health).

Suggested change
base, _ := url.Parse(cfg.BaseURL)
base, err := url.Parse(cfg.BaseURL)
if err != nil {
// Fall back to the default base URL if the configured one is invalid.
base, err = url.Parse(defaultOllamaBaseURL)
if err != nil {
// This should never happen; defaultOllamaBaseURL is a constant.
panic(fmt.Sprintf("invalid default Ollama base URL %q: %v", defaultOllamaBaseURL, err))
}
cfg.BaseURL = defaultOllamaBaseURL
}

Copilot uses AI. Check for mistakes.
Comment thread provider_registry.go
Comment on lines +96 to +101
r.Factories["llama_cpp"] = func(_ string, cfg LLMProviderConfig) (provider.Provider, error) {
return provider.NewLlamaCppProvider(provider.LlamaCppConfig{
BaseURL: cfg.BaseURL,
MaxTokens: cfg.MaxTokens,
}), nil
}

Copilot AI Mar 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

llama_cpp is registered in the provider registry without threading through cfg.Model (or any model identifier/path) into LlamaCppConfig. This makes the registry-created provider unable to support the PR's documented managed mode (model path) and prevents selecting a model for external OpenAI-compatible servers. Please include the model config in LlamaCppConfig (e.g., ModelPath: cfg.Model for managed mode, or a dedicated Model field for external mode).

Copilot uses AI. Check for mistakes.
Comment on lines +101 to +106
req, err := http.NewRequestWithContext(ctx, http.MethodGet, llamaServerGitHubAPI, nil)
if err != nil {
return "", "", fmt.Errorf("llama_cpp_download: create GitHub API request: %w", err)
}
req.Header.Set("Accept", "application/vnd.github+json")

Copilot AI Mar 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHub's API generally requires a User-Agent header, and omitting it can lead to request rejection or more aggressive rate limiting. Please set an explicit User-Agent (and consider handling rate-limit / 403 responses with a clearer error) on the GitHub API request.

Copilot uses AI. Check for mistakes.
Comment thread provider/llama_cpp.go
Comment on lines +193 to +199
return fmt.Errorf("llama_cpp: llama-server did not become healthy within timeout")
}
resp, err := p.config.HTTPClient.Get(healthURL) //nolint:noctx
if err == nil {
_ = resp.Body.Close()
if resp.StatusCode == http.StatusOK {
return nil

Copilot AI Mar 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The health-check request uses p.config.HTTPClient.Get(healthURL) without a context, so it can't be canceled promptly if the context is canceled and may hang the startup loop if the server accepts but doesn't respond. Please build the request with http.NewRequestWithContext(ctx, ...) (and/or set a reasonable per-request timeout on the HTTP client).

Copilot uses AI. Check for mistakes.
- Add ModelName field to LlamaCppConfig for configurable model identifier
- Pass model config through registry and module factories for llama_cpp
- Pin llama-server download to specific version tag instead of latest
- Add User-Agent header to GitHub API requests
- Fix zombie process: Wait() after Kill() in Close() and ensureServer
- Use http.NewRequestWithContext for health check (cancelable)
- Handle url.Parse error in OllamaProvider constructor
- Sanitize HuggingFace filename to prevent path traversal
- Use int64 for size_bytes in step.model_pull output

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@intel352
intel352 merged commit 2d3c3fd into master Mar 28, 2026
4 checks passed
intel352 added a commit that referenced this pull request Apr 13, 2026
…mand analyzer

mkfs appeared in both DefaultPolicy BlockedPatterns (pre-parse raw check) and
alwaysDestructive map (AST walk per-CallExpr), producing duplicate Risk entries.
Fix: remove mkfs from alwaysDestructive since BlockedPatterns already covers it.
fdisk and wipefs remain in alwaysDestructive (not in BlockedPatterns).

Issue #2 (isAllowed HasPrefix bypass) was already using exact match — no change needed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
intel352 added a commit that referenced this pull request Apr 13, 2026
* chore: add .worktrees to gitignore

* feat(safety): add command analyzer with shell AST static analysis

Uses mvdan.cc/sh to parse commands and detect: destructive operations,
pipe-to-shell, script execution, encoded commands, chained dangerous.
Supports allowlist and blocklist modes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(orchestrator): add blackboard for structured artifact exchange with step types

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(orchestrator): fix blackboard timestamp parsing and step artifact ID

- parseArtifactTime: try sub-second format first, fall back to seconds-only
- BlackboardPostStep: generate UUID before Post() so art.ID is non-empty in step output

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(orchestrator): add hierarchical guardrails module

Scoped rules (agent > team > model > provider), glob-based tool
access, immutable config sections, command safety integration.
GuardrailsModule implements executor.TrustEvaluator for direct
wiring into the executor tool execution path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(safety): add here-doc, process substitution, and variable expansion detection

Closes spec gaps identified in review:
- Here-doc injection: detect shell commands with heredoc redirects (bash << EOF)
- Process substitution: detect shell commands using <(cmd) as input (bash <(curl ...))
- Variable expansion: detect ANSI-C $'\x72\x6d' hex/octal obfuscation
- TestAnalyzer_BlocklistMode, TestAnalyzer_HereDocInjection,
  TestAnalyzer_ProcessSubstitution, TestAnalyzer_VariableExpansionTricks added

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(executor): wire guardrails into agent tool execution loop

findGuardrailsModule() searches service registry for any GuardrailsModule.
Before each tool call: Evaluate() checks tool access, EvaluateCommand()
checks command safety for shell tools. Blocked calls return error result
without reaching the tool registry.

Tests verify: registry lookup, disallowed tool blocking, dangerous command
blocking via the same code path used in the agent loop.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(orchestrator): add self-improvement steps, review pipeline, and deploy strategies

- step.self_improve_validate: YAML parse + immutability constraint check + graceful MCP skip
- step.self_improve_diff: text diff with blackboard posting support
- step.self_improve_deploy: hot_reload / git_pr / canary strategies with mandatory pre-deploy validation gate
- step.lsp_diagnose: LSPProvider interface wrapper as pipeline step
- review_pipeline.go: ReviewPipelineConfig, InputFromBlackboard, InjectBlackboardInput()
- step.agent_execute: input_from_blackboard config injects blackboard artifacts into agent context
- step.agent_execute: guardrails executor integration (findGuardrailsModule already present)
- plugin.go: registered all 4 new step factories

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(safety): eliminate duplicate risks, HasPrefix bypass; wire guardrails into root executor step

- Remove BlockedPatterns check from checkDestructive() — patterns already
  checked pre-parse, so running them again per-CallExpr caused duplicate risk
  entries (e.g. 2 destructive risks for 'rm -rf /')
- Change isAllowed() from HasPrefix to exact match so 'golang-migrate' is
  not permitted when only 'go' is in the allowlist
- Add 6 tests covering: NoDuplicateRisks, AllowlistNoHasPrefixBypass,
  HereDocInjection, ProcessSubstitution, VariableExpansionTricks, BlocklistMode
- Wire TrustEvaluator into root step_agent_execute.go via interface-based
  service registry lookup (avoids circular import with orchestrator package)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(safety): remove duplicate risk entries for BlockedPatterns in command analyzer

mkfs appeared in both DefaultPolicy BlockedPatterns (pre-parse raw check) and
alwaysDestructive map (AST walk per-CallExpr), producing duplicate Risk entries.
Fix: remove mkfs from alwaysDestructive since BlockedPatterns already covers it.
fdisk and wipefs remain in alwaysDestructive (not in BlockedPatterns).

Issue #2 (isAllowed HasPrefix bypass) was already using exact match — no change needed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(guardrails): use typed ActionDeny constant, remove dead code, drop misleading comment

- Replace string(action) == "deny" with action == executor.ActionDeny for
  consistency with the rest of the executor package
- Remove unused guardrailsAction struct and String() method
- Remove misleading "Also handle prefix/*" comment that implied unfinished work;
  the HasSuffix branch already handles this case

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(orchestrator): correct lines_added in diff artifact, fix vacuous test assertion

- postToBlackboard now uses countDiffLines() for lines_added instead of
  len(diff), which was counting all diff lines (added + removed)
- Add content assertion to TestSelfImproveDiffStep_PostToBlackboard
  verifying lines_added is 1, not total diff line count
- Replace empty if-body in TestSelfImproveValidateStep_MCPUnavailable
  with real assertion checking for lsp-unavailable warning
- InjectAs rename (InjectKey→InjectAs) + system_prompt_append/user_message modes
- RequireApproval field added to ReviewPipelineConfig
- LSP diagnostics integrated into SelfImproveValidateStep

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: address Copilot review comments on self-improvement steps

- Use reflect.DeepEqual for immutability diffing (not fmt.Sprintf)
- Add int64/float64 type switches for max_iterations_per_cycle and max_command_length YAML decode
- Clarify Evaluate() comment: uses default rules only, not scope-aware
- Update matchPattern comment to describe exact + trailing-* prefix match behavior
- Document that artifact_type is ignored when latest_only=true in BlackboardReadStep
- Add app == nil guard in InjectBlackboardInput before SvcRegistry() call
- When latest_only=true and artifact_type is set, filter by both phase+type
- Skip blackboard registration if Migrate fails (log error, return early)
- Replace runCommand with exec.CommandContext (2m timeout) + CombinedOutput to capture stderr

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants