Skip to content

feat(config): persist per-project agent config and resolve it at spawn#154

Open
neversettle17-101 wants to merge 8 commits into
mainfrom
feat/issue-107
Open

feat(config): persist per-project agent config and resolve it at spawn#154
neversettle17-101 wants to merge 8 commits into
mainfrom
feat/issue-107

Conversation

@neversettle17-101
Copy link
Copy Markdown
Collaborator

Closes #107

Summary

Wires the previously-dormant per-project agent config half of the project registry. A project can now store agent config (model, permissions, adapter-specific keys) that survives daemon restart and is resolved into the launch command at spawn.

The DB row is authoritative, written through the existing ao project CLI → daemon HTTP → SQLite path (no YAML loader exists in the Go rewrite — see note below).

Changes

  1. Storage — nullable projects.agent_config JSON column (migration 0008). The store marshals/unmarshals so domain.ProjectRecord.AgentConfig is a plain map[string]any (matches ports.AgentConfig, no translation layer). Unset round-trips to nil, not {}.
  2. Resolutionsession_manager loads the project row and populates LaunchConfig.Config before GetLaunchCommand. A missing project yields a nil config (adapter defaults) rather than a spawn failure.
  3. Validationclaude-code now declares a real ConfigSpec (model, permissions) and validates cfg.Config against it at spawn: unknown key, wrong primitive type, or out-of-set enum → clear error. It applies the model override (--model) and a config-driven permission mode (an explicit LaunchConfig.Permissions still wins).
  4. SurfacePUT /api/v1/projects/{id}/agent-config + ao project set-config <id> (--set key=value, --config-json, --clear); config shown in ao project get. OpenAPI + frontend TS schema regenerated.

Note on the issue comments

The "solution design" comment framed this as YAML-authoritative with a write-through cache and a config sync loop. That's the legacy TS model — the Go rewrite has no YAML project loader (global config is env vars; projects come via ao project add). This PR follows the issue body + the first comment: the DB row is the source of truth, written via the CLI/API. The file checklist from that comment is otherwise followed (migration number bumped 00040008).

Acceptance criteria

  • ✅ Config survives daemon restart (persisted in SQLite).
  • ✅ A spawn receives the resolved AgentConfig in LaunchConfig.Config.
  • ✅ Invalid keys/values rejected by the owning adapter with a clear error.
  • ✅ OS-agnostic; no new env-only config for per-project state.

Tests

  • adapter: applies model/permissions, explicit Permissions overrides config, rejects bad config (unknown key / wrong type / bad enum)
  • store: agent_config round-trips (mixed values, unset→nil, clear→nil)
  • service: SetAgentConfig persists + not-found path
  • session manager: spawn resolves project config into the launch command; nil for projects without config

🤖 Generated with Claude Code

Each project can now carry its own agent config (model, permissions,
adapter-specific keys) that survives daemon restart and is resolved into
the launch command when a session spawns.

- storage: add nullable projects.agent_config JSON column (migration 0008);
  marshal/unmarshal in the store so the domain carries map[string]any
- resolution: session manager loads the project row and populates
  LaunchConfig.Config before GetLaunchCommand
- validation: claude-code declares a ConfigSpec (model, permissions) and
  rejects unknown keys / bad types / bad enums at spawn; it applies the
  model override and config-driven permission mode (explicit Permissions
  still wins)
- surface: PUT /projects/{id}/agent-config + `ao project set-config`
  (--set/--config-json/--clear), config shown in `ao project get`

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 7, 2026

Greptile Summary

This PR wires the previously-dormant per-project agent config half of the project registry. A typed domain.ProjectConfig (replacing a free-form map) is persisted as a nullable JSON column, resolved at spawn into LaunchConfig.Config, validated by the owning adapter, and exposed through a new PUT /api/v1/projects/{id}/config endpoint and ao project set-config CLI subcommand.

  • Storage: migration 0008 adds a nullable agent_config column; marshal/unmarshal in the store keeps unset configs as SQL NULL rather than {}.
  • Resolution: Spawn and Restore both call loadProjecteffectiveAgentConfig (merging base + role override), then thread the result through workspace provisioning (BaseBranch, symlinks, post-create commands, rules file) and into the agent launch command.
  • Validation: claudecode now declares a real ConfigSpec and re-validates AgentConfig in GetLaunchCommand as defense-in-depth; ProjectConfig.Validate() is enforced on every write path (service layer).

Confidence Score: 5/5

Safe to merge; the core spawn/restore config-resolution path is correct and well-tested, including the previously-flagged restore fallback, which now properly carries the project config.

The typed-config approach replaces the old free-form map cleanly, validation is enforced at write time (service) and again at launch (adapter), and both Spawn and Restore thread the resolved config through all sub-paths. The two flagged items are confined to authenticated-user-controlled config fields and do not affect spawn correctness or data integrity.

backend/internal/session_manager/manager.go (applySymlinks and projectRules path handling) and backend/internal/storage/sqlite/store/project_store.go (silent unmarshal failure)

Important Files Changed

Filename Overview
backend/internal/session_manager/manager.go Core spawn/restore paths updated to load project config and thread it through to agent launch; path traversal in applySymlinks/projectRules (no confinement check on user-supplied relative paths)
backend/internal/adapters/agent/claudecode/claudecode.go Implements real ConfigSpec (model + permissions enum), applies config in GetLaunchCommand, validates at launch as defense-in-depth; logic is clean
backend/internal/domain/projectconfig.go New ProjectConfig domain type with WithDefaults, IsZero, and Validate; well-structured with appropriate field coverage
backend/internal/domain/agentconfig.go New typed AgentConfig replacing map[string]any; Validate correctly enumerates all PermissionMode values
backend/internal/storage/sqlite/store/project_store.go JSON marshal/unmarshal for ProjectConfig column is correct; unmarshal silently discards parse errors with no log line
backend/internal/service/project/service.go New SetConfig validates config on write, reads-then-upserts project row, and rejects unknown/archived projects correctly
backend/internal/httpd/controllers/projects.go Thin controller for PUT /projects/{id}/config; decodes body, delegates to service, returns standard envelope
backend/internal/cli/project.go New set-config subcommand with named flags and --config-json/--clear; mirrors domain struct cleanly for JSON round-trip
backend/internal/adapters/workspace/gitworktree/workspace.go Passes BaseBranch through to resolveBaseRef; per-project branch override is applied correctly with fallback to adapter default
backend/internal/storage/sqlite/migrations/0008_add_project_config.sql Adds nullable TEXT agent_config column to projects table; backward-compatible migration

Sequence Diagram

sequenceDiagram
    participant CLI as ao project set-config
    participant HTTP as PUT /projects/{id}/config
    participant SVC as project.Service
    participant DB as SQLite store
    participant SM as session_manager.Spawn
    participant WS as gitworktree.Workspace
    participant Agent as claudecode adapter

    CLI->>HTTP: "PUT {config: {model, permissions, ...}}"
    HTTP->>SVC: SetConfig(id, SetConfigInput)
    SVC->>SVC: ProjectConfig.Validate()
    SVC->>DB: GetProject then UpsertProject (JSON column)
    DB-->>SVC: updated row
    SVC-->>CLI: Project read-model

    Note over SM,Agent: At spawn time
    SM->>DB: GetProject(projectID)
    DB-->>SM: "ProjectRecord{Config}"
    SM->>SM: effectiveHarness(kind, config)
    SM->>SM: effectiveAgentConfig(kind, config)
    SM->>WS: "Create(WorkspaceConfig{BaseBranch})"
    SM->>SM: applySymlinks + runPostCreate
    SM->>SM: projectRules (AgentRules + AgentRulesFile)
    SM->>Agent: "GetLaunchCommand(LaunchConfig{Config, Permissions})"
    Agent->>Agent: AgentConfig.Validate()
    Agent-->>SM: argv [claude --model X ...]
Loading

Reviews (6): Last reviewed commit: "fix(config): fail-safe paths for missing..." | Re-trigger Greptile

Comment thread backend/internal/adapters/agent/claudecode/claudecode.go Outdated
Comment thread backend/internal/adapters/agent/claudecode/claudecode.go Outdated
neversettle17-101 and others added 5 commits June 7, 2026 11:14
…led types

Address review on per-project agent config validation:
- handle ConfigFieldStringList (list of strings) explicitly
- reject unhandled ConfigFieldType via a default case rather than
  silently passing
- enforce Required fields are present

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the free-form map[string]any agent config with a typed
domain.AgentConfig{Model, Permissions} so values are validated when set
(CLI/API) instead of silently dropped at spawn, and the OpenAPI/TS schema
and UI get real typed fields.

- domain: AgentConfig struct + Validate(); PermissionMode moves to domain
  and ports re-exports it as a type alias (zero adapter churn)
- storage: marshal/unmarshal the typed struct (IsZero → SQL NULL)
- service: validate on Add and SetAgentConfig; read-model exposes a typed
  *AgentConfig
- claudecode: read typed cfg.Config.Model/.Permissions; drop the
  map/spec-based validateConfig in favor of the typed Validate()
- cli: typed `ao project set-config --model/--permission/--clear`
- docs: add docs/design/per-project-config.md blueprint sequencing the
  remaining # Projects fields toward fully typed per-project config

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…urface)

Expand per-project config from agentConfig-only to the full legacy
`projects.<id>` surface, modeled as one typed domain.ProjectConfig
persisted in a single projects.config JSON column.

Wired end-to-end at spawn:
- defaultBranch  → base branch for the session worktree (ports.WorkspaceConfig.BaseBranch)
- env            → merged into the runtime env (AO-internal vars still win)
- symlinks       → repo files linked into the workspace
- postCreate     → commands run in the workspace (OS-agnostic shell)
- agentRules / agentRulesFile / orchestratorRules → merged into the prompt
- worker/orchestrator role overrides → harness + agent-config resolution

Stored + validated + surfaced now, consumption deferred (no consumer yet):
tracker, scm(+webhook), opencodeIssueSessionStrategy; sessionPrefix feeds
the display prefix only (session-id generation unchanged).

Validation lives on domain.ProjectConfig.Validate() and runs when config is
set (CLI/API). PermissionMode/AgentConfig stay typed; harness names validated
via domain.AgentHarness.IsKnown().

Surface: PUT /projects/{id}/config (replaces /agent-config) + typed
`ao project set-config` flags (--default-branch/--env/--symlink/--post-create/
--agent-rules/--worker-agent/… or --config-json). OpenAPI + TS regenerated.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add domain.DefaultProjectConfig / ProjectConfig.WithDefaults with a single
DefaultBranchName ("main") source of truth, replacing the literal "main"
scattered in the read-model and the gitworktree adapter. Unconfigured
projects now resolve the default branch through one path; every other field
defaults to its zero value.

Tests: defaults present for all fields (DefaultProjectConfig/WithDefaults),
and an unconfigured project reports the default branch + derived session
prefix while omitting the empty config object.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread backend/internal/domain/projectconfig.go
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@neversettle17-101
Copy link
Copy Markdown
Collaborator Author

Review: default-config / fail-safe paths

Reviewing specifically from the lens of: if a user defines no config (or partial config), spawning should never fail.

The overall direction is correct — loadProject returns a zero record on miss, effectiveAgentConfig merges over zero safely, and WithDefaults() ensures BaseBranch is always "main" even with no config. But there are four places where a missing or bad config can still surface as a hard failure.


1. AgentRulesFile turns a typo into a spawn failure (high impact)

projectRules in session_manager/manager.go treats a missing rules file as an error that aborts spawn:

data, err := os.ReadFile(path)
if err != nil {
    return "", fmt.Errorf("agent rules file: %w", err)  // ← spawn fails
}

If a user sets agentRulesFile: .rules.md and that file is later deleted, renamed, or was never there, every subsequent spawn for that project fails — silently from their perspective, because the error is buried in spawn logs. The file is optional context, not a hard dependency.

Suggested fix: treat a missing file as a warning, not an error. Return the inline rules without the file contents, and log a warning (or surface it on the session record).

data, err := os.ReadFile(path)
if err != nil {
    if !errors.Is(err, os.ErrNotExist) {
        return "", fmt.Errorf("agent rules file: %w", err)
    }
    // Missing file: skip silently or log, don't abort spawn
} else {
    rules = appendPromptSection(rules, strings.TrimRight(string(data), "\n"))
}

2. Corrupted config JSON in DB makes the project inaccessible (medium impact)

unmarshalProjectConfig returns an error on bad JSON, which propagates through GetProject, ListProjects, and FindProjectByPath. If a row's config column ever contains invalid JSON (direct DB edit, migration bug, encoding edge case), every operation on that project — and every list call — fails.

func unmarshalProjectConfig(s sql.NullString) (domain.ProjectConfig, error) {
    ...
    if err := json.Unmarshal([]byte(s.String), &cfg); err != nil {
        return domain.ProjectConfig{}, fmt.Errorf("unmarshal project config: %w", err)
        // ↑ this causes ListProjects to fail entirely
    }
}

Suggested fix: on unmarshal error, log the error and return a zero config rather than propagating. The project row is still valid; only the config is damaged. This matches how the legacy TS handled resolveError — degrade gracefully rather than blocking all access.


3. Restore path does not apply AgentConfig to the restore command (low impact, but inconsistent)

The Spawn path correctly passes effectiveAgentConfig(...) into LaunchConfig.Config. The Restore path (around line 383 in the diff) loads the project for Env but the argv for restore is built from GetRestoreCommand(ctx, RestoreConfig{...}) — which receives an empty Config (the RestoreConfig struct has a Config AgentConfig field, but it isn't populated from project.Config in this PR).

So a project configured with model: claude-opus-4-5 will get that model on fresh spawns, but not on restores. Probably fine for now but worth noting so a follow-up wires it.


4. runtime.GOOS inline check in runPostCreate violates the cross-platform rule

if runtime.GOOS == "windows" {
    cmd = exec.CommandContext(ctx, "cmd", "/c", command)
} else {
    cmd = exec.CommandContext(ctx, "sh", "-c", command)
}

The go-rewrite skill's Golden Rule says: never inline runtime.GOOS at call sites — put it in the platform package. This should use getShell() (or whatever the Go rewrite's platform helper is) so Windows pty-host nuances (Git Bash, AO_SHELL override, etc.) are respected consistently. This is a spawn failure risk on Windows if cmd /c is unavailable or the wrong shell is used.


What is already handled correctly

  • loadProject with !ok → returns zero ProjectRecord{} → all field accessors on zero config return safe zero values. ✓
  • effectiveAgentConfig on zero config → returns zero AgentConfig{} → adapter emits no flags. ✓
  • WithDefaults() always fills DefaultBranchBaseBranch is never empty. ✓
  • AgentConfig.Validate() accepts "" for both fields → zero config always passes validation at launch time. ✓
  • applySymlinks / runPostCreate on nil slices → no-ops. ✓
  • spawnEnv with nil projectEnv → range over nil map is a no-op. ✓

The one structural note: IsZero uses reflect.DeepEqual. This is correct today but fragile — if ProjectConfig ever gains a field whose zero value is non-nil (a pointer, interface, or function), reflect.DeepEqual will return false for semantically-zero configs. A hand-written IsZero comparing each field explicitly is more robust long-term.

🤖 Generated with Claude Code

Address review on default-config / fail-safe spawning:
- projectRules: a missing AgentRulesFile is optional context, skipped
  rather than aborting every spawn (only a real read error surfaces)
- store: a corrupt config JSON column degrades to a zero config instead
  of failing GetProject/ListProjects/FindProjectByPath for that row
- restore: re-apply the project's resolved AgentConfig so a configured
  model/permissions carry across a restore (matches fresh spawn)

Tests: missing rules file skips, corrupt config degrades to zero, restore
applies the project agent config.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@neversettle17-101
Copy link
Copy Markdown
Collaborator Author

Thanks — great fail-safe review. Addressed in 1f86e8c:

1. AgentRulesFile typo → spawn failure (high) — Fixed. projectRules now treats a missing file (os.ErrNotExist) as absent optional context and returns the inline rules without aborting; only a real read error (e.g. permissions) surfaces. Test flipped to assert the skip.

2. Corrupt config JSON blocks the project (medium) — Fixed. unmarshalProjectConfig now degrades a damaged config column to a zero config instead of erroring, so the project row and ListProjects stay accessible. projectRowFromGen / GetProject / ListProjects / FindProjectByPath no longer propagate an unmarshal error. (The store has no logger, so it degrades silently rather than logging — happy to surface it as a resolveError on the read-model in a follow-up if you'd like.) Added an internal test for NULL / valid / corrupt.

3. Restore doesn't apply AgentConfig (low) — Wired it now rather than deferring: Restore loads the project and passes effectiveAgentConfig(rec.Kind, project.Config) into both GetRestoreCommand's RestoreConfig.Config and the fallback LaunchConfig. Test added.

4. runtime.GOOS inline in runPostCreate — Kept inline, intentionally: there is no platform package in this repo, and runtime.GOOS is the established pattern — adapters/runtime/zellij/zellij.go inlines it 3× and all ~25 agent adapters do the same. Introducing a platform package for this one call site would be a single-impl abstraction against the codebase grain. If the team wants a shared shell helper, that's worth its own PR sweeping zellij + the adapters too, and I'm glad to file it.

IsZero / reflect.DeepEqual note — Keeping reflect.DeepEqual(c, ProjectConfig{}): comparing a value against its type's zero value is robust for every field kind — a nil pointer/interface/func/slice/map all DeepEqual their zero counterpart, so a semantically-zero config always reports zero. It can't regress the way a stale hand-written comparison can when a field is added. Open to switching if you still prefer explicit.

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.

feat(config): persist per-project agent config and resolve it at spawn

1 participant