design: New dispatch workflow default to be included with agentfactory (#73)

[stempeck]

Issue

Design for #73 — New dispatch workflow default to be included with agentfactory
#73

A freshly bootstrapped factory cannot drive autonomous work from GitHub labels:
af install --init writes an empty dispatch.json (repos:[], mappings:[],
no workflows — internal/cmd/install.go:145), so a new user must hand-edit the
file before label-triggered dispatch works. The issue asks for a useful baked-in
default, populated with the real owner/repo discovered at install time.

This is a design + cross-review PR (no production code yet). It produces the
validated design and a 3-phase implementation plan; implementation is a follow-up.

Root cause

• runInstallInit ships an empty dispatch.json and performs no repo-name
  discovery, even though quickstart has already cd'd into the repo (the fact is
  discarded). — install.go:145, verified.
• The naive fix (ship the proposed default as-is) is necessary but not
  sufficient: the default's four mappings reference specialist agents
  (rapid-soldesign-plan, rapid-implement, ultra-review, rapid-increment)
  that a fresh agents.json does not contain (install ships only
  manager+supervisor), and ValidateDispatchConfig hard-fails the entire
  dispatch cycle on the first unknown mapped agent (internal/config/dispatch.go,
  invoked unconditionally at internal/cmd/dispatch.go:146). So the default alone
  would break dispatch on every fresh factory.

Proposed solution

A single systemic change (valid-by-construction), not just a literal default:

• K1 DefaultDispatchConfigJSON(repo) — single-source builder beside
  DefaultFactoryConfigJSON() (config.go:111), emitting the 4 label→agent
  mappings + feature-workflow + trigger_label:"agentic".
• K2 Non-interactive repo discovery (gh repo view --json nameWithOwner, with
  git remote get-url origin fallback) feeding repos; warn-don't-abort.
• K3 Strict owner/name validator at the write boundary (guards flag/escape
  injection from a crafted remote).
• K5 DefaultAgentsConfigJSON() — seeds the 4 specialists into the default
  agents.json within runInstallInit itself (the role templates are already
  embedded), making the default valid on every init path, including the bare
  af install --init "hard way" — not just quickstart.
• K9 Hoist dispatcher auto-start out of the blanket-only gate so the
  documented af up manager (positional) also starts the polling loop (idempotent).
• K6/K7/K8 Defense-in-depth: dispatch-loop skip-and-warn tolerance (K6) with
  mandatory observability (K8), plus a golden cross-file drift test (K7).

Architecture-elevation verdict: Frame correct (the dispatch fields must exist;
deleting dispatch.json only relocates them) with one offered lift adopted —
repo self-derivation (K2).

Cross-review (round 1)

Analyst (rootcause-all) reviewed the design grounded in its root-cause analysis
(9 concerns; 8 validated). It converged with the design on every primary point
and raised, the designer (design-v7) incorporated, all findings:

• C1 (CRITICAL): "valid-by-construction" held only on the quickstart path; bare
  af install --init left the populated default referencing unregistered agents.
  → Fixed by K5 seeding specialists in runInstallInit itself.
• C2 (CRITICAL): the documented af up manager (positional) does not
  auto-start the dispatcher (gated to blanket af up). → Fixed by K9 hoisting
  the auto-start.
• H1 (HIGH): commit to the minimal provisioning mechanism (drop heavyweight
  agent-gen-all.sh). → K5 now commits to direct agents.json seeding.
• H2 (HIGH): make observability K8 mandatory wherever the K6 tolerance is
  enabled (else a "running-but-dispatching-nothing" loop hides the failure).
• L1–L4 (LOW): agreements/nits; no change.

Both CRITICAL items were genuine entry-point/sequencing gaps; both are small,
localized, and strengthen the valid-by-construction thesis rather than redirect
it. With C1+C2 incorporated, the design satisfies AC-2 and AC-3 on every documented
setup path. (design-doc.md grew 319 → 365 lines.)

Artifacts

• .designs/73/design-doc.md — full design (dimension analyses, AC traceability
  6/6, elevation verdict, risk registry, 3-phase implementation plan)
• .designs/73/cross-review/analyst-review-design.md — analyst round-1 review
• .designs/73/cross-review/designer-update-log.md — incorporation log
• .designs/73/problem-summary-73.md, .designs/73/design-refinement-progress.md

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

---

agentfactory seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[stempeck]

design-plan-impl: implementation plan outline added

Outline Summary

• Outline: .designs/73/implementation-plan/implementation_plan_outline.md (now also carries an appended # Peer Review section)
• Phases (3 total):
  • 1 — Single-source default builder + repo discovery + agents seed (K1–K5 + fix existing test) — recommended skill: *implement (e.g. rapid-implement)
  • 2 — Dispatcher auto-start + graceful degradation + observability (K9 K6 K8) — *implement
  • 3 — Drift/golden test gate + docs (K7 + docs) — *implement
• Dependency chain: Phase 1 → Phase 2 → Phase 3. P1 is foundational; P1 and P2 share no compile-level dependency (sequenced only so P2's acceptance can rely on P1's valid default); P3 must run last (pins P1's emitted default, exercises P2's behaviors). Intra-P1 topo: K3 → K1 → K2 → K4 → K5.
• Deployment coverage: bare af install --init, container/quickstart, documented af up manager, fresh-install dispatch visibility, and the CI validity gate are all covered — no gaps. web/ module, MaxWorktrees, MCP issue-store are correctly out of scope. AC-5 (PR-push) is a formula-layer property, intentionally not a phase.

Independent Peer Review (appended to the outline)

• Verdict: PARTIALLY VALIDATED, High confidence. Plan is architecturally sound and systemic; all code anchors independently re-verified. The outline correctly corrects the design-doc's install.go:176 mis-citation (:176 is mcpstore.New; the only dispatch literal is :145).
• 3 actionable defects to address before phase extraction:
  • E-1 (must): Phase-1 AC Respect existing Anthropic env vars in quickstart.sh #6 is a false-green — internal/cmd/install_integration_test.go is //go:build integration, so go test ./internal/cmd/ -run 'TestInstall' (no tag) skips the updated assertions while other TestInstall* make it print ok. Route through -tags=integration / make test-integration; the break does not redden make test.
  • E-2 (must): drift-model path is wrong — formula_drift_test.go lives in internal/cmd/, not internal/config/.
  • G-1 (should): K8's config_state cannot distinguish discovery_failed from empty_by_design from disk (both are repos:[]); needs an install-time breadcrumb or a 3-state taxonomy.

Next Step

Review the outline (with the appended peer review). After approval, extract phases one at a time via:

af sling --agent design-phase-impl '<this PR URL>'

[stempeck]

rapid-implement: implementation landed on this PR's branch

Resolves the implementation for #73 via the rapid-implement process (classify → test-first → implement → verify → blind review). The code continues this PR's branch (af/rapid-soldesign-plan-7605ca, commit b2b33056) — no separate PR was opened.

Implemented .designs/73/design-doc.md components K1–K9:

• K1 config.DefaultDispatchConfigJSON(repo) — single-source dispatch.json default (4 label→agent mappings + feature-workflow, trigger_label:agentic); empty-repo degrades to the loadable empty shape; notify_on_complete omitted (Gap-7).
• K2/K3 non-interactive repo discovery (gh repo view → git remote origin fallback) via the runGitDetect seam, validated by a strict owner/name allowlist (flag-injection / shell-meta guard) before use.
• K4 wired runInstallInit to the builders + escape-safe banner; write-if-absent preserved.
• K5 config.DefaultAgentsConfigJSON() seeds the 4 specialists into the default agents.json (valid-by-construction on every init path — cross-review C1). Each carries a description (validator requires it) + formula.
• K6 dispatch loop skip-and-warns on unprovisioned mapping agents (loop-scoped); af config dispatch set stays strict.
• K8 config_state surfaced at af dispatch status --json + af up pre-flight (mandatory with K6).
• K9 dispatcher auto-start hoisted out of the blanket-only gate so positional af up manager also starts it (idempotent — cross-review C2).
• K7 golden + cross-file tests; docs note the two-label requirement + net-new scope.

Gates: test-first failing test proven (GATE 2) → target tests pass (GATE 3) → full suite green across root + web modules (GATE 4, 0 FAIL) → blind review 9/10 (GATE 5).

Deviations (each serves the design's goal): added ,omitempty to NotifyOnComplete so the default truly omits it; gave seeded specialists a description (the agent validator requires non-empty descriptions — the design's literal snippet would have produced an invalid agents.json).

[stempeck]

Code review

Found 4 issues. 3 are posted as inline comments on the diff; 1 is below (its lines are outside the diff hunks):

1. Stale af up help text and comment after the K9 hoist. This PR moves startDispatch/launchWatchdog outside the if blanket { } gate, so start_dispatch: true now fires on the positional path too (af up manager auto-starts the dispatcher). But the upCmd.Long help text still promises the old behavior — "Positional 'af up ' ignores startup.json and starts exactly those agents." — and the inline comment that documents the blanket-only invariant was not updated either. Stale-by-consequence of this diff.

agentfactory/internal/cmd/up.go

Lines 39 to 41 in b2b3305

	A start set larger than max_worktrees is warned about (not aborted) before launch.
	Positional 'af up <names>' ignores startup.json and starts exactly those agents.`,
	RunE: runUp,

(the now-stale invariant comment is at

agentfactory/internal/cmd/up.go

Lines 87 to 89 in b2b3305

	// Resolve the agent list. Positional args ALWAYS win (R-2, the highest C-4 risk):
	// `af up <names>` stays byte-identical to today, so the startup.json-driven actions
	// below are gated on this `blanket` flag. A bare `af up` is driven by startup.json:

)

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[stempeck]

K6 reassigns dispatchCfg.Mappings = knownMappings to drop unprovisioned agents, but dispatchCfg.Workflows is left unfiltered. matchWorkflow still matches workflow-labeled items against the unfiltered Workflows and routes them to handleWorkflowItem, where phaseMapping(w.dispatchCfg.Mappings, phase) returns !ok for any stripped phase and hits w.stall(...). Because stall does not mutate the item's labels, this fires on every dispatch cycle (default 300s), incrementing stats.errors — a repeating error-level stall, not the clean stats.skipped skip that non-workflow items get. The CRITICAL-2 guard comment calls this path "belt-and-suspenders for the Phase-1 validator", i.e. assumed unreachable for a valid config — but the K6 partition makes it reachable (the post-filter ValidateDispatchConfig continues over the stripped phase). The K8 pre-flight warning in up.go also tells the operator the dispatcher will "skip those mappings", which is inaccurate for workflow-labeled items.

[stempeck]

This pre-flight only warns when the state is dispatchStateUnprovisioned; dispatchStateInvalid is never surfaced. When the config loads at the struct level but fails ValidateDispatchConfig cross-file checks (e.g. notify_on_complete names an absent agent, or a workflow phase maps to a formula-less agent), no warning is printed here, and startDispatch's guard only catches ErrNotFound/ErrMissingField, so the dispatcher session still launches. The loop then fails ValidateDispatchConfig every cycle, logging only to the tmux pane — so af up exits cleanly and reports "Dispatcher started" while silently dispatching nothing, defeating the K8 observability goal for the invalid state.

[stempeck]

These two inline comments — // scheme://host/owner/repo -> strip "scheme://host". here and // scp-like user@host:owner/repo -> keep the part after ':'. at line 43 — restate what the adjacent code does rather than why, and the function-level doc comment already enumerates the URL shapes handled. Per CLAUDE.md (Code comments): "Code comments should ONLY exist when it makes sense to explain WHY the code is doing what it does ... Comments should be minimal or ideally, not exist at-all UNLESS they add VALUE to the code."