Skip to content

feat(agent): support canonical global execution in trusted broker - #4484

Merged
kojiwakayama merged 3 commits into
mainfrom
feat/trusted-global-execution
Sep 13, 2026
Merged

kojiwakayama merged 3 commits into
mainfrom
feat/trusted-global-execution

Conversation

@kojiwakayama

@kojiwakayama kojiwakayama commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

The trusted broker requires a non-null project ID even for canonical runs backed by a globally owned source. Global chat therefore cannot use the project-tools executor profile without inventing a project identity.

Accept explicit projectId: null for canonical execution with a global source. Keep the canonical run binding, omit project identity from the tool's execution context, and normalize the broker's internal tool context consistently. Project-owned sources still require a project ID; missing projectId and a project slug without an ID are rejected. Optional global steering remains an explicit capability with matching callbacks. The agent loop, model/host operations and private data stay in the broker; no full-runtime fallback is added.

Validation: the global end-to-end broker fixture failed on the original project-only guard and now passes. The five focused files pass 93 nested checks, including real in-memory channel installation, model/host/project-tool execution, private-data non-disclosure on the project wire, global steering and pre-allocation rejection of invalid contexts. Full framework and explicit test types, lint/format, module/dependency boundaries and generated API references pass. No native/prototype security probes were executed locally.

This is framework support for veryfront/veryfront-issue-inbox#1037 and #367. Product broker composition, ephemeral execution, authorized project transitions, infrastructure readiness and actual routing cutover remain separate delivery work. It does not activate isolated staging or production traffic.

Summary by CodeRabbit

  • New Features

    • Added support for canonical projectless execution with globally owned tools.
    • Global runs may explicitly use a null project ID while retaining their canonical run identity.
  • Bug Fixes

    • Improved validation for project ownership, project IDs, and project slugs.
    • Prevented project identities from being inferred when global execution is explicitly selected.
  • Documentation

    • Documented projectless global execution requirements, including that project-owned sources still require a project ID.
    • Clarified that ephemeral execution and project transitions remain unsupported.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

@github-actions

Copy link
Copy Markdown

📦 Client bundle boundary

Entrypoint Modules Source size Server leaks
src/index.client.ts 289 2307 KiB ✅ 0

A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in scripts/lint/client-bundle-baseline.json to burn down.

@coderabbitai

coderabbitai Bot commented Sep 11, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 13a508d4-d6a8-4331-b057-c75eb16409bf

📥 Commits

Reviewing files that changed from the base of the PR and between 2a8d28d and 7f61a8a.

📒 Files selected for processing (3)
  • docs/api-reference/veryfront/agent.md
  • src/agent/hosted/executor-project-install.test.ts
  • src/agent/service/trusted-managed-broker.ts

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


📝 Walkthrough

Walkthrough

The runtime now supports canonical executions for globally owned tools without a project identity. Schemas, trusted-runtime validation, broker context construction, tests, and documentation reject invalid project-scoped or project-slug combinations.

Changes

Global projectless execution

Layer / File(s) Summary
Projectless context contracts
src/agent/hosted/executor-project-tools.ts, src/agent/hosted/executor-runtime-install-schema.ts
Context and installation schemas accept projectId: null for global-owned sources and reject incompatible project slugs or scopes.
Trusted runtime global execution
src/agent/hosted/trusted-managed-runtime.ts, src/agent/hosted/managed-executor-broker.ts
Canonical global executions may omit a project identity. Broker-created tool contexts expose no project ID.
Global execution validation
src/agent/hosted/managed-executor-broker.test.ts, src/agent/hosted/executor-project-runtime.test.ts, src/agent/hosted/executor-project-install.test.ts, docs/guides/agent-service-runtime.md, docs/api-reference/veryfront/agent.md, src/agent/service/trusted-managed-broker.ts
Tests cover valid global installations and runs, invalid project contexts, and installation capabilities. Documentation describes explicit global configuration and unsupported execution modes.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant ManagedExecutorBroker
  participant TrustedManagedRuntime
  participant ProjectTool
  ManagedExecutorBroker->>TrustedManagedRuntime: Validate canonical execution with null projectId
  TrustedManagedRuntime-->>ManagedExecutorBroker: Allow global-scope execution
  ManagedExecutorBroker->>ProjectTool: Provide context without projectId
Loading

Merge Risk: ⚪ Minimal · up to 7f61a

The change is mergeable; the new global execution boundaries and context handling have no unresolved material risk identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 23.08% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 8 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding canonical global execution support to the trusted broker.
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 23.08% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 8 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/trusted-global-execution

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.

@gitar-bot

gitar-bot Bot commented Sep 11, 2026 •

Copy link
Copy Markdown

Gitar is working

Gitar

Copy link
Copy Markdown
Contributor

Code Review — Score: 87/100 (Good)

Focused, well-tested change that lets the trusted broker accept an explicit projectId: null for canonical execution on a globally-owned source, without opening the door to project-owned runs skipping their project identity.

Strengths

  • Scope discipline: touches exactly the trusted-broker validation path (executor-project-tools.ts, executor-runtime-install-schema.ts, managed-executor-broker.ts, trusted-managed-runtime.ts) and leaves the agent loop / private-data boundary untouched, matching the PR description's stated boundaries.
  • Consistent with prior art: execution.projectId: string | null already existed at the grant-schema level and in managed-broker-project-state.ts's Scope type — this PR just lets the trusted-broker guards stop rejecting the null case for global owners instead of introducing a new concept.
  • Good negative-path test coverage: rejects a project-owned source with a null projectId, rejects a project slug alongside a projectless context, and rejects duplicate/invalid grants — all before allocation.
  • Explicit non-disclosure test (executes a canonical global run without inventing a project identity) asserts the real project id never crosses the wire and authToken isn't leaked into the project tool call context — good security-conscious test design for a broker whose whole job is isolating private state.
  • Docs (agent-service-runtime.md) were updated in the same PR to describe the new contract, including the "omitting projectId does not select global execution" and "no ephemeral/transition support" caveats.

Concerns

  • getExecutorProjectToolInstallSchema's refine only constrains the null case (projectId === null → owner must be global && no slug); it does not constrain the reverse — a global-scoped owner can still carry a non-null projectId. If that's intentionally allowed (e.g. future transition states), a comment would help; if not, worth tightening now while the invariant is fresh.
  • Neither this PR nor the pre-existing code appears to check that a project-owned source's execution.projectId actually matches owner.projectId — the guard only checks "not null". Not a regression introduced here, but since this PR is specifically hardening this exact validation, it's a good moment to note/confirm that gap is out of scope rather than silently relying on it.
  • Single commit, no CI results yet at review time (CodeRabbit/Automated review both still pending) — worth a look once they land, particularly since a security/isolation boundary is involved.

Nothing here blocks merging; the two schema-boundary notes above are worth a quick confirmation from the author rather than required changes.


Generated by Claude Code

@kojiwakayama

Copy link
Copy Markdown
Contributor Author

The global-owner/non-null-project combination is intentional and already supported: source ownership identifies the deployed code, while execution.projectId is the approved application context. The platform's global agent can operate on a selected project. Existing executor-project-runtime tests already use a global artifact owner with a non-null project context; the new cases add explicit projectless canonical execution.

This PR preserves the existing non-null project behavior and does not introduce cross-project authorization. Initial project-registered API dispatch separately checks that service.projectId equals run.projectId in resolveRuntimeAgentServiceIdForDispatch (veryfront-api/src/usecases/agent-runtime-targets/registered-project-runtime-identity.ts). Product broker integration must derive source ownership and execution context from canonical authorized state; project transitions and their fresh capabilities remain explicit outstanding work under inbox #1037. I am not treating these framework schema checks as a substitute for that authorization.

The new null case is stricter: it requires a globally owned source and no project slug. Missing projectId remains invalid. Local verification passed 88 focused checks, full types/lint, module/dependency boundaries and references. Current-head CI and the required automated review are still completing.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

@github-actions

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 63d1d8fe77

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/agent/hosted/managed-executor-broker.ts
@codecov

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/agent/hosted/trusted-managed-runtime.ts 16.66% 4 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

@github-actions

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. What shall we delve into next?

Reviewed commit: 7f61a8a4f8

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@sonarqubecloud

Copy link
Copy Markdown

@kojiwakayama
kojiwakayama added this pull request to the merge queue Sep 13, 2026
Merged via the queue into main with commit f105ec1 Sep 13, 2026
67 checks passed
@kojiwakayama
kojiwakayama deleted the feat/trusted-global-execution branch September 13, 2026 09:46
@kojiwakayama kojiwakayama mentioned this pull request Sep 16, 2026
3 tasks
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