feat: add public Agent Template library and onboarding intent - #2134
Conversation
baixiaohang
left a comment
There was a problem hiding this comment.
Recommendation: request changes
- Rationale: The two findings from #2133 are fixed, but a remaining async invalidation gap can carry the previous Team's Template into the newly selected Team.
Risk level: B-low
- Path baseline: includes
packages/server/**, but only the non-coreoauth-bootstrapservice. - Semantic lift: the Web auth/data layer and cross-Team state keep the effective risk at B-low.
PR summary
- Author / repo: yuezengwu / agent-team-foundation/first-tree
- Problem: Let anonymous visitors choose an official Agent Template, preserve that intent through authentication, and create the Template-backed Agent in an explicitly confirmed Team.
- Approach: Replaces #2133 with a CLA-valid single commit while retaining the public Library/detail, strict OAuth intent, authoritative Team selection, onboarding handoff, safe creation, and analytics behavior. The aggregate patch-id matches the stated
4f39a40b0bc7bbb3bee3be25ea6c11e063419b05. - Impacted modules: Shared intent parsing, Server OAuth bootstrap, Web auth/Team selection, public Template pages, onboarding, New Agent creation, workspace routing, and design tokens.
Review findings
❌ 1. The onboarding request sequence is invalidated only when the new passive effect runs. On an organizationId change, the render-time reset can commit Team B while intentFetchSeqRef still carries Team A's sequence. If Team A's pending promise resolves in the commit-to-effect window, its callback passes the sequence check, writes Team A's intentTemplate into Team B, makes intentPending false, and allows a persistent Agent creation with the wrong Template id. The existing late-response test resolves only after rerenderStep() has flushed the new effect, so it misses this window. Invalidate synchronously with the org/slug transition, or compare the callback against a current org+slug identity that is updated before a stale callback can run. [R4 / packages/web/src/pages/onboarding/steps/step-create-agent.tsx:82-104]
✅ The explicit-intent pending guards now cover both disabled controls and submit handlers, including an explicit plain-create escape.
✅ /me now validates session generation and token subject before any successful response mutates auth, Team, persistence, cache, or API-target state.
Action taken
- Submitted request changes on replacement PR #2134 at exact head
b0744b78232f80efdaab2a5d41ef3f0fe91faae7.
baixiaohang
left a comment
There was a problem hiding this comment.
Recommendation: request changes
- Rationale: Team confirmation can still borrow a pre-attempt
/mesnapshot, and an initial/metransport failure leaves the explicit Template flow stuck without recovery.
Risk level: B-high
- Path baseline:
packages/server/**is touched outside the high-risk core services, so the path baseline is B-low. - Semantic lift: the Web auth/data layer now governs cross-Team selection, query-cache rollback, and the admin WebSocket target, lifting the review to B-high.
PR summary
- Author / repo: yuezengwu / agent-team-foundation/first-tree
- Problem: Anonymous visitors need to discover official Agent Templates, preserve a chosen responsibility through sign-in, and create the resulting Agent in the intended Team without leaking private Template content or silently losing the intent.
- Approach: Adds public Library/detail routes, a strict canonical OAuth handoff, Team-aware onboarding and New Agent preselection, atomic submit-time Template adoption, safe analytics, and AA foreground tokens. The cross-Team path optimistically selects a Team and then uses
/meas confirmation authority before opening creation. - Impacted modules:
packages/shared, Server OAuth bootstrap, Web auth/Team selection, public Template pages, onboarding, New Agent creation, workspace chat routing, and design tokens.
Review findings
❌ 1. A /me request that started before selectOrganization can satisfy this switch's “fresh confirmation” requirement. loadMe derives the settled Team from the mutable selectedOrgIdRef when the response finishes and then increments meConfirmEpochRef; the catch path only checks that the epoch advanced after the attempt began. Therefore, if an older refreshMe() is already in flight, the switch optimistically writes Team B, that older response returns and confirms B, and the switch's own post-switch request fails, lines 445–449 still return success. The accepted membership snapshot may predate both the switch and a concurrent membership revocation, allowing the Template flow to hand off or open creation for a Team that was never confirmed by a post-attempt read. Track request-start identity/sequence and accept only a /me begun after this switch attempt (with the exact target), and cover the pre-existing-refresh ordering explicitly. [R4 / packages/web/src/auth/auth-context.tsx:311, packages/web/src/auth/auth-context.tsx:411]
❌ 2. A signed-in Template intent has no recovery when the initial /me request fails. loadMe sets meLoaded=true for an ordinary same-session transport error and fetchMe swallows the rejection; TemplateDetailPage treats that flag as sufficient authority and mounts TemplateUseIntent. With the provider's initial empty memberships and no organizationId, the page renders an empty chooser and a permanently disabled Continue button, with neither an error nor a retry path. Expose whether /me produced an authoritative snapshot (or surface its error) and keep this flow in a recoverable loading/error state until Team membership is known. [R4 / packages/web/src/auth/auth-context.tsx:367, packages/web/src/pages/templates/template-detail-page.tsx:152, packages/web/src/pages/templates/template-use-intent.tsx:191]
✅ The two findings carried from #2133 are otherwise addressed: unresolved explicit Template lookups now block plain submission until removal/unavailability, and old-session successful /me responses are discarded before mutating auth or Team state. The public-safe projection and one-time Team Resource import boundary also match the Context Tree contract.
Action taken
- Submitted request changes.
|
All requested changes are now available at exact head The repair closes the three reported gaps and two adjacent regressions:
Exact-head local gates are green: Please re-review the new head when available. |
baixiaohang
left a comment
There was a problem hiding this comment.
Recommendation: approve
- Rationale: The follow-up commit closes all three reported cross-Team and
/meauthority races without weakening the explicit-intent, public-projection, or one-time Team import boundaries.
Risk level: B-high
- Path baseline:
packages/server/**is touched outside the high-risk core services, so the path baseline is B-low. - Semantic lift: the Web auth/data layer governs cross-Team selection, query-cache rollback, and the admin WebSocket target, lifting the review to B-high.
PR summary
- Author / repo: yuezengwu / agent-team-foundation/first-tree
- Problem: Anonymous visitors need to discover an official Agent Template, preserve that responsibility through authentication, and create the resulting Agent in an explicitly confirmed Team without exposing private Template content or silently losing the intent.
- Approach: Adds the public Library/detail flow, strict OAuth intent preservation, authoritative Team resolution, per-Team onboarding or dialog handoff, and atomic submit-time Template adoption. Follow-up head
4ef18058c1441400cd1b35b4bdd4128692094ff7makes stale onboarding lookups commit-time invalid, accepts only post-attempt exact-target/meconfirmation, restores the full confirmed Team baseline on failure, and provides a guarded retry before any intent resolution mounts when initial Team authority is unavailable. - Impacted modules: Shared intent parsing, Server OAuth bootstrap, Web auth/Team selection, public Template pages, onboarding, New Agent creation, workspace routing, analytics, and design tokens.
Review findings
- ✅ The committed
{organizationId, slug}identity now owns onboarding lookup state; same-slug Team changes refetch and stale success or failure callbacks cannot cross the Team boundary. - ✅ Team switching accepts only a
/merequest begun after the attempt with the exact target, and a failed attempt restores selected, confirmed, persisted, API, WebSocket, and cache state. - ✅ Initial
/metransport failure remains outside chooser/dialog/handoff state and exposes a guarded retry until an authoritative membership snapshot succeeds. - ✅ The earlier explicit-intent fail-closed and stale-session success guards remain intact, and the public-safe projection plus one-time Team Resource import continue to match the Context Tree contract.
- ✅ Required GitHub checks are green at the reviewed exact head. This was a static re-review; I did not independently rerun the product checks or tests.
Action taken
- Approved exact head
4ef18058c1441400cd1b35b4bdd4128692094ff7.
Summary
/templatesLibrary and/templates/:slugdetail routes over the existing public-safe catalog projection, including active, retired/replacement, empty, missing, loading, and recoverable error states./templates/:slug?use=1through GitHub/Google login, while rejecting extra, duplicate, encoded, absolute, hash, backslash, trailing-separator, and trailing-slash variants./merequest that started after the attempt confirms the exact target membership; failed switches roll back every optimistic Team target, stale cross-session successes are discarded, and initial authority failure has an explicit Retry path.Replacement provenance
b0744b78232f80efdaab2a5d41ef3f0fe91faae7is authored and committed byyuezengwu, based onmainat2cf0c27dd17819a60160b7c0c69c447076da1f97. Its aggregate patch-id is4f39a40b0bc7bbb3bee3be25ea6c11e063419b05, exactly matching repaired headc881569b32d13739f7f88a17a66da1aba6303d6bfrom feat: add public Agent Template library and onboarding intent #2133.4ef18058c1441400cd1b35b4bdd4128692094ff7, also authored and committed byyuezengwu. No commit was amended and no branch was force-pushed. Kiven-FTE collaborated as the implementation agent; the submitting maintainer owns both replacement commits and the CLA record.Review findings addressed
/meresponses must match the live session generation and token subject before any auth or Team mutation./merequest started after that switch attempt; pre-existing refreshes cannot be borrowed as authority./metransport failure keeps canonical intent outside the chooser/dialog/handoff and exposes guarded Retry until an authoritative membership snapshot succeeds.{organizationId, slug}identity: layout-time invalidation closes the commit-to-passive-effect window, and same-slug Team changes restart the new Team's lookup while ignoring the old Team's late result.Boundaries
/?c=draft&with=<uuid>; a real first work loop remains a separate post-merge staging pilot.Validation
pnpm check— passed with 16 established warnings and no errors.pnpm typecheck --force— 11/11 tasks, 0 cached.pnpm build --force— production build 5/5 tasks, 0 cached.VITEST_MAX_FORKS=1 pnpm test --force --concurrency=1— 12/12 tasks, 0 cached; 10,129 tests passed / 4 skipped. Web 239 files / 2,095 tests; Server 258 / 2,984; Client 173 passed + 1 skipped files / 2,128 passed + 3 skipped tests; CLI 1,592 passed + 1 skipped; Shared 821; skill evals 505; QA 4.git diff --checkand clean exact-head worktree at4ef18058c1441400cd1b35b4bdd4128692094ff7.4ef18058c1441400cd1b35b4bdd4128692094ff7.The fresh QA run is asked to cover public discovery and leakage boundaries, canonical OAuth intent, pending/hung lookups in both creation surfaces, commit-window and same-slug cross-Team ownership, initial
/mefailure/Retry, pre-attempt and post-attempt Team authority, abort/401/logout/session replacement, ChatById retry bounds, Template and zero-Template atomic creation with API/DB readback, and all new WCAG surfaces and CTA states.Change Surface
apps/clipublic CLI or help outputNotes