Skip to content

feat: show confirmed AO actions in chat - #6507

Open
harshitsinghbhandari wants to merge 12 commits into
mainfrom
ao/agent-orchestrator-387/chat-action-activities
Open

harshitsinghbhandari wants to merge 12 commits into
mainfrom
ao/agent-orchestrator-387/chat-action-activities

Conversation

@harshitsinghbhandari

@harshitsinghbhandari harshitsinghbhandari commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

Code changes: +1061 -50
Tests: +867 -37
Others: +2 -1 (generated OpenAPI and frontend schema)

Summary

Chat records daemon-confirmed AO session mutations as ao_action activities. The collapsed label is a short verb. Expansion holds the target, prior value, link, and status. Replay of the same operation settles one row. A write failure does not change the mutation result.

Desktop Archive calls POST /api/v1/sessions/{id}/kill. Kanban archive is derived from isTerminated. The daemon emits session.terminated for that kill. There is no separate archive mutation.

There is no daemon pull-request creation path (CreatePullRequest does not exist). pull_request.created is renderer vocabulary only. A shell ao spawn or an MCP create_pull_request stays an ordinary command or tool activity. This PR does not claim end-to-end capture of pull request creation.

Changes

  • Kind ao_action with detail.action (no detail.event, so existing run batching stays intact). Actions: session.spawned, session.terminated, session.renamed, session.restored, session.agent_switched, pull_request.claimed, plus render-only pull_request.created.
  • Stable providerItemId ao-action:{action}:{operationID}. Spawn success uses the child session id and is written on the parent conversation when ParentSessionID is set. Failures and cancellations use the pre-wrap error so context.Canceled stays cancelled.
  • Rename skips an unchanged name and does not record the orchestrator-rename refusal. Claim keeps https hrefs and drops javascript: and other non-allowlisted links. Text is capped at 160 runes.
  • Frontend groups ao_action:{action}, counts repeats, and renders an accessible expandable row with session links. Labels are in all eight catalogs. Mobile ActivityKind includes ao_action.

Regression coverage

  • Session package: parent attribution, replay dedup, cancel-then-fail settle, kill generation dedup, cancelled RequestKill, rename previous/new and same-name skip, restore dedup, switch revision, claim href allowlist.
  • Frontend vitest: activity-action, ActivityRun, ActivityRun.action, ChatTimelineItems (37 tests).
  • go test ./internal/service/session passed with task AO environment variables unset.
  • Pinned golangci-lint v2.13.2 (--path-mode=abs) reported 0 issues.

Validation gaps

These failed in go test ./... against this machine and are outside this diff:

  • opencodev2 TestV2RejectsFailuresWithoutSideEffects/malformed_version: local ~/.opencode/bin/opencode reports OpenCode 1.18.32, and the test expects the word "version".
  • codexappserver TestGeneratedProtocolMatchesTheInstalledProvider: installed provider method set does not match the generated protocol (29 extra installed methods, generated thread/rollback missing from the install).
  • cli TestBrowserRequiresSessionAndValidWait: capability error Unknown session (SESSION_NOT_FOUND) against a live daemon.

go test -race ./... was not run. CLI e2e (go test -tags e2e) was not run. Frontend tsc --noEmit passed before the main merge; the merge and the lint commit did not change TypeScript.

Rollout

Ships with the desktop app and daemon together. Existing conversations gain activities only for mutations after this build. No migration.

Omissions

  • No daemon emitter for pull_request.created.
  • No actor HTTP header and no success inferred from command or tool prose.
  • No merge and no release dispatch.

@github-actions

Copy link
Copy Markdown
Contributor

🏆 Review leaderboard

Oct 3–10, 2026 · UTC

Rank Reviewer PRs reviewed Review rounds PR comments
🥇 @illegalcall @illegalcall 28 47 10
🥈 @ronishrohan @ronishrohan 20 23 17
🥉 @Vaibhaav-Tiwari @Vaibhaav-Tiwari 16 20 11
4 @Prasad-D-Ware @Prasad-D-Ware 10 27 1
5 @nikhilachale @nikhilachale 8 20 3
6 @axisrow @axisrow 8 10 48
7 @Rishet11 @Rishet11 7 16 2
8 @mohakchakraborty2004 @mohakchakraborty2004 7 7 1
9 @Pritom14 @Pritom14 5 5 3
10 @codebanditssss @codebanditssss 4 4 2

Ranked by distinct external PRs reviewed, then review rounds, then PR comments. Self-activity and bot activity are excluded.

Show 6 more reviewers
Rank Reviewer PRs reviewed Review rounds PR comments
11 @Annieeeee11 @Annieeeee11 3 6 1
12 @harshitsinghbhandari @harshitsinghbhandari 2 4 0
13 @neversettle17-101 @neversettle17-101 2 3 0
14 @whoisasx @whoisasx 2 2 0
15 @AgentWrapper @AgentWrapper 1 3 0
16 @ApexYash11 @ApexYash11 1 2 0

@i-trytoohard i-trytoohard added this to the Desktop & UI milestone Oct 10, 2026
@i-trytoohard i-trytoohard added comp/daemon Go daemon, process lifecycle, and backend control plane. enhancement New feature or request labels Oct 10, 2026

@illegalcall illegalcall left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Requesting changes for three verified defects in the new action history: SQLite rejects the new activity kind, asynchronous agent switches never settle their activity, and repeated rename transitions overwrite earlier history.

Verification: the existing session service suite and the real manager's TestSwitchAgentReturnsAfterDurableAdmission pass. A temporary regression against the fully migrated SQLite store fails with the kind CHECK constraint; service regressions reproduce the stuck switch status and the missing third rename. Full local CI and a live desktop run were not repeated; the current remote checks are green.

Comment thread backend/internal/service/session/action_activity.go
Comment thread backend/internal/service/session/agent_switching.go
Comment thread backend/internal/service/session/action_activity.go Outdated

@illegalcall illegalcall left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-reviewed the three original findings at 9e119b2.

  • Fixed: SQLite accepts ao_action. TestMigration0195AllowsAOActionAndKeepsActivitySchema and TestUpsertActivityPersistsAOActionOnMigratedStore pass.
  • Still needs a fix: a terminal switch callback can be overwritten by the later admission write, leaving the activity Running again.
  • Still needs a fix: the process-local rename counter resets on daemon restart and reuses an earlier operation ID.

The new happy-path switch/rename tests and five focused real-manager tests pass. Temporary regressions against the real migrated SQLite store reproduce the restart collision and the callback-before-admission ordering for both completed and failed switches. This re-review is scoped to the three original findings; live desktop UI and full local CI were not repeated.

Comment thread backend/internal/service/session/agent_switching.go
Comment thread backend/internal/service/session/action_activity.go Outdated
Record daemon session and pull request claim outcomes as ao_action activities with a stable operation id, and render them as short batched labels.
Drop the unused session lookup result and group test imports with the local prefix.
SQLite still rejected the new kind, so action writes never landed. Widen the check without rebuilding rows, indexes, or CDC triggers.
A durable terminal switch updates the activity admitted as running, including later recovery of the same switch. Each successful rename gets its own operation id, while an unchanged name still writes nothing.
The pending approval is still the only provider activity. The new session.spawned row is recorded beside it.
The worker can finish before SwitchAgent records admission. A running write now leaves an existing completed, failed, or cancelled row in place. Migration 0195 is recorded in the shipped version ledger.
A process-local counter reused provider item IDs after the daemon started again. Successful renames now use a UUID, and an unchanged name still writes nothing.
@harshitsinghbhandari
harshitsinghbhandari force-pushed the ao/agent-orchestrator-387/chat-action-activities branch from 6118462 to 6d98a92 Compare October 12, 2026 10:01
@illegalcall

Copy link
Copy Markdown
Collaborator

Adil is already working on the agent-switch issue.

We ran the rename sequence live with a real Codex agent on main at fca1b72c: Reviewer → Ship → Reviewer → full app/daemon restart → Ship. The name and all three agent replies persisted.

The rename issue did not reproduce on that build. This does not confirm a fix in this PR: #6507 is still unmerged, and main does not contain its ao_action history feature. We did not run the affected PR commit live.

Main claimed migration 0196, so the ao_action kind check moves to 0197. Orchestrator rename refusal and the action activity behavior stay.

@illegalcall illegalcall left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Retested the latest PR head, 13d4391.

Verified fixed:

  • SQLite accepts ao_action: the migration 0197 schema test and real-store persistence test pass.
  • Rename history survives restart: the real-store regression passes. I also ran the actual PR desktop build with a real Codex agent through Reviewer -> Ship -> Reviewer -> full app/daemon restart -> Ship. All three completed rename activities remain, with distinct operation IDs and the correct order. This live test used the PR build, rather than the earlier main-only demonstration.

Still open:

  • The switch guard has a gap between its status read and its activity write. A terminal callback arriving in that gap is still overwritten by Running. The inline comment gives the reproduced ordering.

The original full session-service suite passes (122.446s), as does the PR's callback-before-admission regression. A temporary deterministic regression against the real SQLite store fails for both completed and failed outcomes when the callback runs between the status read and the admission write. I did not reproduce this forced switch timing in the live UI or repeat all local CI jobs.

Comment thread backend/internal/service/session/action_activity.go
The admission status read can finish before the terminal callback writes. The store now decides from the row it just loaded under the writer lock, so a later running upsert leaves a completed or failed switch in place.
The replay can finish with almost none of the five minute context left. The snapshot and provider-event checks now use their own deadline.
@harshitsinghbhandari
harshitsinghbhandari dismissed illegalcall’s stale review October 13, 2026 19:10

The switch read/write gap in this review is fixed on 5925d0e and remains on d0d4300. UpsertActivity refuses a running ao_action when the locked row is already terminal, and TestAdmissionWriteAfterStatusReadDoesNotDowngrade covers that ordering. All review threads are resolved.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/daemon Go daemon, process lifecycle, and backend control plane. enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants