diff --git a/lib/apps/fabro-cli/tests/it/cmd/attach.rs b/lib/apps/fabro-cli/tests/it/cmd/attach.rs index 54f9ecd15..0475dd88c 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/attach.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/attach.rs @@ -1531,40 +1531,6 @@ fn attach_json_errors_without_prompting_for_human_input() { { "run_id": "[ULID]", "stream_seq": 21, - "kind": "platform", - "id": "[EVENT_ID]", - "recorded_at": "[EPOCH_MS]", - "item": { - "seq": 7, - "recorded_at": "[EPOCH_MS]", - "record": { - "kind": "run.branch", - "run_branch": "fabro/run/[ULID]", - "base_sha": "[DIGEST]", - "workspace": "invocation-0-scope-0" - } - } - }, - { - "run_id": "[ULID]", - "stream_seq": 22, - "kind": "platform", - "id": "[EVENT_ID]", - "recorded_at": "[EPOCH_MS]", - "item": { - "seq": 8, - "recorded_at": "[EPOCH_MS]", - "record": { - "kind": "git.identity", - "name": "Fabro", - "email": "noreply@fabro.sh", - "source": "default" - } - } - }, - { - "run_id": "[ULID]", - "stream_seq": 23, "kind": "petri", "id": "execution 0/7/0", "recorded_at": "[EPOCH_MS]", @@ -1656,7 +1622,7 @@ fn attach_json_errors_without_prompting_for_human_input() { }, { "run_id": "[ULID]", - "stream_seq": 24, + "stream_seq": 22, "kind": "petri", "id": "execution 0/8/0", "recorded_at": "[EPOCH_MS]", @@ -1736,7 +1702,7 @@ fn attach_json_errors_without_prompting_for_human_input() { }, { "run_id": "[ULID]", - "stream_seq": 25, + "stream_seq": 23, "kind": "petri", "id": "execution 0/8/1", "recorded_at": "[EPOCH_MS]", @@ -1805,6 +1771,48 @@ fn attach_json_errors_without_prompting_for_human_input() { } } }, + { + "run_id": "[ULID]", + "stream_seq": 24, + "kind": "platform", + "id": "[EVENT_ID]", + "recorded_at": "[EPOCH_MS]", + "item": { + "seq": 7, + "recorded_at": "[EPOCH_MS]", + "record": { + "kind": "run.branch", + "run_branch": "fabro/run/[ULID]", + "base_sha": "[DIGEST]", + "workspace": "invocation-0-scope-0" + }, + "position": { + "execution": 0, + "firing": 1 + } + } + }, + { + "run_id": "[ULID]", + "stream_seq": 25, + "kind": "platform", + "id": "[EVENT_ID]", + "recorded_at": "[EPOCH_MS]", + "item": { + "seq": 8, + "recorded_at": "[EPOCH_MS]", + "record": { + "kind": "git.identity", + "name": "Fabro", + "email": "noreply@fabro.sh", + "source": "default" + }, + "position": { + "execution": 0, + "firing": 1 + } + } + }, { "run_id": "[ULID]", "stream_seq": 26, diff --git a/lib/apps/fabro-cli/tests/it/scenario/petri.rs b/lib/apps/fabro-cli/tests/it/scenario/petri.rs index d6a2be790..1559058e7 100644 --- a/lib/apps/fabro-cli/tests/it/scenario/petri.rs +++ b/lib/apps/fabro-cli/tests/it/scenario/petri.rs @@ -1185,9 +1185,9 @@ async fn a_finished_petri_run_reads_back_through_the_cli() { [CLOCK] Engine: petri run started [CLOCK] ▶ start [CLOCK] │ checkout: [TEMP_DIR]/petri-workspace is not a Git repository; the workspace starts empty + [CLOCK] ✓ start [DURATION] [CLOCK] Branch: fabro/run/[ULID] from [SHA] [CLOCK] Git identity: Fabro default - [CLOCK] ✓ start [DURATION] [CLOCK] ⎘ Checkpoint [SHA] [CLOCK] ▶ say [CLOCK] start → say continue diff --git a/lib/components/fabro-petri/README.md b/lib/components/fabro-petri/README.md index 62029985c..2ab442f59 100644 --- a/lib/components/fabro-petri/README.md +++ b/lib/components/fabro-petri/README.md @@ -101,7 +101,8 @@ Every adapter the integration plan describes lands here. `hooks` writes the platform records Petri cannot: `run.branch` and `git.identity` when the first checkpoint creates the run branch (the base commit is the workspace's `HEAD` before the branch, or that first commit in -a workspace with no history), `checkpoint` after every route with the +a workspace with no history; both carry that checkpoint's stage position, so +the stream places them with its finish), `checkpoint` after every route with the stage's diff from its parent commit (`diff_summary`, and the patch as a text blob under `patch_blob`), `artifact.collected` for every file under `[run.artifacts] include` a stage left in its workspace (the bytes go to diff --git a/lib/components/fabro-petri/VIEWS.md b/lib/components/fabro-petri/VIEWS.md index 4411f6318..e3cd96624 100644 --- a/lib/components/fabro-petri/VIEWS.md +++ b/lib/components/fabro-petri/VIEWS.md @@ -75,8 +75,8 @@ toasts. `RunProjection` (`GET /runs/{id}/state`) serves `attach`, `inspect`, | stages summary | `Conclusion.stages` | derived from the Stages section | stage | | diff | `Run.diff`, `Conclusion.diff`, `Checkpoint`'s diff | platform record `checkpoint {diff_summary, patch_blob}`; the final one is the run's | stage | | final commit | `Conclusion.final_git_commit_sha` | the last platform record `checkpoint {git_commit_sha}` | stage | -| run branch, base sha | `StartRecord.run_branch`, `base_sha` | platform record `run.branch {run_branch, base_sha}` | run | -| Git identity | `RunProjection.git_identity` | platform record `git.identity {name, email, source}` | run | +| run branch, base sha | `StartRecord.run_branch`, `base_sha` | platform record `run.branch {run_branch, base_sha}`, positioned on the checkpoint that created the branch | run | +| Git identity | `RunProjection.git_identity` | platform record `git.identity {name, email, source}`, positioned with `run.branch` | run | | pull request | `Run.pull_request`, `RunProjection.pull_request`, `pull_request_creation` | see Platform | run | | current question | `Run.current_question` | see Questions | question | | sandbox | `Run.sandbox`, `RunProjection.sandbox` | see Sandbox | invocation | @@ -428,7 +428,7 @@ record where Fabro does. | tools available to an agent | `agent_tools`, the insights sidebar's tool list | a `custom attractor.tools {node, firing, attempt, session, tools[] {name, description, source, category}}` from the native backend once per session, where it calls the `HostTools` builders; Pebble's `SessionStarted` carries only the provider and model | | question option `description` and `preview`, `context_display` | the interview dock, the human Q&A renderer | optional fields on Petri's `QuestionOption` (`description`, `preview`) and `Question` (`context`), set by the human gate from the edge attributes Fabro's lowering already reads | | who answered | `interview.completed` `actor`, Slack attribution | platform record `interview.answered {question, principal, channel}` written by Fabro's interviewer beside its `InterviewReply` | -| run branch and base sha | `StartRecord`, `run diff`, the commits picker | platform record `run.branch {run_branch, base_sha}` written when Fabro creates the run branch | +| run branch and base sha | `StartRecord`, `run diff`, the commits picker | platform record `run.branch {run_branch, base_sha}` written when Fabro creates the run branch, at that checkpoint's stage position | | Git identity | `git_identity` | platform record `git.identity {name, email, source}` | | diff summary and patch per checkpoint | `Run.diff`, `Conclusion.diff`, `StageProjection.diff`, the changes sort | `diff_summary` and `patch_blob` on the `checkpoint` platform record | | lifecycle before the engine, archive, title, parent, supersede, notices | the run list, header, `runs ps`, `run events --pretty` | platform records `run.created`, `run.lifecycle`, `run.archived`, `run.unarchived`, `run.title`, `run.parent`, `run.superseded`, `run.notice` | diff --git a/lib/components/fabro-petri/src/hooks.rs b/lib/components/fabro-petri/src/hooks.rs index 2688571ff..e1f2288ad 100644 --- a/lib/components/fabro-petri/src/hooks.rs +++ b/lib/components/fabro-petri/src/hooks.rs @@ -19,7 +19,8 @@ //! routes, so no route is taken. The commit that creates the run branch also //! records where it started: the `run.branch` platform record (the branch //! name and the base commit) and the `git.identity` record (who authors the -//! commits, and where that identity came from). +//! commits, and where that identity came from), both at that checkpoint's +//! stage position, so the stream orders them with the firing's finish. //! - `transition`: the platform checkpoint record, keyed on the Petri position //! and the checkpoint's operation identity, with the stage's diff from its //! parent commit (`diff_summary`, and the patch as a blob); then the stage's @@ -539,7 +540,7 @@ impl FabroHooks { .base_sha .clone() .unwrap_or_else(|| snapshot.sha.clone()); - if let Err(error) = self.record_branch(workspace, base_sha).await { + if let Err(error) = self.record_branch(key, workspace, base_sha).await { warn!(run_id = %self.run_id, error = %error, "the run branch was not recorded"); } } @@ -547,8 +548,21 @@ impl FabroHooks { /// The `run.branch` and `git.identity` records, once per run: the first /// workspace to create the run branch names where it started. A run /// that already recorded its branch (a resume, or a nested workspace - /// after the root's) records nothing. - async fn record_branch(&self, workspace: &str, base_sha: String) -> Result<(), String> { + /// after the root's) records nothing. Both records take the position of + /// the checkpoint that created the branch, so the stream places them + /// with that firing (after its finish, before its routes) rather than by + /// the clock, which would put them on either side of the finish from + /// one run to the next. + async fn record_branch( + &self, + key: CheckpointKey, + workspace: &str, + base_sha: String, + ) -> Result<(), String> { + let position = StagePosition { + execution: key.execution, + firing: key.firing, + }; let branch = self .branch .get_or_try_init(|| async { @@ -564,7 +578,7 @@ impl FabroHooks { .append( &self.run_id, &PlatformRecord::RunBranch(record.clone()), - None, + Some(position), ) .await .map_err(|error| { @@ -577,7 +591,7 @@ impl FabroHooks { identity: self.identity.clone(), }); self.records - .append(&self.run_id, &identity, None) + .append(&self.run_id, &identity, Some(position)) .await .map_err(|error| { format!(