From 02fb36e9455297727bf12d019e5d10312783987a Mon Sep 17 00:00:00 2001 From: Ian Gulliver Date: Tue, 20 Oct 2026 22:08:12 -0700 Subject: [PATCH] Audit of 5eab259 --- docs/operations.md | 6 +++--- internal/agents/process.go | 13 ++++--------- internal/agents/workflows.go | 10 +++------- internal/workflow/workflow.go | 4 ---- 4 files changed, 10 insertions(+), 23 deletions(-) diff --git a/docs/operations.md b/docs/operations.md index 3735d32..aed4d25 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -9,7 +9,7 @@ - `internal/terminal`: each task's shell on a PTY, shared by the terminal WebSocket and the agents' terminal tools (see "Terminals"). - `internal/git`: worktrees, the task changes view (see "Changes"), snapshots (see "Snapshots"), unmerged-change summaries, squash merges, merge notes and pushes. - `internal/agents`: the agent session manager. It owns the running agent processes, task agents, lead agents (see "Lead agent") and reviewers (see "Reviews") alike, and records their activity as events. -- `internal/claude`: the Claude Code stream-json client (see `docs/claude-protocol.md`). +- `internal/claude`: the Claude Code stream-json client (see `docs/claude-protocol.md`). The agent manager assembles its hooks from one shared guidance hook, adding the post-tool hook for non-review sessions and the prose-question hook for task implementers. - `internal/codex`: the `codex app-server` JSON-RPC client, with types generated from the CLI's schema by `go generate ./internal/codex` (see `docs/codex-protocol.md`). One goroutine reads the CLI's stdout and blocks on the bounded `Events` channel when its consumer falls behind, so responses queued behind those notifications wait too: the goroutine draining `Events` (`codexProcess.drive`) therefore never waits on a reply itself. It writes `turn/steer` in order and waits for the reply on another goroutine, and starts the prose-question nudge and the turn-end `Deliver` on their own goroutines. Every request gives up after `rpcTimeout`, or the caller's earlier deadline, and that deadline covers waiting for the write lock, writing the request to stdin and waiting for the reply. Stdin is an `os.Pipe`, so writes take a deadline. A request whose deadline passes before any of it is written fails and leaves the CLI running. One that times out after part of its line is written has broken the framing, so byre kills the CLI, logging `codex stopped reading its stdin mid-line`, and the session ends as any exited process does. - `internal/antigravity`: the Antigravity CLI (`agy`) stream-json client (see `docs/antigravity-protocol.md`). Each session has its own plugin directory and a `127.0.0.1` listener that serves byre's MCP tools and hooks to the CLI, and restarts the CLI on the same conversation after an interrupt or a model switch. - `internal/workflow`: parsing and validating workflow definitions and walking their steps (see "Workflows"). @@ -496,7 +496,7 @@ Reviews look at one task's change; an audit checks the whole codebase as it stan ## Workflows -A workflow is a named tree of steps: stages, each a stage name, a model and the stage agent's prompt written out in full, nested in `order` and `parallel` blocks. Every stage runs as its own task, once the step before it has finished, so a stage's output can go to several branches of a `parallel` (fan-out) and the stage after a `parallel` waits for all its branches (fan-in). Workflows live in byre's database, in the `workflows` table (`name`, `definition_id`) pointing at normalized definition rows (see "Storage" below), shared by every project and never in a repo. byre stores, validates, exports, imports and freezes them, and runs each run's frozen copy stage by stage (`internal/agents/workflows.go`; the graph helpers `After`, `Next`, `Starts`, `Ends` and `FromMain` share one tree walk in `internal/workflow/workflow.go`). The human starts one from the task switcher (see "Starting"), or from the left pane's Workflows tab, which lists them and hands a run to the lead (see "UI"); the human edits them in that tab (see "UI"), and the lead with its tools, drawing a workflow for the human as a Mermaid flowchart of its stage names, which its chat renders (see "Inbox"). A run's progress is shown read-only in the stage tasks' Workflow tab (see "Progress" below). +A workflow is a named tree of steps: stages, each a stage name, a model and the stage agent's prompt written out in full, nested in `order` and `parallel` blocks. Every stage runs as its own task, once the step before it has finished, so a stage's output can go to several branches of a `parallel` (fan-out) and the stage after a `parallel` waits for all its branches (fan-in). Workflows live in byre's database, in the `workflows` table (`name`, `definition_id`) pointing at normalized definition rows (see "Storage" below), shared by every project and never in a repo. byre stores, validates, exports, imports and freezes them, and runs each run's frozen copy stage by stage (`internal/agents/workflows.go`; the graph helpers `After`, `Next`, `Starts` and `Ends` share one tree walk in `internal/workflow/workflow.go`). The human starts one from the task switcher (see "Starting"), or from the left pane's Workflows tab, which lists them and hands a run to the lead (see "UI"); the human edits them in that tab (see "UI"), and the lead with its tools, drawing a workflow for the human as a Mermaid flowchart of its stage names, which its chat renders (see "Inbox"). A run's progress is shown read-only in the stage tasks' Workflow tab (see "Progress" below). - **Names.** The name is the workflow's key and is shown to the human on buttons and pickers, so it is free text: one line of 1 to 100 characters, trimmed (`workflow.CleanName`). It isn't part of the definition, so the file doesn't carry it. In URLs it is a path-escaped segment. - **Definition.** A workflow is a name and a `WorkflowDefinition` (`api/openapi.yaml`): an optional `description` and its `steps`, run in order. Any workflow can run on a brief, each stage its own task, and any whose stages are single agents can be a project's review workflow, each stage a reviewer of every snapshot (see "Reviews"). A step (`WorkflowStep`) is exactly one of a `stage` (its name, with `model` or `agents`, and `prompt`), an `order` (steps one after another) or a `parallel` (branches at the same time). A stage gets the outputs of the stages that end the step before it (the last stage of an `order`, the last stage of each branch of a `parallel`; `workflow.After` derives them by walking the tree), and a stage with nothing before it starts from the brief. Where its task branches from follows from that: from the commit its one earlier stage finished at, from the default branch when it comes after several, and for a stage with none, from where the run was started. In place of `model`, a stage may list `agents` (each `name`, `model` and `prompt`) to run as a team in its one task (see "Teams"): each agent gets the stage's message and then its own prompt, and the first agent calls `finish_stage`. It travels as JSON everywhere; `workflow.Parse` reads it, refuses unknown fields and validates it. @@ -504,7 +504,7 @@ A workflow is a named tree of steps: stages, each a stage name, a model and the - **Validation.** `workflow.Validate` refuses a definition (400, everything wrong listed, each located by its path such as `steps[0].parallel[1]`) with no steps, a step that isn't exactly one of `stage`, `order` and `parallel`, `model`, `prompt` or `agents` on an `order` or `parallel`, an empty `order` or `parallel`, a stage name that isn't a valid one-line name or repeats another stage's, an empty model without `agents` or a model with them, `agents` that fail `workflow.TeamProblems`, or an empty prompt. A workflow may end with several stages (a last `parallel`). The tree can't express a cycle. A stage's vendor follows from its model (`workflow.VendorOf`, the same rule sessions use: `gpt-*` is Codex, `gemini-*` Antigravity, everything else Claude Code); a file with a `vendor` key is refused as an unknown field. - **API.** `GET /api/workflows` lists every workflow's name and description. `GET /api/workflows/{name}` returns its definition; `PUT` takes a definition as the body and creates or replaces the workflow; `DELETE` removes it and turns reviews off (a `project.updated` event each) for the projects that reviewed with it. Each save or delete records a `workflow.updated` or `workflow.deleted` event carrying the name. `sqlite3 byre.db 'SELECT name FROM workflows'` lists them. Saves and reads go through the Manager (`internal/agents/workflows.go`) so the HTTP API and the lead's tools share them. - **Lead tools.** `list_workflows`, `read_workflow` (`name`, or `task_id` for a task's frozen copy; returns the definition as JSON), `save_workflow` (`name` and `definition`, the definition as an object whose input schema spells out every field, so nothing is quoted or escaped by hand) and `delete_workflow`, with the same checks and errors as the API. Task agents don't get them. -- **Starting.** The lead's `create_task` and `start_todo` take an optional `workflow` (404 when there is none of that name) and then start a run of it instead of an agent on the prompt (`Manager.StartWorkflow`); `model` together with `workflow` is refused, since each stage names its own. `POST /api/projects/{id}/tasks` takes the same optional `workflow`, with the title, required then, as the run's brief, and returns the run's first stage task. The task switcher (`mac/Sources/Byre/Switcher.swift`) loads `GET /api/workflows` when it opens, and when the message (after any project prefix) starts with a workflow's name, ignoring case, followed by a space or a colon and more text (the longest such name when several match), it offers a "Start in …" row before each "Start in …" row: `deploy: byre` or `deploy byre` starts a run of `deploy` with `byre` as its title and brief, and opens its first stage task. Text that starts with no workflow's name gets no workflow rows. The switcher's box grows with what is typed (Shift+Enter adds a line) and takes pasted or dropped images; with images attached it lists only the "Start in …" rows, and "Start in …" uploads them as blobs and sends their ids as `POST /api/projects/{id}/tasks`'s `images`, which start the agent with or without a `prompt` (refused with `workflow`). Closing the switcher drops its images. A run gets a new id and creates a task for every stage with nothing before it (`workflow.Starts`), branched from the lead's `from_task_id` HEAD when given, else from the default branch. Each stage task keeps the workflow's name and definition as they are when the run starts in `tasks.workflow_name` and `tasks.workflow_definition_id`, so later saves or deletes don't change it; its brief into `tasks.workflow_brief` (the lead's `create_task` `prompt`, optional with a workflow and may be empty, or for `start_todo` the TODO item as `todoPrompt` words it); the run's title into `tasks.workflow_title`; the run id into `tasks.workflow_run`; and its stage's name into `tasks.workflow_stage`. Stage tasks are titled `: <stage>`, with the run's title as their merge title, and carry the run's TODO item. The Task's `workflow`, `workflow_run` and `workflow_stage` are those; `GET /api/tasks/{id}/workflow` returns the copy as `{name, definition}` (404 for a task without one). A task agent's `create_subtask`, `POST /api/projects/{id}/tasks` without `workflow` and the Project tab's Start create tasks without one. A fork of a stage task copies its frozen workflow, brief, title, run and stage, so it is another attempt at the same stage of the same run. `sqlite3 byre.db "SELECT id, status, workflow_stage, stage_output != '' FROM tasks WHERE workflow_run = '<run>'"` lists a run's tasks and which have finished. +- **Starting.** The lead's `create_task` and `start_todo` take an optional `workflow` (404 when there is none of that name) and then start a run of it instead of an agent on the prompt (`Manager.StartWorkflow`); `model` together with `workflow` is refused, since each stage names its own. `POST /api/projects/{id}/tasks` takes the same optional `workflow`, with the title, required then, as the run's brief, and returns the run's first stage task. The task switcher (`mac/Sources/Byre/Switcher.swift`) loads `GET /api/workflows` when it opens, and when the message (after any project prefix) starts with a workflow's name, ignoring case, followed by a space or a colon and more text (the longest such name when several match), it offers a "Start <workflow> in …" row before each "Start in …" row: `deploy: byre` or `deploy byre` starts a run of `deploy` with `byre` as its title and brief, and opens its first stage task. Text that starts with no workflow's name gets no workflow rows. The switcher's box grows with what is typed (Shift+Enter adds a line) and takes pasted or dropped images; with images attached it lists only the "Start in …" rows, and "Start in …" uploads them as blobs and sends their ids as `POST /api/projects/{id}/tasks`'s `images`, which start the agent with or without a `prompt` (refused with `workflow`). Closing the switcher drops its images. A run gets a new id and creates a task for every stage with nothing before it (`workflow.Starts`), branched from the lead's `from_task_id` HEAD when given, else from the default branch. Initial stages use that starting base directly; later stages choose their base from their loaded parents: one parent supplies its commit, several parents use the default branch (the audited commit for an audit). Each stage task keeps the workflow's name and definition as they are when the run starts in `tasks.workflow_name` and `tasks.workflow_definition_id`, so later saves or deletes don't change it; its brief into `tasks.workflow_brief` (the lead's `create_task` `prompt`, optional with a workflow and may be empty, or for `start_todo` the TODO item as `todoPrompt` words it); the run's title into `tasks.workflow_title`; the run id into `tasks.workflow_run`; and its stage's name into `tasks.workflow_stage`. Stage tasks are titled `<title>: <stage>`, with the run's title as their merge title, and carry the run's TODO item. The Task's `workflow`, `workflow_run` and `workflow_stage` are those; `GET /api/tasks/{id}/workflow` returns the copy as `{name, definition}` (404 for a task without one). A task agent's `create_subtask`, `POST /api/projects/{id}/tasks` without `workflow` and the Project tab's Start create tasks without one. A fork of a stage task copies its frozen workflow, brief, title, run and stage, so it is another attempt at the same stage of the same run. `sqlite3 byre.db "SELECT id, status, workflow_stage, stage_output != '' FROM tasks WHERE workflow_run = '<run>'"` lists a run's tasks and which have finished. - **Stages.** A stage task's agent is an implementer session on the stage's model (for a team stage, one per agent, see "Teams"), created by the user who started the run; any later session of the task (a Compact handoff's, one started by a message) runs the same stage, since the stage belongs to the task. Starting a stage records a `task.workflow_stage` event on its task (`workflow`, `run`, `stage` the name, `after` the ids of the tasks whose outputs it gets; actor the lead for the stages a run starts with, `byre` after) and sends the session its first message, actor `byre`, so the transcript shows it collapsed under its first line: `Workflow "<name>", stage "<stage>"`, then the stage's prompt, a `## Brief` section when the brief isn't empty, a `## From stage "<stage>" (task <id>, branch <branch>)` section with each `after` stage's output, and a line saying where the task starts: from the default branch, with the branches above to `git merge` or read as it needs, or from the commit its one `after` stage finished at. Nothing else of earlier stages is passed on; their branches carry their work. - **Advancing.** A stage session gets the `finish_stage` tool (`output`, required) and a guidance line naming the workflow, its stage and the stages after it. The call stores the output in `agent_sessions.stage_output`, replacing any earlier one; it is refused once the task's stage has finished. When a turn of that session ends normally (not an error or interrupt, not a handoff, not a Merge turn that squashes) with an output stored, byre sets `agent_sessions.stage_finished` before marking the session idle, and then advances (`nextStage`): it records the output and the worktree's HEAD in `tasks.stage_output` and `tasks.stage_commit`, then, holding `workflowMu` so two stages finishing at once can't both start the one after them, goes through each stage after this one. A stage starts once every stage it comes after has a finished task in the run (the latest one, when a fork made several); its task branches from that task's `stage_commit` when it comes after one stage, else from the default branch. Once every stage after it has been dealt with, byre clears `stage_output` and `stage_finished` on the session. The session stays idle in its task for more messages, and queued messages and held answers are delivered to it once advancing is done. The workflow is finished once every stage has: when a last stage (one with nothing after it, `workflow.Ends`) finishes, byre checks that every last stage has a finished task in the run, and until they all do it only clears the flags. Since each stage records its result before taking `workflowMu`, two last stages finishing at once could both see the run complete, so under the lock a run that already has `task.workflow_finished` only clears the flags too. When the last of them finishes, byre starts discarding every other active task of the run, keeping each last stage's finished task (as `byre`, through the same `Discard` as the button, so each gets its summary turn first and closes when that turn ends; one already merging or being discarded is skipped and logged as `stage task not discarded`), then records `task.workflow_finished` (`workflow`, `output`, and `kept`, the kept tasks' ids) on the task that finished last and clears the flags in the same transaction, leaving the kept tasks for the human to merge; each of them is then asked whether it is ready to merge. With one last stage `output` is its output; with several, each one's under a `## <stage>` heading. A turn that ends without the call leaves the task in its stage. The output belongs to the turn that stored it: when that turn ends any other way (an error, an interrupt, a handoff, a Merge turn that squashes, an agent process that exits mid-turn, or a session that can't be resumed after a restart), byre clears `stage_output` before marking the session idle, so a later turn can't hand off a stale output; the agent calls `finish_stage` again. A turn cut off by a byre restart and resumed keeps it. If a stage can't start, byre logs `advance workflow stage` and records the error in the finishing task's transcript, and the flag stays set. On startup, before resuming running sessions, `recoverStages` advances every session with `stage_finished` set, skipping the steps already done: the recorded output isn't overwritten; a stage that already has a task in the run isn't created again, but when that task is active and has no session, its sessions are started, and each of its sessions that never got a message (`SessionStarted`) is sent the stage's first message (a team member's with its own prompt), and the restart notice isn't sent to them. `sqlite3 byre.db 'SELECT id, task_id FROM agent_sessions WHERE stage_finished = 1'` lists stages waiting to advance. `list_tasks` shows each stage task's workflow and `stage "<name>"`, with `(finished)` once its stage has finished and `workflow finished` once any task of the run has a `task.workflow_finished` event. A stage belongs to its task (`tasks.workflow_stage`); migration `079-drop-obsolete-inbox-and-session-columns` removes the unused `agent_sessions.workflow_stage` column. - **Progress.** `GET /api/tasks/{id}/workflow/run` (`Manager.WorkflowRun`) returns the run of a stage task (404 for a task without one): the definition that task froze, name, title, brief, `finished` (any task of the run has `task.workflow_finished`), and per stage in the tree's order its `after` (the stages whose outputs it gets, from `workflow.After`), `waiting_on` (the `after` stages with no finished attempt), its `attempts` and a `state`. An attempt is every task of the run at that stage, closed ones included, oldest first (the first and its forks), with its `output` and `commit` once `tasks.stage_output` is set, `forked_from`, `discarded_by` (the `task.discarded` actor) and `inputs`: the tasks named in the `after` of its latest `task.workflow_stage` event, each with its stage, so it says which attempt fed it; a fork has no such event and takes its source's. An attempt's state is `merged`, `cleaned_up` (discarded after finishing; `byre` as `discarded_by` is the run's cleanup when it finished), `discarded` (before finishing), `finished`, `running` (a session is running) or `idle`. A stage is `waiting` with no attempts, else it takes the state of its latest finished attempt, or its latest attempt when none finished. The Workflow tab (`mac/Sources/Byre/Workflow.swift`) draws the graph natively with the flowchart layout from `Diagram.swift`, each stage's node coloured by its state (`stateColors`, from the palette's tint and accent pairs: waiting grey, running accent, idle warn, finished, merged and cleaned up ok, discarded danger) and redrawn whenever a refetch changes a state; then a card per stage with its state, its attempts linking to their tasks, each attempt's inputs ("started from") and the attempts it fed, and its output collapsed as Markdown. It refetches on any stored `task.*` or `session.*` event of a task in the run, and on a `task.created` whose payload carries the run, so a new stage or fork shows up. diff --git a/internal/agents/process.go b/internal/agents/process.go index 7607f7d..b19c0bc 100644 --- a/internal/agents/process.go +++ b/internal/agents/process.go @@ -55,18 +55,13 @@ type claudeProcess struct { func (m *Manager) startClaude(ctx context.Context, sess api.AgentSession, l *live, g string, servers []*mcp.Server, stdio []mcp.Stdio, env []string) (agentProcess, error) { hooks := map[string][]claude.HookMatcher{ "UserPromptSubmit": {{Func: remindGuidance(g)}}, - "Stop": {{Func: m.bounceProseQuestion(l)}}, - "PostToolUse": {{Func: m.afterTool(l)}}, } - if l.owner != "" { - hooks = map[string][]claude.HookMatcher{ - "UserPromptSubmit": {{Func: remindGuidance(g)}}, - "PostToolUse": {{Func: m.afterTool(l)}}, + if l.reviewID == "" { + hooks["PostToolUse"] = []claude.HookMatcher{{Func: m.afterTool(l)}} + if l.owner == "" { + hooks["Stop"] = []claude.HookMatcher{{Func: m.bounceProseQuestion(l)}} } } - if l.reviewID != "" { - hooks = map[string][]claude.HookMatcher{"UserPromptSubmit": {{Func: remindGuidance(g)}}} - } opts := claude.Options{ Cwd: l.dir, diff --git a/internal/agents/workflows.go b/internal/agents/workflows.go index 863ab4f..0e2f7b3 100644 --- a/internal/agents/workflows.go +++ b/internal/agents/workflows.go @@ -234,11 +234,7 @@ func (m *Manager) StartWorkflow(ctx context.Context, project api.Project, creato run := store.NewID() tasks := []api.Task{} for _, stage := range workflow.Starts(frozen.Definition) { - from := base - if workflow.FromMain(frozen.Definition, stage) { - from = project.DefaultBranch - } - task, err := m.startStage(ctx, project, creator, frozen, run, stage, from, todoItem, nil, actor) + task, err := m.startStage(ctx, project, creator, frozen, run, stage, base, todoItem, nil, actor) if task.Id != "" { tasks = append(tasks, task) } @@ -312,7 +308,7 @@ func stageMessage(frozen store.FrozenWorkflow, stage string, parents []parentSta } switch { case len(parents) == 0: - case workflow.FromMain(frozen.Definition, stage): + case len(parents) > 1: fmt.Fprintf(b, "\n\nThis task starts from %s. The branch of each stage above holds that stage's work: `git merge` the ones this stage needs, or read them with `git show` and `git diff`.", defaultBranch) default: fmt.Fprintf(b, "\n\nThis task starts from the commit stage %q finished at, so its work is already in this worktree.", *parents[0].task.WorkflowStage) @@ -485,7 +481,7 @@ func (m *Manager) advanceTo(ctx context.Context, from api.Task, frozen store.Fro base := parents[0].commit switch { - case !workflow.FromMain(frozen.Definition, stage): + case len(parents) == 1: case frozen.Audit != "": base = frozen.Audit default: diff --git a/internal/workflow/workflow.go b/internal/workflow/workflow.go index 84d05c3..0ae5316 100644 --- a/internal/workflow/workflow.go +++ b/internal/workflow/workflow.go @@ -214,7 +214,3 @@ func Starts(def api.WorkflowDefinition) []string { } return starts } - -func FromMain(def api.WorkflowDefinition, name string) bool { - return len(After(def, name)) > 1 -}