Skip to content

feat(run-token): investigate REST calls use the run token in enforce - #788

Merged
mashkovd merged 2 commits into
mainfrom
feat/run-token-rest-clients-717
Oct 24, 2026
Merged

mashkovd merged 2 commits into
mainfrom
feat/run-token-rest-clients-717

Conversation

@mashkovd

Copy link
Copy Markdown
Member

Why

Option (c) on #717 (owner decision 2026-10-24): move each run's REST calls off the static MCTL_TOKEN onto its scoped run token, then remove MCTL_TOKEN from the CWFT. This is slice 2 of the plan posted on #717, for investigate.

Slice 1 (mctlhq/mctl-api#779, 4.119.0) already admits everything the DevLoop path needs under the investigate scope, so this slice needs no mctl-api change:

Call (investigate pod, run with a we_ execution) Route Admitted by
work item + ledger, intents, execution request, snapshot read GET /api/v1/work-items/{own}/... BindWorkItem GET route
seal the context snapshot POST /api/v1/work-items/{own}/executions/{own}/snapshot BindWorkItem + BindExecution (#779)

DevLoop work items are created with visibility: tenant, and the scope's ReadAllTenants passes canSeeWorkItem.

What

  • run_token.mctl_api_bearer(): the run token in enforce mode; "" (no credential) when enforce has no run token, never the static token; otherwise MCTL_TOKEN.
  • WorkItemClient resolves its bearer per request through it, so a client built before setup() still uses the run token. An explicit token= argument still wins (Temporal workers are unchanged: their mode is off).
  • capability_gateway._default_remote_headers() uses run_token.mcp_auth() like mctl_mcp_config(). Before this, discovery mode would have connected with the static token in an enforce run.
  • run_issue_investigator.main() resolves --issue-url from the work item after run_token.setup().

Not in this PR (later slices on #717)

  • Self-attaching runs (no we_ execution): POST .../executions is outside the scope, so the attach is refused. Investigate's work context is at observe, where a refused attach proceeds without work context (no snapshot). Design pending owner decision.
  • ActionApprovalClient stays on MCTL_TOKEN. Under the investigate scope only read tools pass mctl-api anyway, so a granted approval could not be used by investigate today.

Tests

tests/test_run_token.py, each mutation-verified (reverting the change turns it red):

  • enforce REST calls carry the run token, including from a client built before setup() (red with the old MCTL_TOKEN read)
  • enforce without a run token sends nothing and answers WORK_ITEM_UNKNOWN (red with a static fallback)
  • the gateway connects with the run token / fails closed without setup() (red with the old header code)
  • main() calls setup() before the issue-url read (red with the old order)

Full suite: 6731 passed, 21 skipped.

Refs #717

🤖 Generated with Claude Code

In run-token enforce mode the investigate pod still called mctl-api's
work-item routes (and, in discovery mode, the capability gateway's MCP
connection) with the static MCTL_TOKEN, so MCTL_TOKEN could not leave the
investigate template. mctl-api 4.119.0 already admits everything the
DevLoop path needs under the investigate scope: reads of the run's own
work item and sealing its own execution's snapshot.

- run_token.mctl_api_bearer(): the run token in enforce mode, "" (no
  credential, never the static token) when enforce has none, otherwise
  MCTL_TOKEN.
- WorkItemClient resolves its bearer per request through it, so a client
  built before setup() still uses the run token.
- The capability gateway uses run_token.mcp_auth() like mctl_mcp_config().
- The investigator resolves --issue-url from the work item after
  run_token.setup(), not before.

Self-attaching runs (no we_ execution) and action approvals are refused
by the scope until their own slices land; investigate's work context is
at observe, where a refused attach proceeds without a snapshot.

Refs #717
@claude

claude Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Claude finished @mashkovd's task in 15s —— View job


Review complete

Approved at 072a444: no P1/P2 findings, 1 P3.

  • Enforce mode with no run token never falls back to the static token, in both the REST client and the gateway.
  • The per-request bearer resolution handles a client built before setup().
  • main() now calls setup() before the issue-url read.
  • P3: with --dry-run, setup() is skipped, so under enforce the --issue-url resolution from the work item gets no credential. Behavior is consistent with fail-closed, so I'm only flagging it.


run_token.setup(shadow_scope="investigate")

# After run_token.setup(): in enforce mode this read uses the run token,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3 — With --dry-run, run_token.setup() is skipped, so under MCTL_RUN_TOKEN_MODE=enforce this --issue-url resolution gets no credential and fails with WORK_ITEM_UNKNOWN (before, it used MCTL_TOKEN). Probably fine since enforce pods shouldn't hold MCTL_TOKEN, but worth a note if dry-run is used with work-item-only invocations.

claude[bot]
claude Bot previously approved these changes Oct 24, 2026

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

No P1/P2 findings (1 P3). Good to merge.

@github-actions

Copy link
Copy Markdown
Contributor

Antigravity (agy) review

Model: gemini-3.8-flash-high · commit 072a444.

Finding 1: run_issue_investigator fails in enforce mode during dry-run with --work-item-id

  • Severity: P2
  • File & Line: orchestrator/run_issue_investigator.py:4660
  • Anchor: Anchor: + issue_url = args.issue_url
  • Concrete Failure Scenario:
    When run_issue_investigator is executed in enforce mode (MCTL_RUN_TOKEN_MODE=enforce) with --work-item-id <id> and --dry-run, run_token.setup is skipped because it is guarded by if not args.dry_run: (lines 4649–4658).
    Execution then reaches line 4661: if not issue_url: invokes WorkItemClient().get(args.work_item_id). In enforce mode, mctl_api_bearer() returns "" because _state.token is None. WorkItemClient._request() raises WorkItemUnavailable("no mctl-api credential: MCTL_TOKEN is not set, or enforce mode has no run token"), resulting in WORK_ITEM_UNKNOWN and triggering:
    raise SystemExit(f"could not resolve --issue-url from work item {args.work_item_id!r}: {answer.reason}")
    The purpose of --dry-run is to resolve the issue URL and repository slug without requiring Anthropic/Claude credentials (as documented in lines 4644–4648). Moving WorkItemClient().get() after run_token.setup() without decoupling run_token.setup() from SDK authentication breaks --dry-run under enforce mode.
  • Proposed Fix:
    Separate Claude SDK authentication from run-token setup. run_token.setup only interacts with mctl-api token exchange and does not require Claude credentials, so it should run prior to work-item resolution even in dry-run mode:
    if not args.dry_run:
        from orchestrator.auth import ensure_auth_for_sdk
        ensure_auth_for_sdk()
    
    from orchestrator import run_token
    run_token.setup(shadow_scope="investigate")
    
    issue_url = args.issue_url
    if not issue_url:
        ...

Finding 2: WorkItemClient._bearer bypasses enforce mode check when an explicit token is configured

  • Severity: P3
  • File & Line: orchestrator/work_context/client.py:148
  • Anchor: Anchor: + if self._token is not None:
  • Concrete Failure Scenario:
    In WorkItemClient._bearer, if self._token was passed explicitly into WorkItemClient.__init__, it returns self._token immediately without calling mctl_api_bearer(self._token). If a caller instantiates WorkItemClient(token=static_token) in enforce mode, the client uses static_token rather than the run token, bypassing the enforce-mode invariant ("never the static token: a run meant to be scoped does not fall back to it"). Additionally, mctl_api_bearer(static_token: str | None = None) accepts static_token to handle this exact case, but WorkItemClient never passes self._token to it.
  • Proposed Fix:
    Delegate token resolution to run_token.mctl_api_bearer:
    def _bearer(self) -> str:
        from orchestrator import run_token
        return run_token.mctl_api_bearer(self._token)

Finding 3: Missing test coverage for dry-run work item resolution and gateway observe mode

  • Severity: P3
  • File & Line: tests/test_run_token.py:284
  • Anchor: Anchor: +def test_investigator_resolves_the_issue_url_after_the_run_token(monkeypatch):
  • Concrete Failure Scenario:
    test_investigator_resolves_the_issue_url_after_the_run_token tests main() when --dry-run is absent, which masks the fact that run_token.setup() is skipped when --dry-run is passed. Furthermore, while tests were added for _default_remote_headers in enforce mode, there is no test in tests/test_run_token.py verifying that _default_remote_headers() includes X-Mctl-Agent-Scope-Shadow when in observe mode.
  • Proposed Fix:
    In tests/test_run_token.py, add test coverage for:
    1. run_issue_investigator.main() with --work-item-id and --dry-run to ensure setup() is called before get().
    2. capability_gateway._default_remote_headers() in observe mode asserting that X-Mctl-Agent-Scope-Shadow is populated.

agy P2 on #788: under enforce, a --dry-run with only --work-item-id
skipped run_token.setup() and so resolved the issue url with no
credential. setup() needs no Claude credential, so it now runs in dry-run
as well. An explicit WorkItemClient token no longer overrides the run
token in enforce mode, and the gateway's observe shadow header is tested.

Refs #717
@mashkovd

Copy link
Copy Markdown
Member Author

Addressed in 896e5d8:

  • agy P2 (dry-run under enforce resolved the issue url with no credential): run_token.setup() now runs in dry-run as well; it needs no Claude credential. Test test_investigator_resolves_the_issue_url_after_the_run_token[True], red with setup back inside the non-dry-run branch. Same as the Claude P3.
  • agy P3 (explicit token bypasses enforce): WorkItemClient._bearer now delegates to mctl_api_bearer(self._token). Test test_enforce_rest_calls_ignore_an_explicit_static_token, mutation-verified.
  • agy P3 (tests): dry-run case added as above; test_observe_gateway_shadows_on_the_static_token covers the gateway's observe shadow header, red without headers.update(scope_headers).

Full suite: 6734 passed, 21 skipped.

@claude

claude Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Claude finished @mashkovd's task in 16s —— View job


Review complete

Approved at 896e5d8 with no P1/P2 findings.

  • Enforce mode never falls back to the static token. mctl_api_bearer returns "" when there is no run token, and an explicit token= only replaces MCTL_TOKEN.
  • The bearer is resolved per request, so a client built before setup() still uses the run token.
  • The gateway headers now go through run_token.mcp_auth(), matching mctl_mcp_config().
  • run_token.setup() now runs in --dry-run too, ahead of the --issue-url work-item read. This fixes the earlier P3 and the agy P2 about dry-run failing under enforce.
  • agy Finding 2 is already handled: _bearer passes self._token to mctl_api_bearer.
  • Not addressed: agy Finding 3, the optional test for --dry-run with --work-item-id and the observe-mode gateway header test. It isn't blocking.

This was a static review. I didn't run the tests.

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

No P1/P2 findings (0 P3). Good to merge. At 896e5d8 the dry-run gap is closed (setup() now runs before the work-item read), and WorkItemClient._bearer passes an explicit token through mctl_api_bearer, so enforce never uses a static token.

@github-actions

Copy link
Copy Markdown
Contributor

Antigravity (agy) review

Model: gemini-3.8-flash-high · commit 896e5d8.

No significant issues found.

@mashkovd
mashkovd merged commit 94d840e into main Oct 24, 2026
19 checks passed
@mashkovd
mashkovd deleted the feat/run-token-rest-clients-717 branch October 24, 2026 07:47
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