Skip to content

feat: implement T0.1, T0.2, T0.2a, T0.2b, T0.2c — agent interface, source layer, artifact history#31

Merged
muhammadbassiony merged 3 commits into
madrid-hackathonfrom
agent-interface
May 13, 2026
Merged

feat: implement T0.1, T0.2, T0.2a, T0.2b, T0.2c — agent interface, source layer, artifact history#31
muhammadbassiony merged 3 commits into
madrid-hackathonfrom
agent-interface

Conversation

@muhammadbassiony
Copy link
Copy Markdown
Collaborator

T0.1 — Define Agent interface in orchestrator (Go convention: interface at consumer)

  • Agent interface with Start/ExecuteChunk/GenerateSummary/Stop in orchestrator package
  • copilotcli.Client satisfies it implicitly via structural typing
  • Compile-time check in orchestrator_test.go
  • Shared MockAgent in internal/agent/mock.go for any test package

T0.2 — Refactor copilotcli to implement orchestrator.Agent

  • copilotcli.Client method signatures match Agent interface
  • Orchestrator depends on Agent interface, not concrete *copilotcli.Client
  • orchestrator no longer imports copilotcli in production code
  • Tests use agent.MockAgent

T0.2a — Create internal/source with adapters and SourceBundle

  • Adapter interface (Name, Fetch) defined in source package (consumer = Manager)
  • GDocsAdapter creates gdocs.Client per-Fetch (credentials vary per-request)
  • SourceBundle type combines Document + Design (reserved for Figma)
  • Manager coordinates adapters and assembles SourceBundle
  • Tests cover: gdocs result, no adapters, error, unknown type

T0.2b — Refactor orchestrator to consume source layer

  • Orchestrator calls source.Manager.Fetch() instead of gdocs directly
  • Orchestrator no longer imports internal/gdocs
  • OrchestrationResult uses *source.SourceBundle
  • PromptData unchanged (explicit named fields per spec)

T0.2c — Add append-only artifact history foundation

  • internal/artifacts/manager.go: timestamped run dirs, runs.jsonl index
  • Run directories: extraction/, prompts/, outputs/, logs/, screenshots/
  • --artifacts-dir CLI flag, BAUER_ARTIFACTS_DIR env var (default: ./bauer-artifacts)
  • Config.ArtifactsDir field with ApplyDefaults
  • .env.example with BAUER_ARTIFACTS_DIR and all env vars
  • bauer-artifacts/ added to .gitignore
  • Orchestrator writes extraction, prompts, outputs, metadata, and runs.jsonl

Spec 001 updated:

  • T0.1/T0.2: interface defined at consumer (orchestrator), not standalone package
  • All T0.1-T0.2c marked DONE with acceptance criteria checked

…urce layer, artifact history

T0.1 — Define Agent interface in orchestrator (Go convention: interface at consumer)
- Agent interface with Start/ExecuteChunk/GenerateSummary/Stop in orchestrator package
- copilotcli.Client satisfies it implicitly via structural typing
- Compile-time check in orchestrator_test.go
- Shared MockAgent in internal/agent/mock.go for any test package

T0.2 — Refactor copilotcli to implement orchestrator.Agent
- copilotcli.Client method signatures match Agent interface
- Orchestrator depends on Agent interface, not concrete *copilotcli.Client
- orchestrator no longer imports copilotcli in production code
- Tests use agent.MockAgent

T0.2a — Create internal/source with adapters and SourceBundle
- Adapter interface (Name, Fetch) defined in source package (consumer = Manager)
- GDocsAdapter creates gdocs.Client per-Fetch (credentials vary per-request)
- SourceBundle type combines Document + Design (reserved for Figma)
- Manager coordinates adapters and assembles SourceBundle
- Tests cover: gdocs result, no adapters, error, unknown type

T0.2b — Refactor orchestrator to consume source layer
- Orchestrator calls source.Manager.Fetch() instead of gdocs directly
- Orchestrator no longer imports internal/gdocs
- OrchestrationResult uses *source.SourceBundle
- PromptData unchanged (explicit named fields per spec)

T0.2c — Add append-only artifact history foundation
- internal/artifacts/manager.go: timestamped run dirs, runs.jsonl index
- Run directories: extraction/, prompts/, outputs/, logs/, screenshots/
- --artifacts-dir CLI flag, BAUER_ARTIFACTS_DIR env var (default: ./bauer-artifacts)
- Config.ArtifactsDir field with ApplyDefaults
- .env.example with BAUER_ARTIFACTS_DIR and all env vars
- bauer-artifacts/ added to .gitignore
- Orchestrator writes extraction, prompts, outputs, metadata, and runs.jsonl

Spec 001 updated:
- T0.1/T0.2: interface defined at consumer (orchestrator), not standalone package
- All T0.1-T0.2c marked DONE with acceptance criteria checked
@muhammadbassiony muhammadbassiony requested a review from Copilot May 13, 2026 09:37
@muhammadbassiony muhammadbassiony changed the base branch from main to madrid-hackathon May 13, 2026 09:41
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces foundational orchestration abstractions for Bauer v2: a consumer-side agent interface, a normalized source layer, and append-only artifact history for runs.

Changes:

  • Refactors orchestration to use orchestrator.Agent and source.Manager.
  • Adds artifact run management with run directories, metadata, and runs.jsonl.
  • Adds supporting tests, CI, docs updates, and environment/config surface for artifacts.

Reviewed changes

Copilot reviewed 22 out of 24 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
internal/workflow/workflow.go Reads suggestion counts from the new source bundle.
internal/source/types.go Adds normalized SourceBundle.
internal/source/source.go Defines source adapter and request interfaces.
internal/source/manager.go Coordinates source adapters into a bundle.
internal/source/manager_test.go Tests source manager behavior.
internal/source/gdocs.go Adds Google Docs source adapter.
internal/orchestrator/orchestrator.go Refactors orchestration around agent/source/artifact managers.
internal/orchestrator/orchestrator_test.go Adds agent-focused orchestrator tests.
internal/copilotcli/client.go Adjusts Copilot client to satisfy the new agent interface.
internal/config/config.go Adds artifacts directory config default.
internal/config/config_test.go Updates credential fixtures.
internal/config/cli.go Adds --artifacts-dir flag.
internal/artifacts/manager.go Adds artifact history manager.
internal/artifacts/manager_test.go Tests artifact directory and JSONL writing behavior.
internal/agent/mock.go Adds shared mock agent test double.
internal/agent/mock_test.go Tests mock agent conformance and call tracking.
docs/specs/001_v2_reconciliation.md Updates spec status and architecture notes.
config.json Updates sample document ID.
cmd/bauer/main.go Wires production agent/source/artifact managers into CLI workflow.
cmd/app/main.go Wires production agent/source/artifact managers into API server.
.workshop.lock Adds workshop lock file.
.gitignore Ignores artifact output directory.
.github/workflows/ci.yml Adds CI formatting, vet, test, and build checks.
.env.example Adds example Bauer environment variables.

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

Comment thread cmd/bauer/main.go Outdated
Comment thread cmd/app/main.go Outdated
return err
}

copilotClient, err := copilotcli.NewClient(cwd)
Comment thread cmd/app/main.go Outdated
Comment thread internal/orchestrator/orchestrator.go Outdated
Comment on lines +113 to +115
bundle, err := o.sources.Fetch(ctx, req)
if err != nil {
return nil, fmt.Errorf("failed to process document: %w", err)
return nil, fmt.Errorf("source fetch: %w", err)
Comment on lines +59 to +64
dirs := []string{
filepath.Join(runDir, "extraction"),
filepath.Join(runDir, "prompts"),
filepath.Join(runDir, "outputs"),
filepath.Join(runDir, "logs"),
}
Comment thread internal/artifacts/manager.go Outdated
Comment on lines +10 to +13
"bauer/internal/copilotcli"
"bauer/internal/source"
"bauer/internal/config"
"bauer/internal/prompt"
Comment thread internal/source/types.go Outdated
Comment thread .env.example Outdated
1. Move mock agent from internal/agent to internal/testutil (shared test doubles)
2. Clarify workflow vs orchestrator: orchestrator is the processing pipeline,
   workflow wraps it with GitHub clone/commit/PR steps
3. CLI bug fix: Copilot client created via factory after chdir to cloned repo,
   not at process startup (wrong cwd)
4. API bug fix: per-request Copilot client created with correct target repo cwd,
   not shared from server startup
5. Concurrency fix: per-request orchestrator in API handler eliminates shared
   mutable state across concurrent HTTP handlers
6. Fix chunk.Filename passed to WritePrompt: use filepath.Base() to extract
   just the filename, preventing nested directory structures
7. Fix finalizeRun called on errors: use defer in Execute() so failed runs
   are written to metadata.json/runs.jsonl with status="failed"
8. Fix ArtifactDir: use full relative path "bauer-artifacts/<run-id>"
   instead of just the run ID
9. Fix GenerateSummary: actually return fullSummary instead of empty string
10. Fix screenshots/ directory: created in NewRun alongside other subdirs
11. Fix run ID entropy: use crypto/rand with timestamp + 32 random bits
    instead of second-level timestamp with 16 bits
12. Fix SourceBundle.Document comment: clarify it can be nil when no gdocs
    adapter configured or fetch returns unrecognized type
13. Wire BAUER_* env vars as CLI fallbacks in cmd/bauer/main.go
14. Remove shared Orchestrator from RouteConfig; API builds per-request
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 26 out of 28 changed files in this pull request and generated 15 comments.

Comments suppressed due to low confidence (2)

docs/specs/001_v2_reconciliation.md:516

  • The documentation points readers to internal/agent/mock.go, but this PR adds the shared mock under internal/testutil/mock.go. This will send contributors to a package that does not exist in the updated codebase.
- `internal/orchestrator/orchestrator.go` — **modify** (add `Agent` interface)
- `internal/agent/mock.go` — **create** (shared test double)
- `internal/agent/mock_test.go` — **create** (verify mock satisfies interface)

docs/specs/001_v2_reconciliation.md:555

  • The acceptance criterion names internal/agent/mock.go and mock_test.go, but the implementation in this PR is under internal/testutil. The checked-off item is therefore misleading for future readers trying to locate the shared mock.
- [x] `Agent` interface exists in `internal/orchestrator/orchestrator.go` with exactly four methods
- [x] Interface is defined at the consumer (orchestrator), following Go convention
- [x] `internal/orchestrator` does not import `internal/copilotcli` anywhere
- [x] A `MockAgent` is available in `internal/agent/mock.go` for use in any test package
- [x] Compile-time check `var _ orchestrator.Agent = (*MockAgent)(nil)` in mock_test.go passes

Comment thread cmd/app/main.go
Comment thread cmd/bauer/main.go
Comment on lines +26 to +40
// envBoolFallback returns the env var bool if the flag value is false.
func envBoolFallback(flagVal bool, envKey string) bool {
if flagVal {
return flagVal
}
v := os.Getenv(envKey)
if v == "" {
return false
}
b, err := strconv.ParseBool(v)
if err != nil {
fmt.Fprintf(os.Stderr, "WARNING: invalid bool for %s=%q: %v\n", envKey, v, err)
return false
}
return b
Comment thread cmd/bauer/main.go Outdated
Comment on lines +136 to +149
artMgr := artifacts.NewManager(resolvedArtifactsDir)
gdocsAdapter := source.NewGDocsAdapter()
sources := source.NewManager(gdocsAdapter)

// Copilot client factory — called after chdir so cwd is the target repo
newAgent := func() (orchestrator.Agent, error) {
cwd, err := os.Getwd()
if err != nil {
return nil, fmt.Errorf("get working directory: %w", err)
}
return copilotcli.NewClient(cwd)
}

orch := orchestrator.NewOrchestrator()
orch := orchestrator.NewOrchestrator(nil, sources, artMgr)
if newAgent != nil {
agent, err := newAgent()
if err != nil {
return nil, fmt.Errorf("create agent: %w", err)
Comment on lines +113 to +116
// Build result for finalize if we haven't set one yet
if result == nil {
result = &OrchestrationResult{RunID: runID}
}
Comment on lines +194 to +198
_, err := orch.Execute(context.Background(), cfg)

// The fetch will fail, but finalizeRun should still have been called
// We verify by checking that a run directory was created
_ = err // error expected
Comment thread .env.example
# BAUER_FIGMA_TOKEN=figd_xxxxxxxxxxxxx

# --- API Server ---
BAUER_API_PORT=8090
Comment thread .env.example

# --- Copilot / model ---
BAUER_MODEL=gpt-5-mini-high
BAUER_SUMMARY_MODEL=gpt-5-mini-high
Comment thread internal/workflow/api.go
Comment on lines +118 to +120
// Execute workflow — nil agent factory since the workflow handler doesn't
// currently support Copilot execution (it only does extraction + PR)
workflowOutput, err := ExecuteWorkflow(ctx, input, orch, nil)
- `internal/copilotcli` implements it implicitly.
- The orchestrator depends on the `Agent` interface (defined in the orchestrator package), not the concrete `*copilotcli.Client`.
- Allows future agents (REST-based model, different SDK, test mock) to be plugged in without touching the orchestrator.
- A shared `MockAgent` test double lives in `internal/agent/mock.go` for use by any test package.
When ExecuteWorkflow chdirs into the cloned target repo, any relative
paths passed to the orchestrator or artifacts manager are evaluated
against the new cwd — meaning artifacts land inside the target repo.
CommitChanges only excluded bauer-output/ and bauer-doc-suggestions.json,
so bauer-artifacts/ files could be staged and committed into the PR.

Two-part fix:
1. Resolve OutputDir and ArtifactsDir to absolute paths BEFORE chdir,
   both in cmd/bauer/main.go (the caller) and as a safety net in
   ExecuteWorkflow itself. The artifacts manager is also created with
   the absolute path.
2. Add bauer-artifacts/ to CommitChanges' exclude list as a defensive
   safety net, so even if a relative path slips through it won't be
   committed.

Also add ArtifactsDir field to WorkflowInput so the workflow can
propagate the absolute path to its caller-configured artifacts manager.
@muhammadbassiony muhammadbassiony merged commit e2dce79 into madrid-hackathon May 13, 2026
1 check failed
@muhammadbassiony muhammadbassiony deleted the agent-interface branch May 13, 2026 11:01
This was referenced May 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants