Skip to content

F108: F107 Facade Completion (GREEN) Across All Interfaces #373

@pocky

Description

@pocky

F108: F107 Facade Completion (GREEN) Across All Interfaces

Scope

In Scope

  • Route all four interfaces (CLI, TUI, HTTP, ACP) through ports.WorkflowFacade.Run + RunSession.Events() as the sole execution path
  • Enrich the facade event model (ports.Event + ProjectEvent) to losslessly carry step name, message content, error, duration, and InputRequest prompt
  • Implement CLI Adapter.Status properly (persistent by-ID lookup) replacing the not-found stub
  • Switch TUI run action from legacy bridge.RunWorkflow to RunWorkflowViaFacade, drop the 200ms poll
  • Make HTTP POST /api/workflows/{scope}/{name}/run execute via facade and register the session for SSE/respond lookups
  • Activate ACP dispatchViaFacade, wire SetFacade in acp_serve.go, ensure per-session state isolation, project EventInputRequired as session/update carrying the prompt
  • Make conformance gates green: TestFacadeConformance_4Interfaces (cli-stdout, acp-session-update, sse-frames, tui-tea-msg), TestFacadeE2E_CtxCancelProducesWorkflowFailed, TestFacadeResume_RestoresState
  • Delete dead/legacy migration code only after each interface is green (legacy ACP runner branch, input_reader.go + test, CLI interactiveExec, TUI tickMsg/executionPollMsg, legacy HTTP run path, duplicate no-op recorders)
  • Consolidate duplicate no-op recorders into a single transcript.NopRecorder
  • Update .go-arch-lint.yml for new edges and remove stale ones
  • Mark T060/T061/T062/T063/T065 completed in .specify/implementation/F107/tasks/index.json only after they build+test

Out of Scope

  • Tools hardening (F109)
  • New domain features beyond lossless field propagation
  • Changes to F105 SDK update mapping (toSDKUpdate reused as-is)
  • Pushing commits to remote
  • Modifying the conformance golden files to make tests pass

Deferred

Item Rationale Follow-up
Legacy WorkflowEventProjector removal Keep only if facade projection fully replaces it; otherwise document why future
Generalized per-run state context API beyond ACP needs Solve concretely for ACP first; generalize when a second consumer appears future
Reworking F105 emitter routing Service-level routing by sessionID is already correct none

User Stories

US1: All interfaces execute through the facade with no output regression (P1 - Must Have)

As an AWF maintainer,
I want every interface (CLI, TUI, HTTP, ACP) to execute workflows exclusively through ports.WorkflowFacade.Run and consume RunSession.Events(),
So that F107's single-core promise is actually delivered and legacy execution paths can be deleted without regressing user-visible output.

Why this priority: F107 was marked completed but its central promise was never delivered — facade paths are dead code, every interface still runs legacy. Without this, F109 (tools hardening) is blocked and the architecture diverges from the documented design.

Acceptance Scenarios:

  1. Given the CLI is invoked via awf run <workflow>, When the workflow runs to completion, Then execution flows through cfg.Facade.Run, stdout matches the cli-stdout golden, and the exit code maps from the terminal event.
  2. Given the TUI run action is triggered, When the workflow executes, Then RunWorkflowViaFacade drives step state from Events(), the legacy bridge.RunWorkflow call at model.go:168 is gone, and the tui-tea-msg golden passes.
  3. Given an HTTP client POSTs to /api/workflows/{scope}/{name}/run and subscribes to SSE, When the run starts, Then the session is registered in the lookup registry, SSE frames stream end-to-end, and the sse-frames golden passes.
  4. Given an ACP client opens a session and dispatches a turn, When the workflow executes, Then dispatchViaFacade is taken (not the legacy runner.Run branch), session/update frames carry step_name/error/duration/assistant content, and the acp-session-update golden passes.

Independent Test: Run go test -tags=integration ./tests/integration/features/... and verify all four TestFacadeConformance_4Interfaces subtests pass against their respective goldens in tests/fixtures/facade/.

US2: Facade event model is lossless vs legacy projectors (P1 - Must Have)

As an interface implementer,
I want ports.Event and ProjectEvent to carry step name, message content, error, duration, and InputRequest prompt without leaking infrastructure types into the domain,
So that every interface projection reads enriched fields directly and no metadata is silently dropped on the way from transcript to interface.

Why this priority: The current model drops Metadata["step_name"], duration_ms, error, and assistant content before projection. facadeEventToUpdate even sets assistant text to ev.RunID (a bug). Without enrichment, every Phase 2 projection regresses output and the conformance goldens cannot pass.

Acceptance Scenarios:

  1. Given a transcript ExchangeEvent with Metadata["step_name"], Metadata["duration_ms"], and Metadata["error"], When ProjectEvent converts it to ports.Event, Then all three fields are readable from the enriched Payload with no domain layer importing transcript or infra types.
  2. Given an EventInputRequired produced by the facade, When consumers read its Payload, Then the InputRequest prompt text is present and projectable to session/update.
  3. Given a step_failed or step_retrying transcript event, When projected through the facade, Then the resulting ports.Event carries the error message and step name losslessly.

Independent Test: Run unit tests on ProjectEvent asserting per-event-type round-trip of meaningful fields with -race; assert no field is silently zero-valued.

US3: ACP per-session state isolation under concurrent runs (P2 - Should Have)

As an ACP client running multiple sessions concurrently,
I want each session's facade run to use an isolated state context,
So that concurrent sessions running the same workflow do not clobber each other's state file (a bug the legacy path avoided via acpSessionStateDir).

Why this priority: A single service-level facade.Run would reintroduce a known concurrency bug. This is required for ACP correctness but is scoped to ACP rather than user-facing functionality across all interfaces.

Acceptance Scenarios:

  1. Given two ACP sessions dispatch the same workflow concurrently, When both runs execute via the facade, Then their state files do not collide and -race is clean.
  2. Given the legacy acpSessionStateDir isolation pattern, When replaced by per-session/per-run facade state, Then the isolation property is preserved or strengthened with a regression test.

Independent Test: Run a -race test in internal/application launching two concurrent ACP sessions on the same workflow and asserting no state-file collision and no data race.

US4: Dead/legacy code removed after facade is live (P2 - Should Have)

As an AWF maintainer,
I want all legacy execution paths, dead facade scaffolding, and duplicate no-op recorders deleted once the facade is the sole path,
So that the codebase has a single source of truth, arch-lint stays clean, and future contributors are not misled by dead code.

Why this priority: Leaving dead code is the lesson from prior F107 attempts — paths get re-attached, regressions hide. Deletion must happen but only after each interface is verifiably green.

Acceptance Scenarios:

  1. Given all four interfaces are green via the facade, When legacy paths are grep'd, Then zero references remain to runner.Run ACP branch, acpinfra.NewACPInputReader, CLI interactiveExec, TUI tickMsg/executionPollMsg, and the legacy HTTP run path.
  2. Given cli.nopRecorder and tui.tuiNopRecorder, When consolidated, Then only transcript.NopRecorder remains and both interfaces import it.
  3. Given .go-arch-lint.yml, When deletions land, Then stale edges are removed and make lint-arch returns OK.

Independent Test: grep for each deleted symbol returns zero hits; go build ./... green; make lint and make lint-arch both pass.

Edge Cases

  • What happens when two ACP sessions run the same workflow concurrently? State files must remain isolated (US3).
  • How does the system handle EventInputRequired when the prompt text is empty? Emit session/update with empty prompt; do not crash; park as today.
  • What is the behavior when facade.Run returns ctx-cancel mid-stream? Terminal event maps to workflow_failed per TestFacadeE2E_CtxCancelProducesWorkflowFailed.
  • What happens when SSE subscribers connect before the run registers its session? The registry must be populated synchronously in the run handler so the next subscriber lookup succeeds; otherwise documented as a known race.
  • How does CLI Adapter.Status behave for unknown IDs? Returns a not-found error mapped to the user-error exit code (1), not a panic.
  • What if the conformance golden file would need updating to reflect a legitimate facade-era format change? STOP and report rather than weakening the test.

Requirements

Functional Requirements

  • FR-001: System MUST execute every CLI awf run invocation through cfg.Facade.Run consuming RunSession.Events(), with terminal event → exit code mapping per the existing error taxonomy
  • FR-002: System MUST execute every TUI run action through RunWorkflowViaFacade, driving step state from Events() with no 200ms poll
  • FR-003: System MUST execute every HTTP POST /api/workflows/{scope}/{name}/run through the facade and register the returned RunSession in the session registry used by SSE and respond handlers
  • FR-004: System MUST execute every ACP turn through dispatchViaFacade with service.SetFacade(...) wired in internal/interfaces/cli/acp_serve.go
  • FR-005: System MUST enrich ports.Event to carry step name, message/assistant content, error message, duration, and InputRequest prompt without importing transcript or infrastructure types into the domain layer
  • FR-006: System MUST update ProjectEvent to propagate transcript Metadata (step_name, duration_ms, error) and message content into the enriched ports.Event payload losslessly
  • FR-007: System MUST fix facadeEventToUpdate so assistant message text is the actual message content (not ev.RunID) and step_name/error/duration/step_failed/step_retrying are present
  • FR-008: System MUST project EventInputRequired to an ACP session/update carrying the prompt text before parking
  • FR-009: System MUST provide per-session/per-run state isolation for ACP facade runs (per-session facade or per-run state context threaded into facade.Run)
  • FR-010: Users MUST be able to invoke awf status <workflow-id> and receive accurate status via Adapter.Status backed by a persistent by-ID lookup (no not-found stub)
  • FR-011: Users MUST be able to invoke list/history/resume/resume-list/validate routed through cfg.Facade
  • FR-012: System MUST delete legacy execution paths after each interface is green: legacy ACP runner.Run branch, internal/infrastructure/acp/input_reader.go and its test, CLI direct execSvc.RunWithWorkflowAndRunID call and interactiveExec, TUI bridge.RunWorkflow + tickMsg/executionPollMsg, and the legacy HTTP run path
  • FR-013: System MUST consolidate cli.nopRecorder and tui.tuiNopRecorder into a single transcript.NopRecorder in internal/infrastructure/transcript/
  • FR-014: System MUST update .go-arch-lint.yml for new package edges introduced by the facade-only path and remove stale edges left by deletions
  • FR-015: System MUST write .agent/todo/F108-f107-facade-completion-green.md (Status/Position/Upstream deps/Unblocks header; Goal; Task breakdown derived from Phases 1–4; Gate/DoD; Out of scope) and commit it alone before any code change
  • FR-016: System MUST implement and pass the four T063 ACP unit tests that pin the facade dispatch contract: TestACPSessionService_RoutesRunThroughFacade (turn dispatch takes dispatchViaFacade, not the legacy runner.Run branch), TestACPSessionService_ProjectsEventsToSessionUpdate (facade events project to session/update frames), TestACPSessionService_RespondCallsSessionRespond (a client respond routes to RunSession.Respond), and TestACPSessionService_ContinuationParkingPreserved (continuation parking survives across turns). These are unit-level guards complementary to the acp-session-update conformance golden, not a substitute for it.
  • FR-017: System MUST preserve the sessionCtx/runCtx split and the ParkedTurnCount/continuation-parking semantics across ACP turns when execution is routed through the facade, so that a per-request context cancellation does not tear down the session and a parked turn resumes on the next prompt exactly as in the legacy path (todo Phase 3 §119)

Non-Functional Requirements

  • NFR-001: go test -tags=integration ./tests/integration/... MUST be green including TestFacadeConformance_4Interfaces (all 4 subtests), TestFacadeE2E_CtxCancelProducesWorkflowFailed, and TestFacadeResume_RestoresState
  • NFR-002: make lint (golangci-lint) MUST report 0 violations AND make lint-arch MUST be OK; both gates are run before every commit
  • NFR-003: internal/application and internal/interfaces/... MUST be -race clean
  • NFR-004: The build MUST NOT be broken between commits; every commit is independently buildable and testable
  • NFR-005: Switching to the facade MUST NOT degrade interface output vs the legacy path; conformance goldens are the judge — no golden weakening, renaming, or skipping to make a test pass

Success Criteria

  • SC-001: Four conformance subtests (cli-stdout, acp-session-update, sse-frames, tui-tea-msg) pass against unchanged golden files
  • SC-002: grep -R for legacy symbols (execSvc.RunWithWorkflowAndRunID in interactive CLI run, bridge.RunWorkflow in TUI, runner.Run ACP branch, acpinfra.NewACPInputReader, interactiveExec, tickMsg, executionPollMsg, tuiNopRecorder, cli.nopRecorder) returns 0 hits in the live codebase
  • SC-003: Two concurrent ACP sessions on the same workflow complete without state-file collision, verified by a -race test
  • SC-004: TestACPInputReader_FileDeleted (D35) no longer exists and TestACP_NoDirectRecorderSubscribe passes
  • SC-005: T060/T061/T062/T063/T065 in .specify/implementation/F107/tasks/index.json flip to completed only after corresponding build+test gates are green
  • SC-006: Phase commits land in order (event-model → CLI → TUI → HTTP → ACP → deletions) with each commit independently building, linting, and testing green
  • SC-007: The four T063 ACP unit tests (TestACPSessionService_RoutesRunThroughFacade, ...ProjectsEventsToSessionUpdate, ...RespondCallsSessionRespond, ...ContinuationParkingPreserved) pass -race, and a regression test asserts the sessionCtx/runCtx split and ParkedTurnCount continuation are preserved under facade dispatch (FR-016, FR-017)

Key Entities

Entity Description Key Attributes
ports.Event Domain-layer facade event carrying enriched, lossless run-level information Seq, Kind, RunID, ParentRunID, Payload (typed per Kind), Timestamp; new fields: step name, message content, error message, duration, InputRequest prompt
RunSession Facade-returned run handle exposing the event stream and terminal semantics Events() channel, Respond(), terminal event mapping
WorkflowFacade Single-core entry point all four interfaces call Run(ctx, spec) → RunSession; per-run/per-session state isolation hook (ACP)
ProjectEvent Pure projection from transcript.ExchangeEvent to ports.Event Lossless metadata propagation; no infra types in signature
transcript.NopRecorder Single shared no-op recorder consolidating cli.nopRecorder + tui.tuiNopRecorder Implements transcript.Recorder; zero-allocation
ACP session state context Per-session/per-run isolation primitive replacing acpSessionStateDir sessionID-keyed; threaded into facade.Run or via per-session facade

Assumptions

  • The F107 facade construction (buildFacade, NewRootCommandAutoFacade, bridge.SetFacade, SetSessionLookup) is correct and reusable; only its consumers need to switch over
  • The F105 emitter and toSDKUpdate SDK mapping are correct and stay as-is; ACP work reuses them
  • The conformance goldens in tests/fixtures/facade/*.golden accurately encode the expected facade-era output and are the contract — they are not edited
  • Per-session facade construction is acceptable for ACP state isolation; if not, a per-run state context threaded into facade.Run is acceptable instead (decision recorded in the F108 spec doc)
  • The original T063 "do NOT touch CLI" constraint is unsatisfiable because ACP wiring lives in internal/interfaces/cli/acp_serve.go; the override is deliberate and documented in the commit and the F108 spec
  • The legacy WorkflowEventProjector is removable only if facade projection fully replaces it; otherwise it stays with a documented rationale

Metadata

  • Status: in-progress
  • Version: v0.11.0
  • Priority: high
  • Estimation: XL

Dependencies

  • Blocked by: F107
  • Unblocks: F109

Clarifications

Section populated during clarify step with resolved ambiguities.

Notes

  • TDD is mandatory: failing/conformance tests define the contract. Never weaken, skip, rename-to-dodge, or delete a test to make it pass. If a test or the T063 spec is wrong, fix production code or document a corrected decision in the F108 spec — do not bend the test.
  • Commit incrementally after every green step (prior session lost uncommitted work to a working-tree wipe). Never leave the build broken between commits. Do not push.
  • Run BOTH make lint (golangci) and make lint-arch before every commit; a prior commit shipped an arch-lint violation because only golangci was run.
  • Order of phases: (0) F108 spec file → (1) enrich event model → (2a) CLI → (2b) TUI → (2c) HTTP → (3) ACP → (4) deletions. Each phase is its own commit (or small chain of commits).
  • ACP override of T063 "no CLI touch" rule: wiring lives in internal/interfaces/cli/acp_serve.go; override documented in commit message and in the F108 spec as a corrected decision.
  • If a sub-problem turns out to be a genuine architectural contradiction you cannot resolve cleanly, STOP and report it with evidence rather than shipping a regression.
  • Update .specify/implementation/F107/tasks/index.json (T060/T061/T062/T063/T065 → completed) only after corresponding build+test gates pass — not on paper.

Metadata

Metadata

Assignees

No one assigned

    Labels

    featureFeature specificationv0.11.0Target version

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions