Skip to content

[AIGTWY-4876] Test managed config CUJ1 - #991

Merged
JTSIV1 merged 9 commits into
JTSIV1/aigtwy-4876-cuj1-prerequisitefrom
JTSIV1/aigtwy-4876-cuj1-journey
Oct 7, 2026
Merged

JTSIV1 merged 9 commits into
JTSIV1/aigtwy-4876-cuj1-prerequisitefrom
JTSIV1/aigtwy-4876-cuj1-journey

Conversation

@JTSIV1

@JTSIV1 JTSIV1 commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Add CUJ1 tests that configure Claude and Codex from one published policy and verify their model catalogs, defaults, inference headers, and traces. Run six agent tasks to check the selected models and follow-up requests.

@JTSIV1 JTSIV1 changed the title Cover managed two-agent CUJ on dedicated workspace [AIGTWY-4876] Test managed config CUJ1 Oct 5, 2026
@JTSIV1
JTSIV1 marked this pull request as ready for review October 5, 2026 22:02
@JTSIV1
JTSIV1 requested a review from lilly-luo October 5, 2026 22:02
@lilly-luo lilly-luo added the ug-review Run the automated UG review label Oct 5, 2026
@github-actions

github-actions Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

UG review

The visible changes add live managed Claude/Codex configuration, inference, and trace checks plus shared TUI boot handling. The request-count assertion does not establish that tool-result follow-ups were exercised.

Major

  • Assert actual tool-result follow-ups — tests/e2e_cuj/test_ug_managed_sentinel.py:95: can we assert a marked tool_result/function_call_output follow-up? retries or auxiliary POSTs also satisfy this count without verifying tool-result forwarding.

Some patches were unavailable or truncated to fit the review context. Treat this as a partial review.

Automated advisory review of cae93eac2763 using Lilly's UG review rubric. It does not approve or block this PR.

Comment thread tests/e2e_cuj/test_ug_managed_sentinel.py
@JTSIV1
JTSIV1 force-pushed the JTSIV1/aigtwy-4876-cuj1-journey branch 2 times, most recently from 97e08bd to f177360 Compare October 6, 2026 13:43
@JTSIV1
JTSIV1 changed the base branch from main to JTSIV1/aigtwy-4876-cuj1-prerequisite October 6, 2026 13:43
@JTSIV1
JTSIV1 added this pull request to stack #1005 October 6, 2026 13:51
@lilly-luo

Copy link
Copy Markdown
Collaborator

can you ensure Test Claude's Opus, Sonnet, and Haiku aliases each complete a task on the matching actual model is captured?

@david-siqi-liu david-siqi-liu 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.

Reviewed against main's tests/e2e_cuj/AGENTS.md (#1000) and the CUJ 1 plan in the UG Configure E2E doc. Lilly's earlier comments about splitting the single test still apply and aren't repeated here.

P0

  1. tests/integration/utils/terminal.py ~317: the "Select model" render wait now runs only when model_visible is None, so a caller's callback replaces the gate instead of adding to it. test_ug_claude_model_discovery.py cases 07 and 09 (lines 106, 124) pass lambda _text: session.claude_gateway_cache_ready(), which checks only the disk cache, so when the cache is already warm the screen is captured right after /model, before the picker renders. Keeping the render wait and adding the callback as a second condition avoids the flake.
  2. Built on the #971 scaffold with its own tests/e2e_cuj/conftest.py (session / live_session, sys.path imports). main's #1000 conftest defines cuj and requires --confcutdir=tests/e2e_cuj, so the two conftests conflict (add/add), and keeping either one breaks the other's tests. The rebase means moving onto cuj and the package-relative helpers. That also brings main's MANAGED_PATHS preflight, which this conftest lacks; the teardown runs a terminal ug revert but never checks that the machine-wide files are gone.
  3. test_ug_managed_sentinel.py ~87: _assert_inference_requests needs at least two marked HTTP 200 requests with payload["model"] == expected, and FileTask.assert_completed checks only that some answer contains the file value. Neither ties the model to the native completed turn, so a final turn on a different model still passes. SessionEvidence(...).completed(task) plus canonical_model on the turn models closes it.

P1

  • Coverage against the CUJ 1 plan: "each agent's model picker shows exactly the configured models" is checked as "every configured model is on screen" in the Claude TUI (other rows aren't rejected), with exactness only from the settings file. "Neither agent ever sends the other's agent header value" is implied by each request matching its own value, with no explicit check.

No E2E CUJs job has run on this stack yet (#1004 -> #991 -> #992), so none of this has been exercised live.

@JTSIV1
JTSIV1 force-pushed the JTSIV1/aigtwy-4876-cuj1-journey branch from 194da2b to 99097bd Compare October 7, 2026 02:11
@david-siqi-liu

david-siqi-liu commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Correction: I rechecked against a stale local copy of this branch earlier. At 99097bd and now cae93ea, P0 2 and P0 3 are fixed: the stack uses main's cuj conftest with package-relative helpers, and _assert_native_model ties the expected model to the native completed turn via SessionEvidence and canonical_model. The picker render gate (P0 1) is fixed too, and #1004 merges cleanly onto current main. No remaining blockers from my side.

@david-siqi-liu david-siqi-liu 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.

No remaining blockers: the picker render gate, the move onto main's cuj scaffold, and the native completed-turn model checks are all in.

@JTSIV1
JTSIV1 added this pull request to the merge queue Oct 7, 2026
Merged via the queue into main with commit dbadcb7 Oct 7, 2026
27 checks passed
@JTSIV1
JTSIV1 deleted the JTSIV1/aigtwy-4876-cuj1-journey branch October 7, 2026 03:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ug-review Run the automated UG review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants