From 928476bbe85395b1d5e3175107d3ada4c6bbbe18 Mon Sep 17 00:00:00 2001 From: agentfactory Date: Tue, 5 May 2026 03:04:20 +0000 Subject: [PATCH 1/3] design: dimension analyses for af-5836e28c (agent-gen output location) Analyzes the agent-gen output location problem across 6 dimensions: API, Data, UX, Scale, Security, and Integration. Recommends a shared config.ResolveSourceRoot() function that reuses the existing AF_SOURCE_ROOT pattern to route .md.tmpl files to the af source tree. Co-Authored-By: Claude Opus 4.6 --- .designs/af-5836e28c/api.md | 82 ++++++++++++++++ .designs/af-5836e28c/constraints.md | 61 ++++++++++++ .designs/af-5836e28c/data.md | 66 +++++++++++++ .designs/af-5836e28c/integration.md | 118 ++++++++++++++++++++++++ .designs/af-5836e28c/problem-summary.md | 27 ++++++ .designs/af-5836e28c/scale.md | 40 ++++++++ .designs/af-5836e28c/security.md | 67 ++++++++++++++ .designs/af-5836e28c/ux.md | 80 ++++++++++++++++ 8 files changed, 541 insertions(+) create mode 100644 .designs/af-5836e28c/api.md create mode 100644 .designs/af-5836e28c/constraints.md create mode 100644 .designs/af-5836e28c/data.md create mode 100644 .designs/af-5836e28c/integration.md create mode 100644 .designs/af-5836e28c/problem-summary.md create mode 100644 .designs/af-5836e28c/scale.md create mode 100644 .designs/af-5836e28c/security.md create mode 100644 .designs/af-5836e28c/ux.md diff --git a/.designs/af-5836e28c/api.md b/.designs/af-5836e28c/api.md new file mode 100644 index 0000000..e5f26a6 --- /dev/null +++ b/.designs/af-5836e28c/api.md @@ -0,0 +1,82 @@ +# API & Interface Design + +## Summary +The core interface change is how `agent-gen` resolves the af source tree for template placement. The CLI interface (`af formula agent-gen `) must remain unchanged, but internally the command needs to resolve two distinct paths: the factory root (for workspace artifacts) and the af source root (for .md.tmpl files). The resolution should follow the existing `AF_SOURCE_ROOT` pattern already used by mcpstore. + +## Constraint Check +- [x] Templates in AF_SRC: This dimension directly addresses where templates land +- [x] Workspace in factory root: API changes don't affect workspace path logic +- [x] quickstart.sh single entry: CLI interface stays the same +- [x] --delete cleans both: --delete path must also resolve source root +- [x] -o works without source tree: -o never writes files, unaffected +- [x] Self-hosted case: Factory root with agentfactory go.mod IS the source root +- [x] External project case: AF_SOURCE_ROOT provides the path +- [x] Reuse AF_SOURCE_ROOT: Yes, extending existing pattern +- [x] No CLI breaking changes: Interface stays identical + +## Options Explored + +### Option 1: Shared `config.ResolveSourceRoot(factoryRoot)` function +- **Description**: Extract source root resolution from mcpstore into `internal/config/` as a shared function. The function checks: (1) factory root has `go.mod` with "agentfactory" → use factory root, (2) build-time sourceRoot variable, (3) `AF_SOURCE_ROOT` env var. `agent-gen` calls this to determine where to write `.md.tmpl` files. +- **Constraint Compliance**: All constraints pass +- **Pros**: Reuses existing pattern; single source of truth for source root resolution; testable in isolation; no CLI changes needed +- **Cons**: Requires threading build-time sourceRoot from cmd/af/main.go to config package (new package-level var or init function) +- **Effort**: Medium +- **Reversibility**: Easy — function is additive, old behavior is still available + +### Option 2: New `--source-root` flag on agent-gen +- **Description**: Add explicit `--source-root` flag to agent-gen that overrides the template output directory. +- **Constraint Compliance**: Technically passes all, but violates the spirit of "no CLI changes" and "reuse AF_SOURCE_ROOT" +- **Pros**: Explicit, easy to understand +- **Cons**: Another flag to manage; doesn't help agent-gen-all.sh unless flag is threaded through; parallel to AF_SOURCE_ROOT rather than reusing it +- **Effort**: Low +- **Reversibility**: Easy +- REJECTED: Violates "reuse existing AF_SOURCE_ROOT pattern" — creates parallel mechanism + +### Option 3: Auto-detect via `go.mod` walk-up +- **Description**: Walk up from the `af` binary's location to find the source tree by looking for `go.mod` with "agentfactory". +- **Constraint Compliance**: Fails for installed binaries where source tree isn't at binary location +- **Pros**: Zero configuration needed +- **Cons**: Fragile — binary may be in `~/.local/bin/`, not in source tree; doesn't work for go-installed binaries +- **Effort**: Low +- **Reversibility**: Easy +- REJECTED: Violates self-hosted + external case constraints — binary location != source location + +## Recommendation +**Option 1**: Shared `config.ResolveSourceRoot()` function. It aligns with the existing `AF_SOURCE_ROOT` pattern, requires no CLI changes, and handles both self-hosted and external cases. + +### Function Signature +```go +// config/source_root.go +func ResolveSourceRoot(factoryRoot string) (string, error) +``` + +Resolution order: +1. If `factoryRoot` contains `go.mod` referencing "agentfactory" → return factoryRoot +2. If build-time `sourceRoot` is set → return sourceRoot +3. If `AF_SOURCE_ROOT` env var is set → return its value +4. Return error with message listing all three paths checked + +### Changes to formula.go +```go +// In runFormulaAgentGen(), after line 109 (factory root found): +srcRoot, err := config.ResolveSourceRoot(root) +if err != nil { + return fmt.Errorf("cannot determine af source tree for template placement: %w\n"+ + "Set AF_SOURCE_ROOT to the agentfactory source directory", err) +} + +// Line 200 changes from: +tmplDir := filepath.Join(root, "internal", "templates", "roles") +// to: +tmplDir := filepath.Join(srcRoot, "internal", "templates", "roles") +``` + +## Dependencies Produced +- config.ResolveSourceRoot() must be available before formula.go can use it +- Build-time sourceRoot variable must be accessible from config package +- agent-gen-all.sh must export AF_SOURCE_ROOT=$AF_SRC before calling agent-gen + +## Risks Identified +- **Build-time var threading**: sourceRoot is set in cmd/af/main.go via ldflags; need to make it accessible to config package. Severity: Low. Mitigation: Package-level setter function. +- **go.mod detection false positive**: A target project could theoretically have a go.mod referencing "agentfactory" as a dependency. Severity: Low. Mitigation: Check for module path match, not just any reference. diff --git a/.designs/af-5836e28c/constraints.md b/.designs/af-5836e28c/constraints.md new file mode 100644 index 0000000..141c523 --- /dev/null +++ b/.designs/af-5836e28c/constraints.md @@ -0,0 +1,61 @@ +# Constraint Verification Checklist + +## Constraint: Templates must end up in `$AF_SRC/internal/templates/roles/` for `//go:embed` compilation +- **Prohibits:** Writing .md.tmpl files only to the factory root's internal/ tree when it differs from AF_SRC +- **Requires:** agent-gen must resolve the af source tree path and write templates there +- **Verification:** After agent-gen, check `$AF_SRC/internal/templates/roles/.md.tmpl` exists +- **Relaxation Impact:** If relaxed, templates wouldn't compile into the binary — `af prime` and `af install` would fail to render CLAUDE.md for formula-generated agents + +## Constraint: Workspace artifacts must stay in factory root +- **Prohibits:** Moving agents.json, workspace dirs, CLAUDE.md, or settings.json out of the factory root +- **Requires:** `.agentfactory/agents//`, `.agentfactory/agents.json` remain relative to factory root +- **Verification:** After agent-gen, confirm artifacts exist under factory root's `.agentfactory/` +- **Relaxation Impact:** Would break `af prime`, `af up`, `af install` which all use factory root paths + +## Constraint: quickstart.sh remains single entry point +- **Prohibits:** Requiring users to run additional scripts or manual copy steps +- **Requires:** quickstart.sh + agent-gen-all.sh handle everything end-to-end +- **Verification:** Fresh clone → quickstart.sh → agents work (no manual intervention) +- **Relaxation Impact:** Increases onboarding friction; newcomers won't know the workflow + +## Constraint: agent-gen --delete must clean both sides +- **Prohibits:** Orphaned templates in AF_SRC or orphaned workspaces in factory root +- **Requires:** --delete removes template from AF_SRC and workspace from factory root +- **Verification:** After `agent-gen --delete`, neither template nor workspace exists +- **Relaxation Impact:** Template orphans accumulate in binary, wasting size and causing confusion + +## Constraint: -o (dry run) works without source tree detection +- **Prohibits:** Requiring AF_SOURCE_ROOT for stdout-only output +- **Requires:** -o flag renders template content to stdout without writing any files +- **Verification:** `af formula agent-gen -o` succeeds without AF_SOURCE_ROOT set +- **Relaxation Impact:** Would make dry-run mode less useful for debugging/development + +## Constraint: Self-hosted case must work (af source == factory root) +- **Prohibits:** Hard-coding separate paths; assuming source tree is always different from factory root +- **Requires:** When factory root contains `go.mod` with agentfactory, template path == factory root path +- **Verification:** Run agent-gen in the agentfactory repo itself — templates land in correct place +- **Relaxation Impact:** Would break development on agentfactory itself + +## Constraint: External project case must work (af source != factory root) +- **Prohibits:** Assuming source tree is always the factory root +- **Requires:** AF_SOURCE_ROOT env var or build-time sourceRoot to locate the separate af source tree +- **Verification:** Run agent-gen from ~/af/myproject/ with AF_SOURCE_ROOT=~/projects/agentfactory — templates go to AF_SOURCE_ROOT +- **Relaxation Impact:** Would break the primary use case described in the problem statement + +## Constraint: Reuse existing AF_SOURCE_ROOT pattern +- **Prohibits:** Creating a new env var or config mechanism for the same purpose +- **Requires:** Extending the existing `AF_SOURCE_ROOT` / build-time `sourceRoot` pattern from mcpstore to agent-gen +- **Verification:** Same env var works for both mcpstore Python path resolution and agent-gen template placement +- **Relaxation Impact:** Would create parallel discovery mechanisms, increasing cognitive load + +## Constraint: make build still required after template changes +- **Prohibits:** Expecting templates to work without recompilation +- **Requires:** agent-gen output message still says "Run 'make build' to compile the new template" +- **Verification:** User message after agent-gen mentions make build requirement +- **Relaxation Impact:** N/A — this is a fundamental Go embed constraint + +## Constraint: No breaking changes to agent-gen CLI interface +- **Prohibits:** Changing required arguments, removing flags, altering exit codes +- **Requires:** Existing `af formula agent-gen ` invocations continue to work +- **Verification:** Existing scripts (agent-gen-all.sh) work without modification beyond env var setting +- **Relaxation Impact:** Would break existing automation and documentation diff --git a/.designs/af-5836e28c/data.md b/.designs/af-5836e28c/data.md new file mode 100644 index 0000000..6e7dee4 --- /dev/null +++ b/.designs/af-5836e28c/data.md @@ -0,0 +1,66 @@ +# Data Model + +## Summary +This problem has minimal data model impact — no new storage, no schemas, no migrations. The only data-relevant change is the path routing for where `.md.tmpl` files land on disk. The existing file formats (Go templates, agents.json, CLAUDE.md) remain unchanged. The key data concern is the go.mod detection heuristic for identifying the af source tree. + +## Constraint Check +- [x] Templates in AF_SRC: Addressed by path routing, not data model +- [x] Workspace in factory root: No data model changes needed +- [x] Reuse AF_SOURCE_ROOT: Env var is data input, no storage needed +- [x] Self-hosted case: go.mod detection is a read-only check + +## Options Explored + +### Option 1: go.mod module path check (recommended) +- **Description**: Read `go.mod` in factory root, parse the `module` line, check if it matches `github.com/stempeck/agentfactory` (or contains "agentfactory" in the module path). +- **Constraint Compliance**: All pass +- **Pros**: Precise — distinguishes the af source tree from projects that merely import it; uses Go standard library (`go/modfile` or simple string parsing) +- **Cons**: Slightly more complex than a simple file existence check +- **Effort**: Low +- **Reversibility**: Easy + +### Option 2: Check for `internal/templates/roles/` directory existence +- **Description**: If factory root has `internal/templates/roles/`, assume it's the source tree. +- **Constraint Compliance**: Passes but fragile +- **Pros**: Simple +- **Cons**: False positives if target project happens to have that path; false negatives if directory hasn't been created yet +- **Effort**: Low +- **Reversibility**: Easy +- REJECTED: Too fragile — could match non-af projects with similar structure + +### Option 3: Marker file `.agentfactory-source` +- **Description**: Place a marker file in the af source root that agent-gen checks for. +- **Constraint Compliance**: Passes +- **Pros**: Explicit, no false positives +- **Cons**: Extra file to maintain; easy to forget; not self-documenting +- **Effort**: Low +- **Reversibility**: Easy +- REJECTED: Adds maintenance burden for something go.mod already solves + +## Recommendation +**Option 1**: go.mod module path check. Simple, precise, uses existing data (go.mod is always present in the source tree). Parse just the first line starting with "module " — no need for full go.mod parser. + +### Detection Logic +```go +func isAgentFactorySourceTree(dir string) bool { + data, err := os.ReadFile(filepath.Join(dir, "go.mod")) + if err != nil { + return false + } + for _, line := range strings.SplitN(string(data), "\n", 3) { + if strings.HasPrefix(line, "module ") { + return strings.Contains(line, "agentfactory") + } + } + return false +} +``` + +## Dependencies Produced +- This detection logic is used by `config.ResolveSourceRoot()` (from API dimension) + +## Risks Identified +- **Module path rename**: If the agentfactory module path changes, detection breaks. Severity: Very Low. Mitigation: The detection string "agentfactory" is broad enough to survive minor path changes. + +## Constraints Identified +- None new — this dimension operates within existing data structures. diff --git a/.designs/af-5836e28c/integration.md b/.designs/af-5836e28c/integration.md new file mode 100644 index 0000000..c761464 --- /dev/null +++ b/.designs/af-5836e28c/integration.md @@ -0,0 +1,118 @@ +# Integration + +## Summary +This change touches three integration surfaces: (1) `formula.go` — the Go command that writes templates, (2) `agent-gen-all.sh` — the batch script that calls agent-gen repeatedly, and (3) `quickstart.sh` — the bootstrap script that builds af and provisions agents. The `agent-gen-all.sh` script already has `$AF_SRC` and sets working directory correctly; it just needs to export `AF_SOURCE_ROOT`. The `quickstart.sh` already exports `AF_SOURCE_ROOT` on line 419. + +## Constraint Check +- [x] Templates in AF_SRC: integration.go changes route templates there +- [x] Workspace in factory root: No changes to workspace routing +- [x] quickstart.sh single entry: quickstart.sh already sets AF_SOURCE_ROOT +- [x] --delete cleans both: --delete must resolve source root for template removal +- [x] agent-gen-all.sh: Must export AF_SOURCE_ROOT=$AF_SRC +- [x] Reuse AF_SOURCE_ROOT: Same env var used everywhere + +## Options Explored + +### Option 1: Minimal changes — export env var in agent-gen-all.sh + use in formula.go +- **Description**: + - `agent-gen-all.sh`: Add `export AF_SOURCE_ROOT="$AF_SRC"` before the agent-gen loop + - `formula.go`: Use `config.ResolveSourceRoot()` for template path, keep factory root for workspace path + - `formula.go` `--delete`: Also use `config.ResolveSourceRoot()` to find template to delete + - `quickstart.sh`: Already correct (sets AF_SOURCE_ROOT on line 419) +- **Constraint Compliance**: All pass +- **Pros**: Minimal changes; uses existing infrastructure; agent-gen-all.sh already has $AF_SRC; quickstart.sh already exports AF_SOURCE_ROOT +- **Cons**: None significant +- **Effort**: Low +- **Reversibility**: Easy + +### Option 2: Make agent-gen-all.sh pass --source-root flag +- **Description**: Add a --source-root flag and thread it through the script. +- **Constraint Compliance**: Violates "reuse AF_SOURCE_ROOT" +- **Pros**: Explicit +- **Cons**: Parallel mechanism; more script changes; doesn't help standalone agent-gen +- **Effort**: Medium +- **Reversibility**: Easy +- REJECTED: Parallel to AF_SOURCE_ROOT + +### Option 3: Embed source root in agents.json +- **Description**: Store resolved source root in agents.json so --delete can find it later. +- **Constraint Compliance**: Passes but adds data model complexity +- **Pros**: --delete always knows where templates went +- **Cons**: Couples runtime data to build-time paths; paths go stale on directory moves +- **Effort**: Medium +- **Reversibility**: Moderate +- REJECTED: Unnecessary complexity — --delete can resolve source root the same way --create does + +## Recommendation +**Option 1**: Minimal changes. The pieces are already in place: +- `agent-gen-all.sh` knows `$AF_SRC` — just export it as `AF_SOURCE_ROOT` +- `quickstart.sh` already exports `AF_SOURCE_ROOT` +- `formula.go` needs one additional function call to resolve source root + +### Changes Required + +**`agent-gen-all.sh`** (1 line added): +```bash +# After line 49 (PROJECT="$(pwd)"), add: +export AF_SOURCE_ROOT="$AF_SRC" +``` + +**`formula.go` `runFormulaAgentGen()`** (3 lines changed): +```go +// After finding factory root (line 109), add: +srcRoot, err := config.ResolveSourceRoot(root) +if err != nil { + return fmt.Errorf("cannot determine af source tree for template placement: %w\n"+ + "Set AF_SOURCE_ROOT to the agentfactory source directory", err) +} + +// Line 200: change root to srcRoot +tmplDir := filepath.Join(srcRoot, "internal", "templates", "roles") + +// Line 208: Update output message to show full path when different from factory root +if srcRoot != root { + fmt.Fprintf(cmd.ErrOrStderr(), "✓ Role template written: %s/%s.md.tmpl\n", tmplDir, agentName) +} else { + fmt.Fprintf(cmd.ErrOrStderr(), "✓ Role template written: internal/templates/roles/%s.md.tmpl\n", agentName) +} +``` + +**`formula.go` `runFormulaAgentGenDelete()`** (2 lines changed): +```go +// After finding factory root (line 283), add: +srcRoot, err := config.ResolveSourceRoot(root) +if err != nil { + return fmt.Errorf("cannot determine af source tree for template cleanup: %w", err) +} + +// Line 307: change root to srcRoot +tmplPath := filepath.Join(srcRoot, "internal", "templates", "roles", agentName+".md.tmpl") +``` + +**`config/source_root.go`** (new file): +```go +package config + +func ResolveSourceRoot(factoryRoot string) (string, error) { /* ... */ } +func isAgentFactorySourceTree(dir string) bool { /* ... */ } + +// SetBuildSourceRoot is called from cmd/af/main.go to inject the build-time value. +var buildSourceRoot string +func SetBuildSourceRoot(root string) { buildSourceRoot = root } +``` + +**`cmd/af/main.go`** (1 line added): +```go +config.SetBuildSourceRoot(sourceRoot) +``` + +## Dependencies Produced +- `config.SetBuildSourceRoot()` must be called before any command runs +- `AF_SOURCE_ROOT` must be documented in USING_AGENTFACTORY.md + +## Risks Identified +- **Circular dependency**: cmd/af imports config, config must receive build-time var from cmd/af. Severity: Low. Mitigation: Use a package-level setter function (same pattern as mcpstore). +- **agent-gen-all.sh forgetting export**: If AF_SOURCE_ROOT isn't exported, agent-gen falls back to go.mod check. Severity: Low. Mitigation: Self-hosted case (af source IS the project) works without the env var. + +## Constraints Identified +- `config.SetBuildSourceRoot()` must be called in cmd/af/main.go before rootCmd.Execute(). diff --git a/.designs/af-5836e28c/problem-summary.md b/.designs/af-5836e28c/problem-summary.md new file mode 100644 index 0000000..9354f73 --- /dev/null +++ b/.designs/af-5836e28c/problem-summary.md @@ -0,0 +1,27 @@ +# Problem Summary + +## Core Requirement +`af formula agent-gen` must place generated role templates (`.md.tmpl` files) in the agentfactory source tree's `internal/templates/roles/` directory rather than in the target project's factory root, so that running `quickstart.sh` is sufficient to build and use agents without manual file copying. + +## User Needs +- As a newcomer, I need `agent-gen` + `quickstart.sh` to produce a working setup without knowing the internal file layout +- As a developer, I need `agent-gen-all.sh` to regenerate all agents without manual `mv`/`cp` steps +- As a developer, I need the same `af formula agent-gen` command to work both when the af source tree IS the factory root and when it's a separate project + +## Constraints (HARD LIMITS - solutions MUST respect these) +- [ ] Templates must end up in `$AF_SRC/internal/templates/roles/` for `//go:embed` compilation +- [ ] Workspace dirs, CLAUDE.md, agents.json, and settings.json must stay in the factory root (target project) +- [ ] `quickstart.sh` must remain the single entry point for build + setup +- [ ] `agent-gen --delete` must still clean up both template files and workspace artifacts +- [ ] `agent-gen -o` (dry run) must continue working without needing source tree detection +- [ ] The solution must work when af source tree == factory root (self-hosted case) +- [ ] The solution must work when af source tree != factory root (external project case) +- [ ] Existing `AF_SOURCE_ROOT` env var pattern (used by mcpstore) should be reused, not a parallel mechanism +- [ ] `make build` must still be required after template changes (templates are compiled into binary) +- [ ] No breaking changes to `agent-gen` CLI interface + +## Scope: medium + +## Scope Calibration +- Dimensions to analyze: Data, API, Integration, Error Handling, Security, Testing +- Depth per dimension: standard diff --git a/.designs/af-5836e28c/scale.md b/.designs/af-5836e28c/scale.md new file mode 100644 index 0000000..5fe18e9 --- /dev/null +++ b/.designs/af-5836e28c/scale.md @@ -0,0 +1,40 @@ +# Scalability + +## Summary +Scale is not a primary concern for this problem. `agent-gen` is a developer tool run locally, not a hot path. The number of formulas is small (currently 6) and unlikely to exceed ~50. The primary scale consideration is whether the source root resolution adds meaningful latency (it doesn't — it's a single file read). + +## Constraint Check +- [x] Templates in AF_SRC: Scale doesn't affect where files go +- [x] Reuse AF_SOURCE_ROOT: Single env var lookup is O(1) +- [x] No CLI breaking changes: No performance-motivated interface changes + +## Options Explored + +### Option 1: Simple sequential resolution (recommended) +- **Description**: Check go.mod, then build-time var, then env var — sequentially, stopping at first hit. Total cost: one file read + two string checks. +- **Constraint Compliance**: All pass +- **Pros**: Simple; ~1ms total; no caching needed; no complexity +- **Cons**: Reads go.mod on every invocation (negligible) +- **Effort**: Low +- **Reversibility**: Easy + +### Option 2: Cache resolution result +- **Description**: Cache the resolved source root path in a temp file or env var for subsequent calls. +- **Constraint Compliance**: All pass but adds unnecessary complexity +- **Pros**: Saves one go.mod read on subsequent calls +- **Cons**: Cache invalidation complexity; stale cache if user moves directories; overkill for microsecond operation +- **Effort**: Medium +- **Reversibility**: Moderate +- REJECTED: Over-engineering — reading go.mod takes <1ms, caching adds complexity for no benefit + +## Recommendation +**Option 1**: Simple sequential resolution. No scale concerns at the volumes this tool operates at. + +## Dependencies Produced +- None — scale dimension doesn't add requirements. + +## Risks Identified +- None — this is a CLI dev tool, not a production service. + +## Constraints Identified +- None new. diff --git a/.designs/af-5836e28c/security.md b/.designs/af-5836e28c/security.md new file mode 100644 index 0000000..785c08c --- /dev/null +++ b/.designs/af-5836e28c/security.md @@ -0,0 +1,67 @@ +# Security + +## Summary +The primary security concern is path traversal: `AF_SOURCE_ROOT` is an environment variable that controls where `agent-gen` writes files. If set maliciously, it could write `.md.tmpl` files to arbitrary locations. However, the threat model is local developer tooling — the user controls their own environment. The secondary concern is ensuring the go.mod check isn't spoofable in a way that causes template files to be written to an attacker-controlled directory. + +## Constraint Check +- [x] Templates in AF_SRC: Must validate the path is actually an af source tree before writing +- [x] Self-hosted case: go.mod check must be robust +- [x] External project case: AF_SOURCE_ROOT value must be validated + +## Options Explored + +### Option 1: Validate source root before writing (recommended) +- **Description**: After resolving the source root (via go.mod, build-time var, or env var), validate that `$srcRoot/internal/templates/roles/` exists (or can be created) and that the directory looks like an af source tree. This prevents writing to arbitrary paths via a misconfigured AF_SOURCE_ROOT. +- **Constraint Compliance**: All pass +- **Pros**: Catches misconfigured AF_SOURCE_ROOT early with a clear error; prevents writing templates to random directories +- **Cons**: Slightly more validation code +- **Effort**: Low +- **Reversibility**: Easy + +### Option 2: No validation, trust the env var +- **Description**: Whatever AF_SOURCE_ROOT says, write there. +- **Constraint Compliance**: Technically passes all constraints +- **Pros**: Simpler code +- **Cons**: Misconfigured AF_SOURCE_ROOT writes templates to wrong location silently +- **Effort**: Low +- **Reversibility**: Easy +- REJECTED: Silent misplacement is the exact problem we're solving — we shouldn't introduce a new variant of it + +### Option 3: Require go.mod validation for all paths (including env var) +- **Description**: Even when AF_SOURCE_ROOT is set, verify it contains an agentfactory go.mod. +- **Constraint Compliance**: All pass +- **Pros**: Strongest validation; prevents ALL misconfiguration +- **Cons**: Adds one file read for env-var-sourced paths +- **Effort**: Low +- **Reversibility**: Easy + +## Recommendation +**Option 3**: Validate go.mod for all resolution paths. The cost is one file read. The benefit is catching misconfigured AF_SOURCE_ROOT before writing templates to the wrong place. This means `ResolveSourceRoot()` always validates the returned path. + +### Validation Logic +```go +func ResolveSourceRoot(factoryRoot string) (string, error) { + candidates := []struct{ path, source string }{ + {factoryRoot, "factory root"}, + {buildTimeSourceRoot, "build-time source root"}, + {os.Getenv("AF_SOURCE_ROOT"), "AF_SOURCE_ROOT env var"}, + } + for _, c := range candidates { + if c.path != "" && isAgentFactorySourceTree(c.path) { + return c.path, nil + } + } + return "", fmt.Errorf(/* list all checked paths */) +} +``` + +Every candidate is validated with `isAgentFactorySourceTree()` before being accepted. + +## Dependencies Produced +- Validation logic is part of `config.ResolveSourceRoot()` from the API dimension + +## Risks Identified +- **Legitimate non-standard source tree**: If someone forks agentfactory and changes the module path, go.mod validation will fail. Severity: Very Low. Mitigation: They can override with a renamed module that still contains "agentfactory" in the path, or the detection can be extended. + +## Constraints Identified +- AF_SOURCE_ROOT value must point to a directory with a valid agentfactory go.mod. diff --git a/.designs/af-5836e28c/ux.md b/.designs/af-5836e28c/ux.md new file mode 100644 index 0000000..df266ac --- /dev/null +++ b/.designs/af-5836e28c/ux.md @@ -0,0 +1,80 @@ +# User Experience + +## Summary +The UX improvement is the core motivation for this change. Currently users must manually copy generated templates from the target project to the af source tree — a step that's undiscoverable and error-prone. The desired UX: run `agent-gen` (or `agent-gen-all.sh`), then `quickstart.sh`, and everything works. Error messages when AF_SOURCE_ROOT is missing must clearly tell the user what to do. + +## Constraint Check +- [x] quickstart.sh single entry: This dimension ensures the end-to-end UX is seamless +- [x] No CLI breaking changes: Existing commands work; improvement is in output routing +- [x] Self-hosted case: No change in UX — templates already land in the right place +- [x] External project case: AF_SOURCE_ROOT eliminates manual copy steps + +## Options Explored + +### Option 1: Silent auto-routing with clear error on failure +- **Description**: When AF_SOURCE_ROOT is set (or factory root is the source tree), templates auto-route to the correct location silently. When it's not set and factory root isn't the source tree, emit a clear error explaining what to do. +- **Constraint Compliance**: All pass +- **Pros**: Zero friction for configured users; clear guidance for unconfigured users +- **Cons**: First-time users hit an error if AF_SOURCE_ROOT isn't set; but the error message tells them exactly what to do +- **Effort**: Low +- **Reversibility**: Easy + +### Option 2: Auto-routing with verbose output showing both paths +- **Description**: Same as Option 1 but print both template and workspace paths in the success output, so users can see where things went. +- **Constraint Compliance**: All pass +- **Pros**: Transparency — users understand the dual-path routing; aids debugging +- **Cons**: More output — could be noisy for batch operations +- **Effort**: Low +- **Reversibility**: Easy + +### Option 3: Interactive prompt when AF_SOURCE_ROOT is missing +- **Description**: When the source root can't be detected, prompt the user to enter it. +- **Constraint Compliance**: Violates autonomous operation — agents can't answer prompts +- **Pros**: User-friendly for interactive use +- **Cons**: Breaks in scripts, CI, and agent automation +- **Effort**: Medium +- **Reversibility**: Easy +- REJECTED: Violates non-interactive constraint (agents run autonomously, scripts run non-interactively) + +## Recommendation +**Option 2**: Auto-routing with verbose output. Users need to understand the dual-path model, and showing both paths makes it transparent. The output already prints each artifact's location; we extend this to differentiate "source tree" from "workspace." + +### Updated Output Example +``` +✓ Formula: design-v3 (workflow, 16 steps, 2 gates) +✓ Agent entry added to .agentfactory/agents.json (formula: design-v3) +✓ Role template written: /home/user/projects/agentfactory/internal/templates/roles/design-v3.md.tmpl +✓ Workspace created: /home/user/af/myproject/.agentfactory/agents/design-v3/ +✓ CLAUDE.md written (4.2 KB) +✓ .claude/settings.json written (autonomous) + +Agent "design-v3" is ready. Start with: af up design-v3 +Run 'make build' to compile the new template into the af binary. +``` + +Note: template path is now absolute and shows the source tree location, making it clear it's going to a different place than the workspace. + +### Error Message for Missing AF_SOURCE_ROOT +``` +Error: cannot determine af source tree for template placement + +The af source tree is needed to write role templates (internal/templates/roles/). +Checked: + 1. Factory root (/home/user/af/myproject/) — not the af source tree + 2. Build-time source root — not set + 3. AF_SOURCE_ROOT env var — not set + +Fix: Set AF_SOURCE_ROOT to the agentfactory source directory: + export AF_SOURCE_ROOT=/path/to/agentfactory +``` + +## Dependencies Produced +- Error message format feeds into Integration dimension (agent-gen-all.sh sets AF_SOURCE_ROOT) +- Success output format must be compatible with --delete output + +## Risks Identified +- **Absolute path in output**: Shows full paths which could be long. Severity: Low. Mitigation: Already the case for workspace paths. +- **User confusion about dual paths**: Users may not understand why template goes one place and workspace another. Severity: Medium. Mitigation: The output is self-documenting; add a one-liner to USING_AGENTFACTORY.md. + +## Constraints Identified +- None new. From ce41af0d57ab47e81d4671969389bdb480560fc4 Mon Sep 17 00:00:00 2001 From: agentfactory Date: Tue, 5 May 2026 03:06:58 +0000 Subject: [PATCH 2/3] design: synthesized design document for af-5836e28c Unified design for fixing agent-gen output location. Adds config.ResolveSourceRoot() to route templates to the af source tree while keeping workspace artifacts in the factory root. Reuses existing AF_SOURCE_ROOT pattern. Co-Authored-By: Claude Opus 4.6 --- .designs/af-5836e28c/conflicts.md | 29 +++++ .designs/af-5836e28c/constraint-audit.md | 46 ++++++++ .designs/af-5836e28c/dependencies.md | 55 +++++++++ .designs/af-5836e28c/design-doc.md | 142 +++++++++++++++++++++++ 4 files changed, 272 insertions(+) create mode 100644 .designs/af-5836e28c/conflicts.md create mode 100644 .designs/af-5836e28c/constraint-audit.md create mode 100644 .designs/af-5836e28c/dependencies.md create mode 100644 .designs/af-5836e28c/design-doc.md diff --git a/.designs/af-5836e28c/conflicts.md b/.designs/af-5836e28c/conflicts.md new file mode 100644 index 0000000..ddcdb32 --- /dev/null +++ b/.designs/af-5836e28c/conflicts.md @@ -0,0 +1,29 @@ +# Cross-Dimension Conflict Matrix + +| | API | Data | UX | Scale | Security | Integration | +|--------------|-----|------|----|-------|----------|-------------| +| **API** | - | ○ | ○ | ○ | ○ | ○ | +| **Data** | ○ | - | ○ | ○ | ○ | ○ | +| **UX** | ○ | ○ | - | ○ | ⚠ | ○ | +| **Scale** | ○ | ○ | ○ | - | ○ | ○ | +| **Security** | ○ | ○ | ⚠ | ○ | - | ○ | +| **Integration** | ○ | ○ | ○ | ○ | ○ | - | + +Legend: ○ No conflict, ⚠ Tension (trade-off needed), ✗ Direct conflict (resolution required) + +## Summary +This is a straightforward design with minimal cross-dimension conflicts. The only tension is between Security (validate all paths) and UX (keep errors minimal/friendly). No direct conflicts exist. + +## Tension: Security vs UX + +- **Nature**: Security recommends validating go.mod for ALL source root resolution paths (including AF_SOURCE_ROOT). UX recommends friendly error messages and minimal friction. If validation is too strict, users with legitimate but non-standard setups (e.g., forked repos with different module names) get blocked with no workaround. +- **Impact**: Strict validation could reject valid AF_SOURCE_ROOT values that point to a valid source tree but with a renamed go.mod module. +- **Resolution Options**: + 1. **Security wins**: Always require go.mod with "agentfactory" in module path. No exceptions. Users with renamed forks must change their module name or contribute upstream. + 2. **UX wins**: Accept any AF_SOURCE_ROOT without validation. Users are trusted to set it correctly. + 3. **Hybrid**: Validate go.mod when auto-detecting (factory root check), but trust explicit AF_SOURCE_ROOT if `internal/templates/roles/` directory exists there. The explicit env var implies user intent. +- **Chosen Resolution**: Option 1 (Security wins) because: + - The forked-repo scenario is extremely rare for this tool + - Writing templates to the wrong directory is the exact problem we're solving + - The validation cost is one file read + - If someone really needs to override, they can add "agentfactory" to their fork's module path diff --git a/.designs/af-5836e28c/constraint-audit.md b/.designs/af-5836e28c/constraint-audit.md new file mode 100644 index 0000000..9871e5f --- /dev/null +++ b/.designs/af-5836e28c/constraint-audit.md @@ -0,0 +1,46 @@ +# Pre-Synthesis Constraint Audit + +## Constraints Legend +- C1: Templates in AF_SRC internal/templates/roles/ +- C2: Workspace artifacts stay in factory root +- C3: quickstart.sh remains single entry point +- C4: --delete cleans both sides +- C5: -o works without source tree detection +- C6: Self-hosted case (af source == factory root) +- C7: External project case (af source != factory root) +- C8: Reuse AF_SOURCE_ROOT pattern +- C9: make build still required +- C10: No CLI breaking changes + +## Audit Matrix + +| Dimension | Recommendation | C1 | C2 | C3 | C4 | C5 | C6 | C7 | C8 | C9 | C10 | Status | +|-----------|---------------|----|----|----|----|----|----|----|----|----|----|--------| +| API | Shared config.ResolveSourceRoot() | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | PASS | +| Data | go.mod module path check | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | PASS | +| UX | Auto-routing with verbose output | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | PASS | +| Scale | Simple sequential resolution | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | PASS | +| Security | Validate go.mod for all resolution paths | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | PASS | +| Integration | Export AF_SOURCE_ROOT in agent-gen-all.sh | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | PASS | + +## Detailed Verification + +### C5 Check (most subtle): -o flag without source tree detection +The -o (dry run) flag path in formula.go returns after rendering content to stdout (line 130-133). The source root resolution happens at line ~200, which is AFTER the -o return point. Wait — looking more carefully: + +The current code calls `ResolveSourceRoot()` BEFORE the -o check would be useful. The proposed change adds the source root resolution after finding factory root (line 109) but BEFORE the -o check (line 130). + +**Issue detected**: If we add `srcRoot, err := config.ResolveSourceRoot(root)` before the -o check, then `-o` will fail when AF_SOURCE_ROOT isn't set and factory root isn't the source tree. + +**Fix**: Move source root resolution to AFTER the -o check, right before the template write (line 199). The -o flag only needs the rendered content (which uses factory root for workspace path rendering), not the source root for template placement. + +**Revised plan**: In formula.go: +- Lines 108-115: Find factory root, compute wsDir (unchanged) +- Lines 118-132: Render template, -o flag returns (unchanged) +- NEW: After -o check, resolve source root for template placement +- Line 200: Use srcRoot for template directory + +This preserves C5. + +## Audit Result +All dimensions PASS after the C5 fix for -o flag ordering. No constraint violations found. diff --git a/.designs/af-5836e28c/dependencies.md b/.designs/af-5836e28c/dependencies.md new file mode 100644 index 0000000..78999d8 --- /dev/null +++ b/.designs/af-5836e28c/dependencies.md @@ -0,0 +1,55 @@ +# Dependency Graph + +## Component Dependencies + +``` +config.isAgentFactorySourceTree() [Data dimension] + │ + ▼ +config.ResolveSourceRoot() [API dimension] + │ │ + ▼ ▼ +cmd/af/main.go formula.go: runFormulaAgentGen() +(SetBuildSourceRoot) (template routing) + │ + ▼ + formula.go: runFormulaAgentGenDelete() + (template cleanup) + │ + ▼ + agent-gen-all.sh + (exports AF_SOURCE_ROOT) +``` + +## File Dependencies + +| Component | File | Depends On | Provides To | +|-----------|------|------------|-------------| +| isAgentFactorySourceTree() | internal/config/source_root.go | os, strings, filepath (stdlib) | ResolveSourceRoot() | +| ResolveSourceRoot() | internal/config/source_root.go | isAgentFactorySourceTree(), buildSourceRoot var | formula.go (both create and delete paths) | +| SetBuildSourceRoot() | internal/config/source_root.go | (none) | cmd/af/main.go calls it | +| runFormulaAgentGen() | internal/cmd/formula.go | config.ResolveSourceRoot() | Users, agent-gen-all.sh | +| runFormulaAgentGenDelete() | internal/cmd/formula.go | config.ResolveSourceRoot() | Users, agent-gen-all.sh | +| agent-gen-all.sh | agent-gen-all.sh | AF_SOURCE_ROOT env var | Users, quickstart.sh | +| quickstart.sh | quickstart.sh | (already sets AF_SOURCE_ROOT) | Users | + +## Circular Dependency Check +No circular dependencies. The dependency graph is a DAG: +1. `isAgentFactorySourceTree()` depends only on stdlib +2. `ResolveSourceRoot()` depends on #1 and a package-level var +3. `formula.go` depends on #2 +4. `agent-gen-all.sh` depends on `af` binary (which includes #3) +5. `quickstart.sh` depends on `agent-gen-all.sh` indirectly + +## Critical Path (Implementation Order) + +1. **config/source_root.go**: Create `isAgentFactorySourceTree()` and `ResolveSourceRoot()` with tests +2. **cmd/af/main.go**: Add `config.SetBuildSourceRoot(sourceRoot)` call +3. **internal/cmd/formula.go**: Update `runFormulaAgentGen()` and `runFormulaAgentGenDelete()` to use `config.ResolveSourceRoot()` +4. **agent-gen-all.sh**: Add `export AF_SOURCE_ROOT="$AF_SRC"` line +5. **Tests**: Unit tests for source_root.go, update existing formula_test.go tests + +Steps 1-2 must happen before 3. Step 4 is independent. Step 5 can happen after 1. + +## Minimum Viable Implementation +Steps 1-3 alone fix the core problem for the self-hosted case (af source IS factory root). Step 4 is needed for the external project case. Step 5 validates correctness. diff --git a/.designs/af-5836e28c/design-doc.md b/.designs/af-5836e28c/design-doc.md new file mode 100644 index 0000000..61e1efb --- /dev/null +++ b/.designs/af-5836e28c/design-doc.md @@ -0,0 +1,142 @@ +# Design: Agent-Gen Output Location Fix + +## Executive Summary + +`af formula agent-gen` currently writes all artifacts — including Go role templates (`.md.tmpl`) — relative to the factory root. When the factory root is a target project separate from the agentfactory source tree, the templates land in the wrong place. They need to be in the af source tree's `internal/templates/roles/` directory for Go's `//go:embed` to compile them into the binary. + +This design introduces a shared `config.ResolveSourceRoot()` function that resolves the agentfactory source tree location using a three-tier strategy: (1) check if the factory root itself is the source tree via go.mod, (2) check a build-time embedded path, (3) check the `AF_SOURCE_ROOT` environment variable. This reuses the existing `AF_SOURCE_ROOT` pattern already used by the MCP issue-store server. The `agent-gen` command routes template writes to the resolved source root while keeping all other artifacts (workspace, CLAUDE.md, agents.json) in the factory root. + +The change is minimal: one new file in `internal/config/`, small modifications to `formula.go` and `cmd/af/main.go`, and one line added to `agent-gen-all.sh`. No CLI interface changes. No breaking changes. + +## Constraints Respected + +All proposals in this design respect the following constraints: +- Templates end up in `$AF_SRC/internal/templates/roles/` for `//go:embed` +- Workspace artifacts stay in factory root +- `quickstart.sh` remains the single entry point (already sets `AF_SOURCE_ROOT`) +- `--delete` cleans up templates from source root and workspace from factory root +- `-o` (dry run) works without source tree detection (source root resolution happens after -o early return) +- Self-hosted case works (go.mod check detects factory root as source tree) +- External project case works (`AF_SOURCE_ROOT` env var provides the path) +- Reuses existing `AF_SOURCE_ROOT` pattern from mcpstore +- `make build` still required after template changes +- No CLI breaking changes + +## Problem Statement + +When `agent-gen` runs from a target project (e.g., `~/af/myproject/`), it writes `.md.tmpl` files to `~/af/myproject/internal/templates/roles/` instead of the agentfactory source tree (e.g., `~/projects/agentfactory/internal/templates/roles/`). Users must manually copy these files before running `make build`. The goal: eliminate this manual step so that `quickstart.sh` alone is sufficient. + +## Proposed Design + +### Overview + +Add a source root resolution layer that separates "where templates go" (af source tree) from "where workspace artifacts go" (factory root). The resolution reuses the existing `AF_SOURCE_ROOT` pattern and adds go.mod auto-detection for the self-hosted case. + +### Key Components + +1. **`config.ResolveSourceRoot()`** — Resolves the af source tree path +2. **`config.isAgentFactorySourceTree()`** — Validates a directory is the af source tree via go.mod +3. **`formula.go` changes** — Routes template writes to source root, keeps workspace writes in factory root +4. **`agent-gen-all.sh` change** — Exports `AF_SOURCE_ROOT=$AF_SRC` + +### Component Dependency Graph + +``` +isAgentFactorySourceTree() ──► ResolveSourceRoot() ──► formula.go (create/delete) + ▲ + SetBuildSourceRoot() ◄── cmd/af/main.go +``` + +### Interface + +No CLI changes. The existing interface `af formula agent-gen ` continues to work. The only new environmental input is the existing `AF_SOURCE_ROOT` env var, which `agent-gen-all.sh` and `quickstart.sh` set automatically. + +### Data Model + +No new storage. The go.mod file (already present in the source tree) serves as the detection signal. The check reads the `module` line and verifies it contains "agentfactory". + +## Cross-Dimension Trade-offs + +| Conflict | Resolution | Rationale | +|----------|------------|-----------| +| Security (validate all paths) vs UX (minimal friction) | Security wins: validate go.mod for all paths including AF_SOURCE_ROOT | Writing templates to wrong dir is the exact problem we're solving; validation cost is one file read | + +## Trade-offs and Decisions + +### Decisions Made + +| Decision | Options Considered | Chosen | Rationale | Reversibility | +|----------|-------------------|--------|-----------|---------------| +| Source root resolution mechanism | Shared function, --source-root flag, binary location walk-up | Shared config.ResolveSourceRoot() | Reuses AF_SOURCE_ROOT pattern; no CLI changes; handles both self-hosted and external cases | Easy | +| Source tree detection | go.mod module check, directory existence check, marker file | go.mod module path check | Precise, uses existing data, no maintenance burden | Easy | +| go.mod validation scope | Validate only auto-detected, validate all, no validation | Validate all paths | Prevents misconfigured AF_SOURCE_ROOT; cost is negligible | Easy | +| -o flag interaction | Resolve source root before or after -o | After -o early return | -o doesn't write templates; requiring source root would break dry-run usage | Easy | + +### Open Questions +None — all design decisions are resolved. + +## Risk Registry + +| Risk | Severity | Likelihood | Mitigation | Owner | +|------|----------|------------|------------|-------| +| Build-time var threading from main.go to config | Low | Low | Package-level setter (same pattern as mcpstore) | Implementer | +| go.mod module path rename | Very Low | Very Low | Detection string "agentfactory" is broad enough | Implementer | +| AF_SOURCE_ROOT not set for external projects | Medium | Medium | Clear error message with fix instructions | Implementer | +| Self-hosted case where factory root == source root | Low | Low | go.mod auto-detection handles this transparently | Implementer | + +## Implementation Plan + +### Phase 1: Source Root Resolution (Effort: Small) + +**Deliverables:** +1. New file `internal/config/source_root.go` with `ResolveSourceRoot()`, `isAgentFactorySourceTree()`, and `SetBuildSourceRoot()` +2. New file `internal/config/source_root_test.go` with unit tests +3. One-line addition to `cmd/af/main.go`: `config.SetBuildSourceRoot(sourceRoot)` + +**Acceptance Criteria:** +- [ ] `ResolveSourceRoot()` returns factory root when it contains agentfactory go.mod +- [ ] `ResolveSourceRoot()` returns AF_SOURCE_ROOT when factory root is not source tree +- [ ] `ResolveSourceRoot()` returns error when neither is available +- [ ] All three resolution paths are validated with go.mod check + +**Dependencies:** None — this is the foundation. + +### Phase 2: Formula.go Integration (Effort: Small) + +**Deliverables:** +1. Update `runFormulaAgentGen()` to use `config.ResolveSourceRoot()` for template path +2. Update `runFormulaAgentGenDelete()` to use `config.ResolveSourceRoot()` for template cleanup +3. Update output messages to show full path when source root differs from factory root +4. Update existing formula tests + +**Acceptance Criteria:** +- [ ] Templates written to source root's `internal/templates/roles/` +- [ ] Workspace artifacts written to factory root's `.agentfactory/agents/` +- [ ] Source root resolution happens AFTER -o early return +- [ ] --delete removes template from source root +- [ ] Output messages clearly show where template was written + +**Dependencies:** Phase 1 complete. + +### Phase 3: Script Updates (Effort: Trivial) + +**Deliverables:** +1. Add `export AF_SOURCE_ROOT="$AF_SRC"` to `agent-gen-all.sh` +2. Verify `quickstart.sh` already exports `AF_SOURCE_ROOT` (it does, line 419) + +**Acceptance Criteria:** +- [ ] `agent-gen-all.sh` sets AF_SOURCE_ROOT before calling agent-gen +- [ ] `quickstart.sh` continues to work (no changes needed) + +**Dependencies:** Phase 2 complete (for end-to-end testing). + +## Appendix: Dimension Analyses +- [API Design](api.md) +- [Data Model](data.md) +- [User Experience](ux.md) +- [Scalability](scale.md) +- [Security](security.md) +- [Integration](integration.md) +- [Conflict Matrix](conflicts.md) +- [Constraint Audit](constraint-audit.md) +- [Dependencies](dependencies.md) From 0b0772141edd2ad46ee088a4fa5f3ae10c5473c4 Mon Sep 17 00:00:00 2001 From: agentfactory Date: Tue, 5 May 2026 03:26:08 +0000 Subject: [PATCH 3/3] fix: route agent-gen templates to af source tree Add config.ResolveSourceRoot() that determines the agentfactory source tree via go.mod detection, build-time path, or AF_SOURCE_ROOT env var. Update formula.go to write .md.tmpl files to the resolved source root while keeping workspace artifacts in the factory root. Update agent-gen-all.sh to export AF_SOURCE_ROOT. Closes #4 Co-Authored-By: Claude Opus 4.6 --- agent-gen-all.sh | 2 + cmd/af/main.go | 3 + internal/cmd/formula.go | 22 +++- internal/cmd/formula_test.go | 6 + internal/config/source_root.go | 69 ++++++++++++ internal/config/source_root_test.go | 167 ++++++++++++++++++++++++++++ 6 files changed, 266 insertions(+), 3 deletions(-) create mode 100644 internal/config/source_root.go create mode 100644 internal/config/source_root_test.go diff --git a/agent-gen-all.sh b/agent-gen-all.sh index 1764820..6644751 100755 --- a/agent-gen-all.sh +++ b/agent-gen-all.sh @@ -46,6 +46,8 @@ if [ ! -f "$AF_SRC/go.mod" ] || ! grep -q agentfactory "$AF_SRC/go.mod" 2>/dev/n exit 1 fi +export AF_SOURCE_ROOT="$AF_SRC" + PROJECT="$(pwd)" echo "project: $PROJECT" echo "af source: $AF_SRC" diff --git a/cmd/af/main.go b/cmd/af/main.go index dbf9f2f..9166e78 100644 --- a/cmd/af/main.go +++ b/cmd/af/main.go @@ -4,6 +4,7 @@ import ( "os" "github.com/stempeck/agentfactory/internal/cmd" + "github.com/stempeck/agentfactory/internal/config" "github.com/stempeck/agentfactory/internal/issuestore/mcpstore" ) @@ -12,5 +13,7 @@ var sourceRoot string func main() { mcpstore.SetSourceRoot(sourceRoot) mcpstore.SetEnvSourceRoot(os.Getenv("AF_SOURCE_ROOT")) + config.SetBuildSourceRoot(sourceRoot) + config.SetEnvSourceRoot(os.Getenv("AF_SOURCE_ROOT")) os.Exit(cmd.Execute()) } diff --git a/internal/cmd/formula.go b/internal/cmd/formula.go index 8bed08a..4a7c0f7 100644 --- a/internal/cmd/formula.go +++ b/internal/cmd/formula.go @@ -196,8 +196,14 @@ func runFormulaAgentGen(cmd *cobra.Command, args []string) error { fmt.Fprintf(cmd.ErrOrStderr(), "✓ Agent entry added to .agentfactory/agents.json (formula: %s)\n", f.Name) } + // Resolve source root for template output + srcRoot, err := config.ResolveSourceRoot(root) + if err != nil { + return fmt.Errorf("resolving source root: %w", err) + } + // Write role template to source tree - tmplDir := filepath.Join(root, "internal", "templates", "roles") + tmplDir := filepath.Join(srcRoot, "internal", "templates", "roles") if err := os.MkdirAll(tmplDir, 0755); err != nil { return fmt.Errorf("creating template directory: %w", err) } @@ -205,7 +211,11 @@ func runFormulaAgentGen(cmd *cobra.Command, args []string) error { if err := os.WriteFile(tmplPath, []byte(tmplContent), 0644); err != nil { return fmt.Errorf("writing role template: %w", err) } - fmt.Fprintf(cmd.ErrOrStderr(), "✓ Role template written: internal/templates/roles/%s.md.tmpl\n", agentName) + if srcRoot != root { + fmt.Fprintf(cmd.ErrOrStderr(), "✓ Role template written: %s\n", tmplPath) + } else { + fmt.Fprintf(cmd.ErrOrStderr(), "✓ Role template written: internal/templates/roles/%s.md.tmpl\n", agentName) + } // Create workspace directory wsDirCreated := false @@ -303,8 +313,14 @@ func runFormulaAgentGenDelete(cmd *cobra.Command, agentName string) error { return err } + // Resolve source root for template location + srcRoot, err := config.ResolveSourceRoot(root) + if err != nil { + return fmt.Errorf("resolving source root: %w", err) + } + // Paths - tmplPath := filepath.Join(root, "internal", "templates", "roles", agentName+".md.tmpl") + tmplPath := filepath.Join(srcRoot, "internal", "templates", "roles", agentName+".md.tmpl") wsDir := config.AgentDir(root, agentName) // Check workspace for uncommitted changes (warn only) diff --git a/internal/cmd/formula_test.go b/internal/cmd/formula_test.go index 0dea2de..6c876b5 100644 --- a/internal/cmd/formula_test.go +++ b/internal/cmd/formula_test.go @@ -414,6 +414,11 @@ func setupFormulaFactory(t *testing.T) string { t.Fatal(err) } + // Create go.mod so ResolveSourceRoot identifies this as the source tree + if err := os.WriteFile(filepath.Join(dir, "go.mod"), []byte("module github.com/stempeck/agentfactory\n\ngo 1.24\n"), 0644); err != nil { + t.Fatal(err) + } + // Create template directory for template file writes tmplDir := filepath.Join(dir, "internal", "templates", "roles") if err := os.MkdirAll(tmplDir, 0755); err != nil { @@ -979,6 +984,7 @@ func TestProvisioningPipeline_DescriptionFirstSentence(t *testing.T) { os.MkdirAll(filepath.Join(configDir, "agents"), 0755) os.MkdirAll(filepath.Join(dir, "internal", "templates", "roles"), 0755) + os.WriteFile(filepath.Join(dir, "go.mod"), []byte("module github.com/stempeck/agentfactory\n\ngo 1.24\n"), 0644) os.WriteFile(filepath.Join(configDir, "factory.json"), []byte(`{"type":"factory","version":1,"name":"agentfactory"}`), 0644) os.WriteFile(filepath.Join(configDir, "agents.json"), []byte(`{"agents":{"manager":{"type":"interactive","description":"Interactive agent"}}}`), 0644) diff --git a/internal/config/source_root.go b/internal/config/source_root.go new file mode 100644 index 0000000..05706bc --- /dev/null +++ b/internal/config/source_root.go @@ -0,0 +1,69 @@ +package config + +import ( + "bufio" + "fmt" + "os" + "path/filepath" + "strings" +) + +var buildSourceRoot string +var envSourceRoot string + +// SetBuildSourceRoot stores the build-time embedded source root path. +func SetBuildSourceRoot(root string) { + buildSourceRoot = root +} + +// SetEnvSourceRoot stores the AF_SOURCE_ROOT value read at the CLI boundary. +func SetEnvSourceRoot(root string) { + envSourceRoot = root +} + +// ResolveSourceRoot determines the agentfactory source tree location using a +// three-tier strategy: +// 1. Check if factoryRoot itself is the source tree (self-hosted case) +// 2. Check the build-time embedded path +// 3. Check the AF_SOURCE_ROOT environment variable +// +// All paths are validated via go.mod module line check. +func ResolveSourceRoot(factoryRoot string) (string, error) { + if isAgentFactorySourceTree(factoryRoot) { + return factoryRoot, nil + } + + if buildSourceRoot != "" { + if isAgentFactorySourceTree(buildSourceRoot) { + return buildSourceRoot, nil + } + return "", fmt.Errorf("build-time source root %q is not an agentfactory source tree (go.mod check failed)", buildSourceRoot) + } + + if envSourceRoot != "" { + if isAgentFactorySourceTree(envSourceRoot) { + return envSourceRoot, nil + } + return "", fmt.Errorf("AF_SOURCE_ROOT=%q is not an agentfactory source tree (go.mod check failed)", envSourceRoot) + } + + return "", fmt.Errorf("cannot resolve agentfactory source root: factory root %q is not the source tree, no build-time root set, and AF_SOURCE_ROOT is not set.\nFix: set AF_SOURCE_ROOT to the agentfactory source checkout", factoryRoot) +} + +// isAgentFactorySourceTree checks if dir contains a go.mod with the agentfactory module path. +func isAgentFactorySourceTree(dir string) bool { + f, err := os.Open(filepath.Join(dir, "go.mod")) + if err != nil { + return false + } + defer f.Close() + + scanner := bufio.NewScanner(f) + for scanner.Scan() { + line := scanner.Text() + if strings.HasPrefix(line, "module ") { + return strings.Contains(line, "agentfactory") + } + } + return false +} diff --git a/internal/config/source_root_test.go b/internal/config/source_root_test.go new file mode 100644 index 0000000..b7753e1 --- /dev/null +++ b/internal/config/source_root_test.go @@ -0,0 +1,167 @@ +package config + +import ( + "os" + "path/filepath" + "testing" +) + +func setupSourceTree(t *testing.T) string { + t.Helper() + dir := t.TempDir() + realDir, _ := filepath.EvalSymlinks(dir) + if err := os.WriteFile(filepath.Join(realDir, "go.mod"), []byte("module github.com/stempeck/agentfactory\n\ngo 1.24\n"), 0644); err != nil { + t.Fatalf("write go.mod: %v", err) + } + return realDir +} + +func TestResolveSourceRoot_FactoryRootIsSourceTree(t *testing.T) { + root := setupSourceTree(t) + + SetBuildSourceRoot("") + SetEnvSourceRoot("") + + got, err := ResolveSourceRoot(root) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != root { + t.Errorf("got %q, want %q", got, root) + } +} + +func TestResolveSourceRoot_BuildTimeSourceRoot(t *testing.T) { + factoryRoot := t.TempDir() + sourceRoot := setupSourceTree(t) + + SetBuildSourceRoot(sourceRoot) + t.Setenv("AF_SOURCE_ROOT", "") + + got, err := ResolveSourceRoot(factoryRoot) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != sourceRoot { + t.Errorf("got %q, want %q", got, sourceRoot) + } +} + +func TestResolveSourceRoot_EnvVar(t *testing.T) { + factoryRoot := t.TempDir() + sourceRoot := setupSourceTree(t) + + SetBuildSourceRoot("") + SetEnvSourceRoot(sourceRoot) + + got, err := ResolveSourceRoot(factoryRoot) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != sourceRoot { + t.Errorf("got %q, want %q", got, sourceRoot) + } +} + +func TestResolveSourceRoot_ErrorWhenNoneAvailable(t *testing.T) { + factoryRoot := t.TempDir() + + SetBuildSourceRoot("") + SetEnvSourceRoot("") + + _, err := ResolveSourceRoot(factoryRoot) + if err == nil { + t.Fatal("expected error, got nil") + } +} + +func TestResolveSourceRoot_ValidatesEnvVar(t *testing.T) { + factoryRoot := t.TempDir() + badDir := t.TempDir() + // badDir has no go.mod + + SetBuildSourceRoot("") + SetEnvSourceRoot(badDir) + + _, err := ResolveSourceRoot(factoryRoot) + if err == nil { + t.Fatal("expected error for invalid AF_SOURCE_ROOT, got nil") + } +} + +func TestResolveSourceRoot_ValidatesBuildTime(t *testing.T) { + factoryRoot := t.TempDir() + badDir := t.TempDir() + // badDir has no go.mod + + SetBuildSourceRoot(badDir) + t.Setenv("AF_SOURCE_ROOT", "") + + _, err := ResolveSourceRoot(factoryRoot) + if err == nil { + t.Fatal("expected error for invalid build-time source root, got nil") + } +} + +func TestResolveSourceRoot_PriorityOrder(t *testing.T) { + factoryRoot := setupSourceTree(t) + buildRoot := setupSourceTree(t) + envRoot := setupSourceTree(t) + + SetBuildSourceRoot(buildRoot) + SetEnvSourceRoot(envRoot) + + // Factory root (self-hosted) wins over build-time and env + got, err := ResolveSourceRoot(factoryRoot) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != factoryRoot { + t.Errorf("got %q, want %q (factory root should win)", got, factoryRoot) + } +} + +func TestResolveSourceRoot_BuildTimeOverEnv(t *testing.T) { + factoryRoot := t.TempDir() + buildRoot := setupSourceTree(t) + envRoot := setupSourceTree(t) + + SetBuildSourceRoot(buildRoot) + SetEnvSourceRoot(envRoot) + + // Build-time wins over env when factory root is not source tree + got, err := ResolveSourceRoot(factoryRoot) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != buildRoot { + t.Errorf("got %q, want %q (build-time should win over env)", got, buildRoot) + } +} + +func TestIsAgentFactorySourceTree(t *testing.T) { + tests := []struct { + name string + content string + want bool + }{ + {"valid", "module github.com/stempeck/agentfactory\n\ngo 1.24\n", true}, + {"no module line", "go 1.24\n", false}, + {"wrong module", "module github.com/other/project\n\ngo 1.24\n", false}, + {"empty", "", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + if tt.content != "" { + if err := os.WriteFile(filepath.Join(dir, "go.mod"), []byte(tt.content), 0644); err != nil { + t.Fatalf("write: %v", err) + } + } + if got := isAgentFactorySourceTree(dir); got != tt.want { + t.Errorf("isAgentFactorySourceTree() = %v, want %v", got, tt.want) + } + }) + } +}