Skip to content

feat(ai): add optional backend commands#118

Open
CoderMungan wants to merge 3 commits into
ActiveMemory:mainfrom
CoderMungan:feat/issue-92-ai-backend
Open

feat(ai): add optional backend commands#118
CoderMungan wants to merge 3 commits into
ActiveMemory:mainfrom
CoderMungan:feat/issue-92-ai-backend

Conversation

@CoderMungan

Copy link
Copy Markdown
Member

No description provided.

@CoderMungan CoderMungan force-pushed the feat/issue-92-ai-backend branch from 294dd07 to 791785d Compare June 19, 2026 13:16
@CoderMungan CoderMungan requested a review from bilersan as a code owner June 19, 2026 13:16
@CoderMungan

Copy link
Copy Markdown
Member Author

This PR is the full implementation of Block A: AI Backend as specified in
issue #92 (specs/ctx-ai-backend.md).

What ships:

  • internal/backend/Backend interface, Registry, and OpenAI-compatible
    HTTP client
  • internal/backend/vllm.go + openaicompat.go — vLLM and generic
    openai-compatible backends
  • internal/backend/builtin_internal.go — thin wrappers for openai,
    anthropic, ollama, lmstudio
  • internal/cli/ai/ctx ai ping and ctx ai extract commands (Option 1:
    new ctx ai top-level namespace)
  • internal/cli/setup/core/backend/ctx setup --backend <name> extension;
    templates endpoint + auth wiring into .ctxrc
  • internal/rc/.ctxrc backends: YAML table parsing and validation
  • internal/compliance/ai_boundary_test.go — deterministic-core boundary guard;
    CI fails if ctx agent / ctx status / hooks ever import internal/backend
    (structural enforcement of Invariant 2)
  • internal/cli/ai/core/run/ — validation consumer: schema-constrained
    completion → proposal artifact write (.context/*.md files are never touched
    directly)
  • docs/cli/ai.md, docs/cli/setup.md, docs/recipes/local-inference-with-vllm.md
    — CLI reference and recipe docs

Related issues and PRs:

@CoderMungan CoderMungan requested a review from Copilot June 19, 2026 13:25
@CoderMungan CoderMungan requested review from hamzaerbay and v0lkan and removed request for Copilot June 19, 2026 13:27
@josealekhine

Copy link
Copy Markdown
Member

Thanks for your contributions @CoderMungan

This is the structural enforcement of Invariant 2 ("zero runtime deps for core functionality") and is the strongest thing in the PR.

Security posture — good

  • API key is read from env only, never written to .ctxrc, never logged, never echoed in error bodies:

    // internal/backend/openaicompat_internal.go (headers)
    apiKey := os.Getenv(backend.config.APIKeyEnv)
    if apiKey == "" { return }
    request.Header.Set(cfgBackend.HeaderAuthorization, cfgBackend.AuthorizationBearerPrefix+apiKey)
  • TLS verification on by default (no InsecureSkipVerify); request timeout enforced with a safe 30s fallback; context propagated.

  • Fail-closed registry semantics (no-backend / multiple / missing) match the spec and are tested.

  • Typed error sentinels with Unwrap + externalized message constants — matches repo conventions.

  • The CLI namespace decision (Option 1, ctx ai <verb>) was recorded in DECISIONS.md as the spec's first task required.


Findings worth acting on

1. (functional, headline) Schema-constrained output is plumbed but never sent

Propose passes a schema, but Complete drops it:

// internal/cli/ai/core/run/run.go (Propose)
response, completeErr := resolved.backend.Complete(ctx, backendPkg.Request{
    Prompt: cfgAI.PromptPrefix + emit + token.NewlineLF + string(data),
    Schema: cfgAI.SchemaMinimal,   // <-- set here
})
// internal/backend/openaicompat.go (Complete)
payload := chatRequest{                 // <-- chatRequest has only Model + Messages
    Model:    model,
    Messages: []chatMessage{{Role: cfgBackend.RoleUser, Content: req.Prompt}},
}
// req.Schema is never referenced

chatRequest (types.go) has no response_format/schema field, and TestOpenAICompatibleCompleteSuccess confirms the wire shape is just {model, messages}. So Request.Schema and SchemaMinimal are dead, and propose relies entirely on the prompt prefix coaxing JSON, then:

decoded := map[string]any{}
decodeErr := json.Unmarshal([]byte(response.Text), &decoded)  // fails on ```json fences or any prose

This is the spec's core value prop ("schema-constrained structured outputs", vLLM guided_json). For a validation-only Block A it half-works, but it overclaims. Either wire response_format: {"type":"json_object"} (and vLLM guided_json/extra_body) into chatRequest, or drop Schema/SchemaMinimal and document JSON as best-effort so the type doesn't imply enforcement.

2. (spec deviation) Malformed backend tables warn instead of refuse

The spec's Edge Cases / Validation Rules say:

.ctxrc malformed (missing required key) → Refuse with a clear parse error naming the offending key; do not silently default.

But the implementation downgrades unknown backend sub-keys to a warning, and a test locks that in:

// internal/rc/validate_test.go
func TestValidate_BackendsUnknownNestedField(t *testing.T) {
    data := []byte("backends:\n  vllm:\n    endpoint: http://localhost:8000\n    api_token: nope\n")
    warnings, err := Validate(data)
    if err != nil { t.Fatalf("unexpected error: %v", err) }   // <-- asserts NO error
    if len(warnings) == 0 { t.Fatal("expected warning for unknown backend field") }
}
// internal/rc/validate.go — only two shapes hard-fail; everything else warns
if backendsShapeError(te.Errors) { return nil, decErr }
return te.Errors, nil

So a typo like endpont: is a warning, the field silently doesn't apply, and at runtime the backend loads with an empty endpoint (caught only later at dispatch). It's intentional and tested, but it contradicts the spec. Reconcile one or the other — if backends are meant to be stricter than the repo's general unknown-field tolerance, route their key errors through a sentinel that backendsShapeError treats as fatal.

3. (correctness) Endpoint path is overwritten, not joined

// internal/backend/openaicompat_internal.go (url)
parsed.Path = path   // discards any path already in the configured endpoint

endpoint: https://host/openai → request goes to https://host/v1/models, dropping /openai. That breaks Azure-style and reverse-proxied/subpath OpenAI-compatible deployments. The spec's examples are all bare hosts so it passes today, but it's a real-world footgun. Prefer joining:

parsed.Path = strings.TrimRight(parsed.Path, "/") + path

4. (spec deviation, UX) Setup doesn't validate the endpoint

validate(options) checks only the backend name:

// internal/cli/setup/core/backend/config_internal.go (validate)
if defaultEndpoint(options.Name) == "" && options.Name != cfgBackend.NameOpenAICompatible {
    return setupErr.UnsupportedBackend(options.Name)
}
return nil

The spec says endpoint http(s) is enforced "at setup time," but it's only checked at dispatch (url()). Worse, ctx setup --backend openai-compatible with no --endpoint writes an empty endpoint (that type has no default) and setup accepts it — the user finds out only on the next load/ping. Add a setup-time endpoint presence + scheme check.

5. (hardening) io.ReadAll(response.Body) is unbounded

// internal/backend/openaicompat_internal.go (do)
raw, readErr := io.ReadAll(response.Body)

A misbehaving or compromised backend can return a huge/streamed body and OOM the CLI (and it flows verbatim into Upstream.Body error strings). The backend is user-configured so severity is low, but an io.LimitReader cap is a cheap guard.

6. (couldn't verify here) gosec G107

golangci-lint isn't installed in my environment, so I couldn't run make lint. The diff correctly adds G101 exclusions for the new env-var-name constants:

# .golangci.yml
- linters: [gosec]
  text: "G101"
  path: "internal/config/backend/"

…but http.NewRequestWithContext(ctx, method, endpoint, …) uses a variable URL, which gosec's G107 often flags, and there's no scoped exclusion or //nosec for internal/backend/. If your CI gosec is stricter than what the author ran, this could fail the lint gate — worth a quick confirm. (If it does fire, a path-scoped exclusion or //nosec G107 with a rationale comment is the established pattern here.)


Minor

  • Downstream BASE_URL templating not implemented. The task list calls for templating ANTHROPIC_BASE_URL/OPENAI_BASE_URL into AI-tool configs; setup writes .ctxrc only, and the "env conflict" warning is repurposed to warn when the API-key env var is already set. Reasonable for Block A, but a scope reduction vs. the tasks.
  • Upstream.Error() omits the status code — it includes Body but not StatusCode (the field is stored but not surfaced in the message).
  • Proposal filename collisionTimestampLayout = "20060102T150405Z" is second-resolution, so two propose runs in the same second overwrite each other. Add sub-second precision or a short suffix.
  • propose --emit is neither required nor defaulted — empty emit still dispatches with an empty kinds list.

Signed-off-by: CoderMungan <codermungan@gmail.com>
Signed-off-by: CoderMungan <codermungan@gmail.com>
Signed-off-by: CoderMungan <codermungan@gmail.com>
@CoderMungan CoderMungan force-pushed the feat/issue-92-ai-backend branch from 4c0c05c to cbbae03 Compare June 26, 2026 11:26
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