Skip to content

feat(api): add typed workspace selectors - #3245

Open
mrunalp wants to merge 2 commits into
NVIDIA:mainfrom
mrunalp:feat/3048-typed-workspace-selectors/mrunalp
Open

feat(api): add typed workspace selectors#3245
mrunalp wants to merge 2 commits into
NVIDIA:mainfrom
mrunalp:feat/3048-typed-workspace-selectors/mrunalp

Conversation

@mrunalp

@mrunalp mrunalp commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • replace ambiguous workspace strings and all_workspaces booleans with the typed WorkspaceSelector protobuf oneof
  • require explicit named selectors for workspace-scoped RPCs and allow the all-workspaces marker only on authorized list operations
  • migrate the CLI, TUI, Rust/Python/Go/TypeScript SDKs, interceptors, generated Go bindings, tests, and workspace documentation

Related Issue

Fixes #3048

Changes

  • add WorkspaceSelector with named-workspace and all-workspaces variants
  • reserve the removed protobuf field names and tags
  • centralize selector validation and workspace authorization in the gateway
  • reject omitted or empty workspace selectors, invalid all-workspaces usage, and selectors on gateway-global policy requests
  • expose explicit all-workspace list methods/scopes in supported SDKs while keeping CLI --all-workspaces behavior
  • document the breaking request-shape migration and stable authorization semantics

Testing

  • mise run pre-commit
  • mise run ci
  • cargo test --workspace
  • mise run sdk:ts:ci
  • mise run go:ci
  • Python unit, lint, and typecheck suites

Checklist

  • Tests added or updated
  • Documentation updated
  • Generated bindings updated
  • DCO sign-off included

@mrunalp
mrunalp requested review from a team, derekwaynecarr and sjenning as code owners September 9, 2026 20:09
@copy-pr-bot

copy-pr-bot Bot commented Sep 9, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
@mrunalp
mrunalp force-pushed the feat/3048-typed-workspace-selectors/mrunalp branch from 4943138 to e3c45dc Compare September 9, 2026 20:27
@johntmyers johntmyers added the gator:in-review Gator is reviewing or awaiting PR review feedback label Sep 9, 2026

@johntmyers johntmyers 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.

gator-agent

PR Review Status

This API migration is project-valid through #3048 and the typed selector/authorization direction is sound, but three concrete migration gaps must be fixed before test dispatch.

Action required: @mrunalp, please fix the non-default template request and update the TypeScript example and TUI contributor guidance described below.

Blocking findings:

  • GATOR-e3c45dc8-01: the CLI hard-codes template metadata to default while sending the selected workspace separately (inline).
  • GATOR-e3c45dc8-03: the published TypeScript raw-client example omits the now-required selector.
  • GATOR-e3c45dc8-04: the TUI contributor skill still documents removed request fields.

Carried findings:

  • None

Warning — GATOR-e3c45dc8-03 · Published raw-client example now sends an invalid request

Summary: A TypeScript SDK user who follows the raw getSandboxPolicyStatus example sends global: false without workspaceScope. This PR makes that selector mandatory for sandbox policy status, so the documented request now fails with workspace_scope is required instead of returning status.

Fix: Import the generated selector type/helper needed by the raw API and update the example to explicitly select default.

Verify: Run the documented request against this head for an existing default-workspace sandbox; it must reach status lookup rather than fail selector validation.

Agent context
  • Location: proto/openshell.proto:2356
  • Affected example: sdk/typescript/README.md:222

Warning — GATOR-e3c45dc8-04 · TUI contributor guidance describes removed protobuf fields

Summary: A contributor following the repository's TUI workflow is told to construct workspace and all_workspaces fields for requests that this PR migrates to workspace_scope. Those literals no longer compile, and the guidance can lead future TUI work back toward the obsolete scope model.

Fix: Update .agents/skills/tui-development/SKILL.md to describe Option<WorkspaceSelector>, named versus all-workspaces variants, and omission for global configuration requests.

Verify: Follow the updated field reference to construct list, log, provider, and config requests at this head; every example must use fields present in the generated Rust types.

Agent context
  • Location: crates/openshell-tui/src/lib.rs:43
  • Stale guidance: .agents/skills/tui-development/SKILL.md:493-498
Gator metadata
  • Validation: Project-valid implementation of linked issue #3048; duplicate search found no competing work.
  • Docs: User-facing workspace migration docs are present; one TypeScript example and the TUI contributor skill remain stale.
  • Checks: DCO and available lightweight checks pass; required branch and Helm checks await copy-PR dispatch.
  • E2E: test:e2e is required for gateway authorization behavior and will be dispatched after review blockers are resolved.
  • Head SHA: e3c45dc88cc9c54bd95f3390d74ea05760c01945
  • Base SHA: f4dc6be4b2ed7d6441c9f9833803e00b92119f05
  • Merge base SHA: f4dc6be4b2ed7d6441c9f9833803e00b92119f05
  • Patch ID: 5dc7768c5c94bc4b87c493fcba131f2a9dfdb79b
  • Gator payload: 8
  • Review mode: initial
  • Previous reviewed SHA: none
  • Review budget exhausted: no
  • Maintainer decision required: no
  • Next state: gator:in-review

Comment thread crates/openshell-cli/src/run.rs Outdated
Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
@mrunalp

mrunalp commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed all findings from the latest review in 3c27970b5:

  • GATOR-e3c45dc8-01: template metadata now uses the selected workspace; the CLI integration test creates in team-a and asserts both request scope and metadata.
  • GATOR-e3c45dc8-03: the TypeScript raw-client example now constructs and sends an explicit default WorkspaceSelector. The example was independently type-checked against the built package.
  • GATOR-e3c45dc8-04: the TUI contributor skill now documents workspace_scope, named/all-workspaces helpers, and selector omission for gateway-global config requests. A cross-skill search found no remaining copies of the removed request fields.

Validation:

  • focused non-default template integration test
  • mise run sdk:ts:ci
  • README raw-client example type-check
  • mise run pre-commit

@johntmyers johntmyers added the test:e2e Requires end-to-end coverage label Sep 9, 2026
@johntmyers

Copy link
Copy Markdown
Collaborator

/ok to test 3c27970

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

Label test:e2e applied for 3c27970. Open Branch E2E Checks, find the run for commit 3c27970, and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

@johntmyers johntmyers 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.

gator-agent

PR Review Status

Thanks @mrunalp. I checked the four-file author delta and confirmed the non-default template metadata and integration assertion, the TypeScript raw-client selector example, and the updated TUI contributor guidance now resolve all three prior findings. No new blocking findings were found in the follow-up delta.

Blocking findings:

  • No blocking findings remain

Carried findings:

  • None
Gator metadata
  • Validation: Project-valid implementation of linked issue #3048.
  • Docs: Published workspace migration docs, the TypeScript raw-client example, and TUI contributor guidance are updated.
  • Checks: Current-head Branch Checks and E2E are running; Helm Lint and Trivy Changes are green.
  • E2E: test:e2e applied; /ok to test 3c27970b53702e3f603d078641d96c0d78fafab7 dispatched; current-head Branch E2E run 34416785444 is queued/running.
  • Head SHA: 3c27970b53702e3f603d078641d96c0d78fafab7
  • Base SHA: f4dc6be4b2ed7d6441c9f9833803e00b92119f05
  • Merge base SHA: f4dc6be4b2ed7d6441c9f9833803e00b92119f05
  • Patch ID: be3e81f30da35b628ee29aaf4f7fddb628db58e2
  • Gator payload: 8
  • Review mode: follow_up
  • Previous reviewed SHA: e3c45dc88cc9c54bd95f3390d74ea05760c01945
  • Review budget exhausted: no
  • Maintainer decision required: no
  • Next state: gator:watch-pipeline

@johntmyers johntmyers added gator:watch-pipeline Gator is monitoring PR CI/CD status gator:in-review Gator is reviewing or awaiting PR review feedback gator:blocked Gator is blocked by process or repository gates and removed gator:in-review Gator is reviewing or awaiting PR review feedback gator:watch-pipeline Gator is monitoring PR CI/CD status labels Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gator:blocked Gator is blocked by process or repository gates test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(api)!: replace implicit workspace selection with typed scope selectors

2 participants