Skip to content

refactor: Use receiver methods - #76

Open
moshloop wants to merge 23 commits into
mainfrom
refactor/receiver-method-calls
Open

refactor: Use receiver methods#76
moshloop wants to merge 23 commits into
mainfrom
refactor/receiver-method-calls

Conversation

@moshloop

@moshloop moshloop commented Aug 23, 2026

Copy link
Copy Markdown
Member

What

  • Replace field access with receiver method calls in API tooling and Command Palette tests.

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of dynamic command execution and patch application in Codex-powered workflows.
    • Command and file-edit activity is now reported in the correct order before the corresponding successful result.
    • Multiple tool events from a single notification are now processed reliably.
    • Transformed tool inputs preserve their adjusted arguments when executed.

…port

Introduce durable token enrollment, HTTPS mailbox and sidecar transport, and hardened Docker/Kubernetes deployment with address preflight and lifecycle controls.
Persist remote task history and expose sandbox catalogs, credential publishing, deployment management, and runtime inspection through the API and web UI.
BREAKING CHANGE: replace single-use git-agent join-token APIs and replay semantics with durable captain tokens; callers must use AdmitToken and context-aware enrollment.
…ions

Add opt-in mirroring for Claude and Codex subscription logins with refresh-token redaction and expiry tracking.
Publish credentials atomically to host directories or Kubernetes Secrets, refresh them from `serve`, and expose local-only status and sync controls.
Wire acquired logins through sandbox isolation while centralizing token-provider selection and resolving model credentials from the Captain vault.
…n access

Protect off-host API and git-agent requests with durable, revocable bearer tokens while preserving loopback access for the local UI and CLI.
Add TLS certificate management and local-only command registration to prevent host-administering commands from being exposed through the REST executor.
Persist hashed credentials with scoped, bound or pooled identities, expiry, revocation, and usage tracking.

BREAKING CHANGE: Non-loopback requests to /api/v1 and /git now require a valid scoped Captain bearer token provisioned with `captain token create`.
Allow non-streaming workflow providers to satisfy the runner's event contract via buffered execution instead of failing. Add coverage for completed text and result events.
Every other configurable axis — model, effort, cliArgs, sandbox — is declared
in the registry, served to clients, and guarded server-side. Permissions had
none of that: any posture or per-tool policy could be written for any backend,
and the mismatch surfaced only when a provider built argv, minutes into a run.

Declare what each backend actually does with a permissions block: which
postures it honours (native, approximated, or not at all, plus the argv or
sandbox/approval pair each compiles to), which per-tool policies it can
enforce, and which resources it can switch. Project it through RuntimeModeEntry
and backends[] so clients read it from the static catalog rather than from a
TTL-d probe, where an unprobed backend would appear to support nothing.

Tool-policy capability is keyed by provenance, not by backend alone, because
where a tool came from decides what is enforceable. Captain builds the
caller-tool list itself and omits a denied tool, so deny is honoured on
codex-agent — which has no tool filter of its own — while deny on a codex
built-in is not.

Resources are keyed by the value requested as well as the kind: MCP is only
switchable off and skills only on, so one cell per kind would report
"supported" for a request that is accepted and dropped.

The table states what the code does today, warts included, and the tests prove
it against CodexSafety, the three CLI arg builders, the cmux command builder
and the claude-agent initialize params, so a mapper and its declaration cannot
drift apart in silence.

Claude-Session-Id: c3c29851-27f6-4cce-831d-ca61a67461d4
Tools and MCP carry every field as json:"-" behind hand-written marshalers, so
reflection reported {} for both — the two fields that decide what an agent may
do validated anything at all and told a client nothing. The permission enums
are plain string types, so they reflected to a bare string with no values,
which is why the editor grew its own hardcoded copies that then went stale.

Add invopop describers for PermissionMode, ToolPolicy, ResourceMode, Preset,
Tools, MCP and ResourcePolicies. ResourcePolicies also declares the legacy
string-array form it has always decoded, so a document using it no longer
fails validation against captain's own schema.

pkg/api hosts both invopop and clicky describer signatures and a type given
the wrong one is silently ignored; the rule is the consumer, so anything
reachable from api.Spec uses invopop.

Claude-Session-Id: c3c29851-27f6-4cce-831d-ca61a67461d4
Print the declared capability table so a silent drop is visible before a run is
spent on it. Settings x backends, grouped one table per agent family: eleven
backend names do not fit a terminal, and truncating them collapses claude-cli
and claude-cmux into the same ambiguous header. Grouping also matches the
question people actually ask — I picked claude, which transport honours this?

Approximated cells get their own mark rather than rounding to yes or no; that
conflation is what let dontAsk read as supported on codex, where it resolves to
the read-only default. --notes prints the reason attached to every cell that is
not honoured exactly.

A separate namespace from sandbox presets, which already owns that word for
container presets. Deliberately not excluded from REST/MCP auto-exposure: it
reads a compile-time table and touches no host state. local_only_test pins
POST /api/v1/permissions/matrix as the group's only route so a future mutating
command cannot slip in beside it.

The gavel fixture is the second pinning mechanism: 44 cases over the printed
grid and the JSON, so a change to what captain does with a permissions block
has to arrive as a reviewable diff to the declared truth.

Claude-Session-Id: c3c29851-27f6-4cce-831d-ca61a67461d4
Claude-Session-Id: a317c3d6-1ab9-47ea-8118-4897be0ec96c
Runs that only touch git-ignored paths (scratch dirs) no longer fail over
the caller's unrelated dirty files. committable() filters ignored paths; the
refusal error lists only committable paths, improving diagnostic accuracy.
@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 27ce881d-9803-4fa5-b094-ec501e40c4d7

📥 Commits

Reviewing files that changed from the base of the PR and between 139ac63 and 7431eeb.

📒 Files selected for processing (4)
  • pkg/ai/provider/codex_appserver.go
  • pkg/ai/provider/codex_appserver_lifecycle_ginkgo_test.go
  • pkg/ai/provider/codex_appserver_protocol.go
  • pkg/ai/provider/codex_appserver_test.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

Codex app-server notification mapping now returns event slices. Dynamic tool calls can emit multiple tool-use events. Notification handling processes all mapped events and queues command and edit events before the correlated tool result.

Changes

Codex multi-event mapping

Layer / File(s) Summary
Notification and tool-call mapping
pkg/ai/provider/codex_appserver_protocol.go, pkg/ai/provider/codex_appserver_test.go
Notification and item mapping now return []ai.Event. Dynamic tool inputs preserve transformed arguments. Tests cover empty and single-event results, failures, and usage updates.
Multi-event notification handling
pkg/ai/provider/codex_appserver.go, pkg/ai/provider/codex_appserver_lifecycle_ginkgo_test.go
Notification handling processes all mapped events. Dynamic exec coverage verifies Bash and Edit tool-use events before the correlated successful result.

Sequence Diagram(s)

sequenceDiagram
  participant CodexAppServer
  participant handleNotification
  participant aiEventQueue
  CodexAppServer->>handleNotification: Send notification
  handleNotification->>aiEventQueue: Queue Bash tool-use event
  handleNotification->>aiEventQueue: Queue Edit tool-use event
  handleNotification->>aiEventQueue: Queue correlated successful tool result
Loading

Suggested reviewers: adityathebe

Merge Risk: ⚪ Minimal · up to 7431e

This localized refactor changes field access to receiver method calls without any supplied evidence of a concrete merge-blocking issue; no actionable risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title describes receiver-method refactoring, but the changes update Codex app-server event mapping and tool-result handling. Rename the title to describe the Codex app-server event mapping and multi-event notification handling changes.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/receiver-method-calls
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch refactor/receiver-method-calls

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 23, 2026

Copy link
Copy Markdown

Gavel summary

Source Pass Fail Skip Duration

Totals: 0 passed · 0 failed · 0 skipped · -

View full results

@socket-security

socket-security Bot commented Aug 23, 2026

Copy link
Copy Markdown

No dependency changes detected. Learn more about Socket for GitHub.

👍 No dependency changes detected in pull request

@github-actions

github-actions Bot commented Aug 23, 2026

Copy link
Copy Markdown

Gavel summary

Source Pass Fail Skip Duration
agentcreds 11 0 0 702.913µs
ai 87 0 0 26ms
aichat 135 0 2 15.8s
anthropicmock 1 0 0 1ms
api 223 0 0 33ms
attachments 5 0 0 4ms
bash 4 0 0 164.016µs
callertools 10 0 0 90ms
captain 17 0 0 10.5s
claude 22 0 0 30ms
claudeagent 13 0 0 204ms
cli 239 0 0 7.5s
cmux 4 0 0 234.527µs
collections 12 0 0 577.128µs
credentials 5 0 0 27ms
credsync 11 0 0 249ms
database 20 0 0 4.4s
deploy 110 0 0 235ms
genkit 25 0 0 86ms
gitagent 122 0 0 15.3s
github.com/flanksource/captain/migrations 11 0 0 4.2s
github.com/flanksource/captain/pkg/ai 250 0 0 550ms
github.com/flanksource/captain/pkg/ai/agent 26 0 0 450ms
github.com/flanksource/captain/pkg/ai/agent/commit 56 0 0 4.2s
github.com/flanksource/captain/pkg/ai/agent/setup 16 0 0 60ms
github.com/flanksource/captain/pkg/ai/agent/verify 21 0 0 390ms
github.com/flanksource/captain/pkg/ai/agent/worktree 6 0 0 -
github.com/flanksource/captain/pkg/ai/assistanttags 15 0 0 -
github.com/flanksource/captain/pkg/ai/fixture 42 0 0 130ms
github.com/flanksource/captain/pkg/ai/fixture/kubeproxy 2 0 0 30ms
github.com/flanksource/captain/pkg/ai/fixture/mcpproxy 6 0 0 -
github.com/flanksource/captain/pkg/ai/history 54 0 0 -
github.com/flanksource/captain/pkg/ai/internal/gen-model-registry 12 0 0 -
github.com/flanksource/captain/pkg/ai/middleware 25 0 0 -
github.com/flanksource/captain/pkg/ai/pricing 9 0 0 -
github.com/flanksource/captain/pkg/ai/prompt 16 0 0 10ms
github.com/flanksource/captain/pkg/ai/provider 169 0 1 20ms
github.com/flanksource/captain/pkg/ai/provider/claudeagent 54 0 0 1.7s
github.com/flanksource/captain/pkg/ai/provider/cmux 128 0 0 850ms
github.com/flanksource/captain/pkg/ai/provider/genkit 36 0 0 10ms
github.com/flanksource/captain/pkg/ai/provider/jsonrpc 6 0 0 50ms
github.com/flanksource/captain/pkg/aichat 9 0 0 100ms
github.com/flanksource/captain/pkg/aimock 50 0 7 470ms
github.com/flanksource/captain/pkg/aimock/anthropicmock 14 0 0 20ms
github.com/flanksource/captain/pkg/aimock/openaimock 18 0 0 -
github.com/flanksource/captain/pkg/api 143 0 0 340ms
github.com/flanksource/captain/pkg/api/registry 96 0 0 -
github.com/flanksource/captain/pkg/bash 348 0 0 30ms
github.com/flanksource/captain/pkg/captainconfig 25 0 0 -
github.com/flanksource/captain/pkg/captaintoken 23 0 0 1.6s
github.com/flanksource/captain/pkg/claude 138 0 0 -
github.com/flanksource/captain/pkg/claude/tools 18 0 0 -
github.com/flanksource/captain/pkg/cli 779 0 1 1m7s
github.com/flanksource/captain/pkg/cmux 1 0 0 -
github.com/flanksource/captain/pkg/codexconfig 10 0 0 10ms
github.com/flanksource/captain/pkg/container 72 0 1 -
github.com/flanksource/captain/pkg/database 130 0 0 18.8s
github.com/flanksource/captain/pkg/dod 11 0 0 1m0s
github.com/flanksource/captain/pkg/gitagent 31 0 0 390ms
github.com/flanksource/captain/pkg/gitagent/proxy 12 0 0 10ms
github.com/flanksource/captain/pkg/monitor 60 0 0 3.3s
github.com/flanksource/captain/pkg/sandbox 13 0 0 -
github.com/flanksource/captain/pkg/sandbox/adapter 39 0 0 -
github.com/flanksource/captain/pkg/sandbox/presets 13 0 0 -
github.com/flanksource/captain/pkg/session 70 0 0 10ms
history 69 0 0 10ms
migrations 12 0 0 5.3s
openaimock 2 0 0 127.369µs
provider 13 0 0 12ms
registry 38 0 0 3ms
session 28 0 0 9ms
tools 53 0 0 2ms

Totals: 4374 passed · 0 failed · 12 skipped · 3m45s

View full results

Comment thread pkg/api/toolpolicy.go Fixed
Comment thread pkg/cli/gitagent_deploy.go Fixed
Comment thread pkg/gitagent/tokenfile.go Fixed
…ations

Carry the clicky RPCOperation whole on ToolInfo/ToolDefinition instead of flattening entity, verb, scope, method and path into a clicky/* annotation map that three call sites parsed back into typed fields. The key names no longer live twice, once as constants here and once as literals in clicky.

Add PermissionStrategy with HTTPVerbStrategy and MCPHintStrategy — the first implementation of the ordered policy in #61. Authority resolves weakest to strongest: what a tool's own facts imply, what its author registered, then an operator's rules. At every layer 'auto' defers to the layer below rather than overriding it, so a rule can hand a tool back to its hints instead of being ignored.

serve_chat's isReadOnlyCaptainTool becomes captainReadOnlyStrategy, declared in the chain where captain hands its command tree over, and ResolveOptions.Strategies threads it through the chat service.
Modified the test to properly wait for agent task completion by checking for a completion marker file instead of immediately returning. This ensures the agent has fully exited before assertions are made, making the test more reliable and preventing race conditions.
@moshloop
moshloop enabled auto-merge (rebase) August 24, 2026 08:15
auto-merge was automatically disabled August 24, 2026 08:23

Rebase failed

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.

2 participants