Skip to content

[ML-69215] Fix matrix App auth and implicit Runtime Store cleanup - #752

Closed
jamesbxwu wants to merge 5 commits into
mainfrom
codex/agentbricks-app-auth-matrix
Closed

jamesbxwu wants to merge 5 commits into
mainfrom
codex/agentbricks-app-auth-matrix

Conversation

@jamesbxwu

@jamesbxwu jamesbxwu commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

The deployed tool matrix expects App service-principal grants, but UC-function add now defaults to request-user auth. Cleanup also omitted implicitly provisioned Runtime Stores and roles for direct-authoring Apps without memory/session bindings.

Select App identity explicitly in both authoring paths and validate CLI bindings before deployment. Clean resources for every owned matrix App after verifying the canonical Runtime Store, App owner, service principal, and Lakebase role. Retain verified cleanup targets after failures and keep cleanup-attempt evidence.

A retry now also handles an App that was deleted before its absence poll failed. If the Runtime Store and App are absent, require a typed not-found response for the cached role before finishing source cleanup. A surviving or unreadable role blocks teardown. All changes are in the test harness, fixtures, tests, and documentation; grant assertions and ownership checks remain intact.

Validation:

  • 59 cleanup/evidence tests pass, including three new two-attempt regressions for an absent, surviving, or unreadable role after App deletion. The new regressions fail the preceding harness.
  • The preceding auth/cleanup implementation passed 256 focused matrix/CLI/access tests. Ruff, formatting, focused type checks, diff checks, and commit hooks pass for the follow-up.
  • Historical live run 38701390693 passed 20/20 LangGraph matrix rows, grant snapshots, transitive controls, and repeat deployment. It predates the cleanup corrections and does not validate these retries. No new model matrix was run.

Independent read-only run 38708044949 confirmed all 13 exact prior-run resources absent. One separate current-window database/role pair remains unattributed and intact, so full cleanup of that matrix run is not established. The historical cache rerun was budget-limited and was not all green; no budgets were changed. Public CI has been retriggered for this follow-up.

Combined follow-up validation

Test-only source 7b6e3ac407a42acc0b86874b9e36ccb8639d0167 combines the twelve bug-bash PRs and these review follow-ups. The Agent Bricks unit suite passed 2,194 tests, 24 skipped in 60.46s; combined Ruff and formatting checks pass. The README conflict was resolved by retaining the guide split and placing the new auth guidance in Authentication. This combined branch is for testing and must not be merged into main. Historical live runs predate the review fixes; no fresh live acceptance is claimed.

Current public CI: 53 green, 1 red. The framework job fails only the same six memory-backed parallel-history ordering cases already reproduced on main. No skips were applied.

@jamesbxwu jamesbxwu changed the title Keep App grant matrix fixtures explicit about tool identity Fix matrix App auth and implicit Runtime Store cleanup Oct 15, 2026

@jamesbxwu jamesbxwu left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

is this still relevant given #706?

@jamesbxwu

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #706's replacement pytest E2E suites, following the active-PR overlap review.

#706 tears down Runtime Stores for every deployed project, records and retries Lakebase identities, handles already-missing resources, and sweeps source folders independently. Its UC-function CLI and direct-TOML paths already use App identity and verify the App's EXECUTE grant. The UC-function command has no --auth option, so this draft's extra UC auth flag must not be ported.

The old nightly still runs tests/e2e/tool_matrix.py until the private runner cuts over to tests/suites; closing this draft does not fix that old nightly path or establish live acceptance of #706. Keep 95051bd available as historical cleanup/retry evidence if an interim old-harness repair is needed. The replacement harness should be the maintained path. No PR was merged and no new live/model matrix was run.

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