Skip to content

Support Atem r8 agent control, approvals and local chat APIs - #56

Merged
guohai merged 12 commits into
mainfrom
feat/agent-control-api
Oct 18, 2026
Merged

guohai merged 12 commits into
mainfrom
feat/agent-control-api

Conversation

@guohai

@guohai guohai commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Astation must connect to the deployed Atem r8 host and operate launched agents through native UI and its protected local API. This changes every hello to revision 8, pins the merged Atem contract at 4f87bab1289848cb6a0da5707d2e7830eb5c057d (#78), and confirms all four implementation contract edits. Older hosts are hidden from instance/session selection with an upgrade notice; there is no fallback.

  • Prompt Send atomically acquires an unowned session. A held session offers confirmed takeover bound to the displayed controller/epoch; durable receipts and recovery prevent replay or late replies from restoring control.
  • Approval answers and rule revocation need control permission without taking the terminal lease. Claude and Codex Always allow echo exact host rules; stale cards close authoritatively.
  • One ordered chat service handles whole-message updates, history boundaries, bounded pages/lookups, result coverage, gaps and retention for native approval activity and the API.
  • The API adds chat read/history/page and explicit lease renewal. Native and API operators share the session client and journal with separate local lease ownership; abandoned API control expires rather than renewing forever.
  • Preserve main’s About/version metadata; remove the redundant AI review document and keep useful guidance in the workflow.

Validation: the personally provisioned build passed 941 Swift tests (6 environment-dependent skips, 0 failures), including all 7 upstream client scenarios through the real decoder/client. Release build, 65 automation tests, 18 packaging tests, signature/entitlement checks and bundled-resource validation passed. Host fixtures are pinned for provenance; Swift does not claim to execute Atem’s host journal tests.

Live verification on 2026-10-18: relaunched the personally provisioned Astation Local app, authenticated to Genie, negotiated all 9 r8 features, discovered running Claude and Codex sessions, and observed explicit complete history_end boundaries through the protected API. Both histories were empty. Read-only history lookups for unknown request IDs completed through chat/history and chat/page with explicit unavailable coverage on both sessions. Neither reads nor lookups acquired control. No prompts, terminal input, interrupts, stops or approval decisions were sent to working agents. Nonempty live pagination and the disposable-provider approval/mutation matrix remain separate validation; deterministic tests cover those client paths. Polling is implemented; SSE replay, a full native conversation UI and offline APNs delivery remain separate work.

Review follow-up: local operator_id values are cooperative ownership labels under one bearer authority, not distinct authenticated principals; docs/local-control-api.md states that boundary. testLargeFinalResultMetadataCannotBlockOrEvictUnconfirmedRequests exceeds 4 MiB with mixed final and unconfirmed records and asserts the persisted limit and survival of unconfirmed work. Failed chat delivery remains an explicit page error available by polling; HTTP 202 means asynchronous admission, not host delivery. Invalid chat messages close the subscription rather than being retried at the same sequence.

Merged as 4f4581ac7c3e453a39a830dc6c4577aeb71dda6a. The final review found no critical issues; three P2 follow-ups (owner-scoped pending cancellation, overlapping history ordering, and definitive refusal eviction) are tracked in #57.

Current lifecycle and security invariants: designs/protocol/r8-client.md. Callable API: docs/local-control-api.md. No Atem checkout was edited or built.

Generated with SMT smt@agora.build

guohai added 12 commits October 17, 2026 02:21
Define the first complete agent-control API milestone and the joint Atem/Astation history, recovery, approval and notification contract.

🤖 Built with SMT <smt@agora.build>
Correct Codex chat and session-rule support, record remaining r8 agreement points, and distinguish current Astation support from the proposed API.

🤖 Built with SMT <smt@agora.build>
Close the four r1 contract findings, record two draft fixture corrections, and preserve the conformance gate. Track the planned r7.3 permission-only session-rule revocation separately.

🤖 Built with SMT <smt@agora.build>
Preserve the completed protocol reviews and the approved chat-send and lease-free approval design while keeping implementation and live verification gates explicit.

🤖 Built with SMT <smt@agora.build>
🤖 Built with SMT <smt@agora.build>
Preserve the native approval inbox, shared chat wire models, protected local API and pinned r7.3 fixture work. Resolve the guide against the implemented routes while retaining the separate r8 implementation gate.

🤖 Built with SMT <smt@agora.build>
Remove the standalone review guide and retain concise operational comments beside the workflow settings.

🤖 Built with SMT <smt@agora.build>
🤖 Built with SMT <smt@agora.build>
Record PR #56 and the combined Swift validation result, and distinguish implemented r7.3 approval support from the remaining r8 and live acceptance work.

🤖 Built with SMT <smt@agora.build>
Confirm all four contract edits at 10e089a, record static fixture checks and remaining runtime gates, and refresh the active session/API handoffs.

🤖 Built with SMT <smt@agora.build>
Pin the contract and fixtures to Atem main 4f87bab. Negotiate revision 8
and known features, hide incompatible hosts, add atomic Send with confirmed
takeover and owner-scoped leases, and answer approvals without a lease.
Share bounded ordered history and result projections between native
approval activity and the protected local API. Preserve durable receipt
recovery and fence delayed control replies after release or replacement.

Validate all seven published client scenarios, API workflows and resource
bounds. Document the four agreed contract edits and the current API.

🤖 Built with SMT <smt@agora.build>
🤖 Built with SMT <smt@agora.build>
@guohai guohai changed the title Consolidate agent approval rules and protocol handoff Support Atem r8 agent control, approvals and local chat APIs Oct 18, 2026
@guohai
guohai marked this pull request as ready for review October 18, 2026 22:59
@github-actions

Copy link
Copy Markdown

Claude Code Review

Code Review: PR #56 - Atem r8 Agent Control

Summary

Large, generally well-structured PR adding r8 protocol support, chat APIs, and approval/rule management. The lease-ownership and receipt-recovery logic is carefully designed with extensive fixture coverage. Below are concrete findings.


Findings

1. AgentChatPart.isValid — name/summary truthiness bug for tool kind

File: Sources/Menubar/AgentChatWire.swift

case "tool": return markdown == nil && name != nil && summary != nil

This validates presence but not content bounds for name/summary on tool parts, while contentBytes/historyBytes count them. Not necessarily a vulnerability, but note that the approval case uses explicit .text(..., minimum:, maximum:) checks while tool does not bound its string lengths. Since total content is capped downstream at 131,072 bytes in AtemSessionChat.receive, this is mitigated but a single oversized tool part is accepted before the aggregate check — worth bounding individually for defense-in-depth.

2. AtemSessionChat.receive — invalid message closes the view after appending state

File: Sources/Menubar/AtemSessionChat.swift

view.snapshot.sequence = sequence
if !valid { close(view.snapshot); view.snapshot.closed = true; view.snapshot.error = "invalid_chat_message" }

When valid == false, view.snapshot.sequence = sequence is committed before the invalid check. In several message/history_end branches that set valid = false via break, partial mutations to view (e.g. view.replies[pageID]?.messages.append, view.pageBytes[pageID]..., view.initial.append) have already occurred on the local view copy. Since views[event.agentId] = view runs unconditionally at the end even on the invalid path, these partial/poisoned mutations are persisted into a view that is then marked closed — mostly harmless because it's closed, but the sequence advance means a legitimate retry at that sequence would be rejected. Confirm this is intended (the fixtures r8-client-shows-truncation-and-resyncs-only-on-a-sequence-gap may cover it, but the invalid-then-persist ordering is subtle).

3. history early-return leaks a journal/reply slot on send failure — minor

File: Sources/Menubar/AtemSessionChat.swift

views[target.session_id] = view   // reply + pageID recorded
...
if !send(event) {
    views[target.session_id]?.replies[pageID]?.pending = false
    views[target.session_id]?.replies[pageID]?.error = "delivery_uncertain"
}
return pageID

On send failure the page is retained as a non-pending errored reply and still counts toward historyTimes (rate limit) and pageIDs. This is acceptable, but the caller receives a pageID for a page that will never be populated — the API returns .accepted with status: "requested" which misrepresents an uncertain delivery. Consider surfacing delivery_uncertain to the API caller.

4. AgentChatTimestamp.date allocates ISO8601DateFormatter per call

File: Sources/Menubar/AgentChatWire.swift

let formatter = ISO8601DateFormatter()
formatter.formatOptions = [.withInternetDateTime, .withFractionalSeconds]
return formatter.date(from: text) ?? ISO8601DateFormatter().date(from: text)

ISO8601DateFormatter is expensive to construct and this is called in validation hot paths (isValid, activities, status checks) that run per chat message. Under high message volume this is a measurable performance cost. Cache the formatters.

5. receiveChat triggers refresh() on generation mismatch — potential resync amplification

File: Sources/Menubar/AtemSessionClient.swift

if let generation = event.generation, generation != descriptor.process_gen,
   let target = ..., chat.snapshot(target)?.chat_id == event.conversationId {
    refresh()
    changed()
    return
}

A stream of stale-generation chat events (e.g. during a process replacement) each call refresh(). refresh() guards against duplicate list ops in pending, so this is bounded, but every event still calls changed() and re-enters. No hard bug, but worth confirming the host cannot flood these to force repeated registry resyncs.

6. Removal of provider == "claude" gate on Always-allow rules

File: Sources/Menubar/ApprovalInbox.swift / AtemSessionClient.swift

// was: entry.provider == "claude", !entry.request.always_rules.isEmpty
if !entry.request.always_rules.isEmpty {

and

// was: registry[target.session_id]?.provider == "claude", approval.always_rules.contains...
guard approval.always_rules.contains(where: { $0.rule_id == ruleID }) else { return nil }

This intentionally generalizes Always-allow beyond Claude to Codex (per the PR description). The security property now rests entirely on the host echoing exact rules in always_rules and the client never synthesizing them — which the code honors. This is correct provided always_rules is always host-sourced. Confirmed: AgentChatPart.isValid and AtemApprovalRequest validate rules from the wire, so no client-side fabrication. No issue, flagging for reviewer awareness of the trust-boundary shift.

7. API lease owner derived from caller-supplied operator_id

File: Sources/Menubar/LocalControlAPI.swift

let owner = "api:" + (operation.operator_id ?? "local-api")

Any local API caller can choose an arbitrary operator_id, meaning one API client can claim/release/renew another's lease by reusing its operator_id. Since the API is described as "protected local," this may be acceptable, but there is no authentication binding operator_id to a caller identity. If two cooperating-but-distrusting local operators are a threat model, this allows cross-operator lease hijacking (control-not-owned is bypassed by spoofing the owner string). Document this assumption or bind ownership to the connection.

8. /api/v1/chat/history returns .accepted even on send-uncertain

As noted in #3 — the endpoint always returns 202 accepted once a pageID exists, with no way for the caller to distinguish a truly requested page from a delivery_uncertain one until they poll /chat/page. Minor API-contract clarity issue.

9. Missing test coverage for AtemSessionJournal byte-eviction path

File: Sources/Menubar/AtemSessionJournal.swift

var bytes = try JSONEncoder().encode(next).count
while bytes > 4_194_304 {
    guard let index = next.requests.firstIndex(where: { $0.receipt?.state.final == true }) else {
        throw AtemSessionJSON.Invalid.limit
    }
    bytes -= try JSONEncoder().encode(next.requests[index]).count + 1
    next.requests.remove(at: index)
}

The - 1 (assumed separator/comma accounting) is an approximation that does not reflect actual JSON re-encoding size after removal; the loop can over- or under-estimate and, in the worst case, not converge below the limit in one pass if all remaining records are non-final, throwing Invalid.limit and blocking durable submission. No fixture in the listed additions clearly exercises the 4 MB byte-cap eviction (the count-cap at 4096 is separate). Recommend a test that drives a journal past 4 MB with mixed final/non-final records.


Non-blocking observations

  • AtemChatPage.valid requires end == (next == nil), a good invariant.
  • acceptReceipt now preserves prior result when a final receipt arrives without one and enforces sameOutcome — solid replay protection.
  • The refusal?.code == "lease_held" gate on resend (submitDurably) correctly restricts replay to explicit takeover retries.
  • Workflow change is comment-only; no security impact.

No critical vulnerabilities found. Findings #7 (owner spoofing) and #9 (journal eviction convergence) are the most worth addressing before merge.

@github-actions

Copy link
Copy Markdown

Codex Code Review

  • [P2] Scope pending cancellation to the releasing operator — Sources/Menubar/AtemSessionClient.swift:227
    When an acquire is pending, controlOwner(for:) is still nil, so another API operator's release passes both ownership checks. This filter then removes every pending acquire/renew for the target, regardless of operation.owner. The original acquire can still succeed on Atem, but its reply is ignored because its pending entry is gone, leaving the requesting operator without a usable local lease. Filter cancellation by owner as well as target, matching the owner-scoped handling of pending prompt receipts.

  • [P2] Preserve timeline position when merging history pages — Sources/Menubar/AtemSessionChat.swift:259
    Every before page is moved to the beginning of the timeline, even when it overlaps the middle of already-loaded history. For example, with messages 1–100 retained, a 50-message page before message 100 produces the order 50–99, 1–49, 100. Because snapshot() returns the last 50 entries, it then exposes messages 1–49 and 100 instead of the actual newest 50. Merge pages at their requested boundary rather than unconditionally prepending them; add coverage for overlapping or repeated before requests.

  • [P2] Allow definitive admission refusals to age out of the journal — Sources/Menubar/AtemSessionClient.swift:408
    Journal eviction only removes records with a final receipt, but lease_held refusals are stored without a receipt and explicitly excluded from receipt recovery. Those records therefore remain permanently ineligible for eviction. After 4,096 distinct refused sends accumulate, appending any new request exceeds the journal limit and all subsequent durable submissions fail, including after reconnecting or restarting. Definitively refused records need a bounded retention/eviction policy alongside final receipts.

@guohai
guohai merged commit 4f4581a into main Oct 18, 2026
4 checks passed
@guohai
guohai deleted the feat/agent-control-api branch October 18, 2026 23:20
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.

1 participant