ci: add web and platform CI/PR workflows#4
Merged
Conversation
- ci-web.yml: Deploy frontend on merge to main (web changes) - pr-web.yml: Lint + type check on PRs (web changes) - pr-platform.yml: Terraform plan on PRs (platform changes) All workflows also trigger on changes to themselves.
…y Federation - Fix React setState-in-effect error by using lazy initializer - Replace GCP_SA_KEY with GitHub Workload Identity Federation - Add OIDC permissions to all workflows - Add comprehensive setup documentation Changes: - web/src/lib/auth.tsx: Use lazy initializer for storedAuth state - .github/workflows/*.yml: Migrate to workload_identity_provider - docs/WORKLOAD_IDENTITY_SETUP.md: Complete setup guide
- Remove unused useEffect import from auth.tsx - Hardcode workload identity provider path - Hardcode service account email
Workflow changes: - Replace all GCP_PROJECT_ID vars with hardcoded vellum-ai-prod - Remove TF_VAR_project_id from terraform workflows Lint fixes: - Add eslint-disable for intentionally unused avatar_url destructure - Replace <img> with next/image <Image> in settings page - Remove unused eventType variable in FocusView - Remove unused catch parameter in FocusView - Add eslint-disable for onboardingState (future use) - Escape apostrophes in JSX text (') - Change isLoading from state to constant in auth - Prefix unused userId param with underscore in db.ts
…rors Workflow fixes: - Replace GCP_REGION var with hardcoded us-central1 in all workflows - Fix gcloud get-credentials commands TypeScript/Schema fixes: - Remove duplicate getDb() declaration in db.ts - Fix snake_case vs camelCase field access (created_by → createdBy, created_at → createdAt) - Cast agent.configuration as Record<string, any> to access nested properties (compute, agentmail, etc.) - Fix spread operator errors by casting configuration before spreading - Remove non-existent CreateAgentInput and UpdateAgentInput imports - Add unknown intermediate cast for postgres.js RowList to typed arrays - Fix all createChatMessage calls to use camelCase fields (agentId, gcsMessageId) All route files, components, and pages now align with Drizzle ORM schema types.
added 3 commits
February 8, 2026 02:06
- Type all catch block error parameters as 'unknown' instead of implicit 'any' - Remove GKE credentials step from pr-platform workflow - Migrate pr-platform to Workload Identity Federation - Hardcode GCP values: project=vellum-ai-prod, region=us-central1, cluster=vellum-ai-prod
…urrences) - Fixes all @typescript-eslint/no-explicit-any lint errors - Changes type assertions from 'any' to 'unknown' for agent.configuration casts
- Export CreateAgentInput and UpdateAgentInput from db.ts - Import types in agents/[id]/route.ts and agents/route.ts - Fixes TS2304 errors for missing type definitions
dvargasfuertes
approved these changes
Feb 8, 2026
5 tasks
ashleeradka
added a commit
that referenced
this pull request
Feb 14, 2026
Address remaining low-severity issues for production-ready shared library: 🔧 API Completeness (Issue #1): - Add public initializers to 17 remaining Encodable structs: • Skills: SkillsDisableMessage, SkillsConfigureMessage, SkillsInstallMessage, SkillsUninstallMessage, SkillsUpdateMessage, SkillsCheckUpdatesMessage, SkillsSearchMessage, SkillsInspectMessage • Apps: AppsListRequestMessage, SharedAppsListRequestMessage, SharedAppDeleteRequestMessage, BundleAppRequestMessage, OpenBundleMessage • Session: SessionListRequestMessage, HistoryRequestMessage • Signing: SignBundlePayloadResponseMessage, GetSigningIdentityResponseMessage - Ensures consistent public API surface for future iOS code 🛡️ Platform Safety (Issue #4): - Add #else fallback in DaemonClient.connect() with #error directive - Prevents silent compilation failures on unsupported platforms (visionOS, watchOS) - Clear compile-time error: "DaemonClient is only supported on macOS and iOS" 📝 Documentation (Issue #5): - Update DaemonClient doc comment to describe both connection types: • macOS: Unix domain socket at ~/.vellum/vellum.sock • iOS: TCP to configurable hostname:port via UserDefaults - Accurately reflects shared library's cross-platform design ✅ Build: passes in 3.70s ✅ All structs now have consistent public init coverage ✅ Future-proof against unsupported platform builds Note: iOS signing request handling (Issue #3) remains logged-only since response messages lack error fields. Proper fix requires daemon-side changes to avoid sending these requests to iOS clients. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
ashleeradka
added a commit
that referenced
this pull request
Feb 14, 2026
…riction Issue #5 (Low severity): - Make SendError enum public for cross-module error handling - Make errorDescription public for LocalizedError protocol conformance - Enables consumers to pattern-match on specific error cases (e.g., .notConnected) Issue #4 (Low severity - documentation): - Add comment clarifying VellumAssistantLib is macOS-only - Links macOS-specific frameworks (AppKit, ScreenCaptureKit, etc.) - iOS apps should depend only on VellumAssistantShared Note: Issue #1 (build script) is NOT a problem - SPM automatically searches parent directories for Package.swift. Build script works correctly as-is. ✅ Build: 4.65s Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
ashleeradka
added a commit
that referenced
this pull request
Feb 14, 2026
…ound Issue #1 (Consistency): - Add #else clause to signing operation switch cases in handleServerMessage - Ensures all platforms have explicit handling (logs error on unsupported platforms) - Complements the #error directive in connect() for defense in depth Issue #3 (Type inference fragility): - Replace .init shorthand with fully qualified ConfirmationAllowlistOption type - Consistent with other preview blocks in ToolConfirmationBubble.swift - Prevents breakage if type inference context changes Issue #7 (Build robustness): - Remove empty Resources directory and .process("Resources") declaration - Eliminates fragile .gitkeep workaround (accidental deletion would break builds) - VellumAssistantShared has no resources yet; can add back when needed Issues #2, #4, #5, #6 (Not actionable in this PR): - #2: VellumAssistantLib platform restriction documented via comment (SPM limitation) - #4: @mainactor deinit data race is pre-existing, not introduced by this PR - #5: iOS signing hang acknowledged, needs daemon-side fix - #6: JSON camelCase/snake_case mixed convention is existing protocol, not changing ✅ Build: 5.83s ✅ More robust against platform/type inference edge cases ✅ Removed unnecessary .gitkeep workaround Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
ashleeradka
added a commit
that referenced
this pull request
Feb 14, 2026
…1821) * Create VellumAssistantShared library target for code reuse - Add multi-platform library target to Package.swift (macOS .v14, iOS .v17) - Move IPC layer (DaemonClient, IPCMessages) to shared library - Add platform-specific connection logic to DaemonClient: - macOS: Unix domain socket at ~/.vellum/vellum.sock - iOS: TCP endpoint (configured via UserDefaults) - Make all IPC types public with necessary initializers - Update macOS app to import VellumAssistantShared in all files using IPC - Add public initializers for Decodable messages used in tests Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Restructure directory layout for multi-platform support Move to flat structure at clients/ level: - Move Package.swift from clients/macos/ to clients/ - Move vellum-assistant-shared/ to clients/shared/ - Add .gitignore at clients/ level to exclude .build/ - Update Package.swift paths to reference shared/ and macos/ This prepares for iOS app in clients/ios/ without nesting everything under an Apple-specific directory. Final structure: clients/ Package.swift (Swift package for all Apple platforms) .gitignore (ignore .build, .swiftpm, etc) shared/ (VellumAssistantShared target) macos/ (macOS app) ios/ (future iOS app) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Address PR feedback: fix Resources directory and port overflow Fixes based on Devin and Codex review comments: 1. Create shared/Resources directory with .gitkeep - Package.swift declares resources: [.process("Resources")] - Git doesn't track empty directories, causing fresh clone failures - Added .gitkeep to ensure directory is tracked 2. Guard iOS daemon port conversion from UserDefaults overflow - Previous: Direct cast Int -> UInt16 can crash if value >65535 or negative - Fixed: Use UInt16(clamping:) with validation (0-65535 range) - Falls back to 8765 for invalid values - Prevents runtime crash from malformed persisted settings Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Add missing public initializers and properties - Add public init to TaskSubmitMessage - Make SessionItem and HistoryMessageItem properties public - Make HistoryToolCallItem properties public Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix access control and iOS compatibility issues in shared IPC layer Address critical compilation issues and improve iOS platform handling: 🔴 Critical fixes: - Make SkillSummaryItem typealias public for cross-module access - Make UpdateInfo properties (name, installedVersion, latestVersion) public 🟡 Medium severity fixes: - Add public initializers to all Encodable IPC message structs: • PingMessage, SkillsEnableMessage • ConfirmationResponseMessage, SecretResponseMessage • AddTrustRuleMessage, TrustRulesListMessage • RemoveTrustRuleMessage, UpdateTrustRuleMessage - Add explicit error logging on iOS for unsupported signing operations (signBundlePayload, getSigningIdentity) instead of silently dropping These changes ensure the shared library has consistent public API surface for both macOS and iOS targets, and makes iOS behavior more transparent. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Complete public API consistency and add platform safety guards Address remaining low-severity issues for production-ready shared library: 🔧 API Completeness (Issue #1): - Add public initializers to 17 remaining Encodable structs: • Skills: SkillsDisableMessage, SkillsConfigureMessage, SkillsInstallMessage, SkillsUninstallMessage, SkillsUpdateMessage, SkillsCheckUpdatesMessage, SkillsSearchMessage, SkillsInspectMessage • Apps: AppsListRequestMessage, SharedAppsListRequestMessage, SharedAppDeleteRequestMessage, BundleAppRequestMessage, OpenBundleMessage • Session: SessionListRequestMessage, HistoryRequestMessage • Signing: SignBundlePayloadResponseMessage, GetSigningIdentityResponseMessage - Ensures consistent public API surface for future iOS code 🛡️ Platform Safety (Issue #4): - Add #else fallback in DaemonClient.connect() with #error directive - Prevents silent compilation failures on unsupported platforms (visionOS, watchOS) - Clear compile-time error: "DaemonClient is only supported on macOS and iOS" 📝 Documentation (Issue #5): - Update DaemonClient doc comment to describe both connection types: • macOS: Unix domain socket at ~/.vellum/vellum.sock • iOS: TCP to configurable hostname:port via UserDefaults - Accurately reflects shared library's cross-platform design ✅ Build: passes in 3.70s ✅ All structs now have consistent public init coverage ✅ Future-proof against unsupported platform builds Note: iOS signing request handling (Issue #3) remains logged-only since response messages lack error fields. Proper fix requires daemon-side changes to avoid sending these requests to iOS clients. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix SendError visibility and clarify VellumAssistantLib platform restriction Issue #5 (Low severity): - Make SendError enum public for cross-module error handling - Make errorDescription public for LocalizedError protocol conformance - Enables consumers to pattern-match on specific error cases (e.g., .notConnected) Issue #4 (Low severity - documentation): - Add comment clarifying VellumAssistantLib is macOS-only - Links macOS-specific frameworks (AppKit, ScreenCaptureKit, etc.) - iOS apps should depend only on VellumAssistantShared Note: Issue #1 (build script) is NOT a problem - SPM automatically searches parent directories for Package.swift. Build script works correctly as-is. ✅ Build: 4.65s Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix platform coverage consistency and remove fragile Resources workaround Issue #1 (Consistency): - Add #else clause to signing operation switch cases in handleServerMessage - Ensures all platforms have explicit handling (logs error on unsupported platforms) - Complements the #error directive in connect() for defense in depth Issue #3 (Type inference fragility): - Replace .init shorthand with fully qualified ConfirmationAllowlistOption type - Consistent with other preview blocks in ToolConfirmationBubble.swift - Prevents breakage if type inference context changes Issue #7 (Build robustness): - Remove empty Resources directory and .process("Resources") declaration - Eliminates fragile .gitkeep workaround (accidental deletion would break builds) - VellumAssistantShared has no resources yet; can add back when needed Issues #2, #4, #5, #6 (Not actionable in this PR): - #2: VellumAssistantLib platform restriction documented via comment (SPM limitation) - #4: @mainactor deinit data race is pre-existing, not introduced by this PR - #5: iOS signing hang acknowledged, needs daemon-side fix - #6: JSON camelCase/snake_case mixed convention is existing protocol, not changing ✅ Build: 5.83s ✅ More robust against platform/type inference edge cases ✅ Removed unnecessary .gitkeep workaround Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix @mainactor deinit data race in DaemonClient Issue: Swift concurrency bug (pre-existing, but now fixed) In Swift 5.9+, deinit on a @mainactor class is NOT guaranteed to run on the main actor. When the last reference to DaemonClient is dropped from a background thread, deinit could run on that thread, causing data races when accessing main-actor-isolated properties (shouldReconnect, connection, subscribers, etc.). Solution: Wrap deinit body in MainActor.assumeIsolated { } This ensures: - Safe access to main-actor-isolated state during cleanup - Trap with clear error if deinit somehow runs on wrong thread - Better than silent undefined behavior from data race Why fix in this PR: - Already heavily modifying DaemonClient for shared library - Real correctness issue (undefined behavior, potential crashes) - Simple fix with clear semantics - Makes shared library more robust for both macOS and iOS Reference: SE-0327 (Actors and Deinitializers) ✅ Build: 1.27s ✅ Eliminates undefined behavior from concurrent deinit Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix build script clean paths and revise deinit approach Issue #1 (🟡 Medium): Build script clean commands broken - .build directory moved from clients/macos/.build to clients/.build - build.sh clean and release pre-clean still referenced old location - Result: Clean commands did nothing, stale artifacts persisted Fix: Update both clean commands to use $SCRIPT_DIR/../.build - Line 64: Clean command now removes correct directory - Line 83: Release pre-clean now removes correct directory Issue #2 (🔴 High): MainActor.assumeIsolated crash risk in deinit - Previous fix (9231d63) used assumeIsolated which crashes if deinit runs on background thread - While assumeIsolated reveals lifecycle bugs, it's too aggressive for cleanup code that should never crash Fix: Revert to direct access pattern with detailed justification - Task.cancel(), NWConnection.cancel(), Continuation.finish() are all thread-safe - Data races on shouldReconnect and subscribers are benign in deinit (object is being destroyed, no other access possible) - Added comprehensive comment explaining the tradeoff Tradeoff analysis: ✅ No crashes during deallocation ✅ Clean command actually works now ✅ Release builds get proper artifact cleanup⚠️ Technically a data race, but benign for cleanup-only code⚠️ Won't trap on incorrect lifecycle (silent vs. loud failure) Devin review feedback addressed (both comments from 07:40-07:45) ✅ Build: 1.22s Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Add explicit Network framework linking to VellumAssistantShared Issue: VellumAssistantShared imports Network (DaemonClient uses NWConnection) but doesn't explicitly link the Network framework. Currently works because VellumAssistantLib links Network, but future iOS targets depending only on VellumAssistantShared would fail. Fix: Add linkerSettings with Network framework to VellumAssistantShared target While SPM usually auto-links system frameworks on import, explicit linking ensures robust behavior when iOS app target depends only on the shared library (not on VellumAssistantLib which is macOS-only). ✅ Build: 16.92s (full rebuild) ✅ Explicit framework dependencies documented ✅ Future-proof for iOS app target Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Add clients/ directory README for code organization documentation Created comprehensive README.md at clients/ level to document: - Directory structure and target organization - VellumAssistantShared vs VellumAssistantLib vs executable - Platform-specific vs shared code strategy - Build instructions and development guidelines - Migration context from single-platform structure - Known limitations (iOS TCP, signing, localhost) Helps developers and other agents understand: - Why Package.swift is at clients/ level - What code goes in shared/ vs macos/ vs ios/ - How to add new shared/platform-specific code - ~45-50% code reuse strategy Valuable for PR reviewers and future iOS development (PRs 2-13). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Add imageData field to ToolResultMessage for protocol completeness The imageData field was added to ToolResultMessage in main (commit ea7c3d7) for browser screenshot support (PR #1864) but was inadvertently dropped during the migration to the shared library. This field enables the daemon to send base64-encoded image data from tool contentBlocks. While not currently consumed by the macOS client, adding it ensures the shared library's protocol definition matches the daemon's implementation. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Add missing public initializers to IPC message types Added public initializers to 7 Decodable message types that are used in test code. These types need explicit public initializers because Swift's auto-generated memberwise initializers are internal by default: - AssistantThinkingDeltaMessage - CuErrorMessage - CuCompleteMessage - CuActionMessage - GenerationHandoffMessage - MessageDequeuedMessage - GenerationCancelledMessage - ErrorMessage - MessageQueuedMessage Without these, tests that construct messages directly (rather than decoding from JSON) fail with "incorrect argument label" errors. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
5 tasks
ashleeradka
added a commit
that referenced
this pull request
Feb 14, 2026
Fix issues #1-3 from manual code review: 1. **Eliminate duplicated socket path resolution logic**: Extract resolveSocketPath() as a standalone function at module level. DaemonConfig.default now delegates to it (3 lines vs 14). DaemonClient.resolveSocketPath() also delegates for backward compatibility with existing tests. 2. **Handle empty hostname from UserDefaults**: Use .flatMap to treat empty string as nil, ensuring fallback to "localhost". Prevents connection attempts to empty hostname if user sets daemon_hostname="". 3. **Add @mainactor to iOS AppDelegate**: Consistent with macOS AppDelegate and required for strict concurrency checking since DaemonClient is @MainActor-isolated. Issues #4-6 acknowledged but not addressed in this PR: - #4: Silently ignored connection errors (acceptable for scaffold) - #5: iOS code under macos/ directory (can refactor later if needed) - #6: Platform-specific DaemonConfig (intentional design tradeoff) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
ashleeradka
added a commit
that referenced
this pull request
Feb 15, 2026
1. Add deleteThread() method for permanent thread deletion - Allows users to clean up hidden threads in drawer mode - Prevents indefinite accumulation of hidden threads 2. Fix hidden thread row click behavior - Removed .tag() from hidden thread rows - Only explicit restore button triggers restoration now 3. Optimize duplicate filter computation - Extract hiddenThreads to local variable - Avoid computing filter twice 4. Add delete button to hidden threads UI - Trash icon alongside restore button - Provides permanent cleanup option Fixes issues #2, #3, and #4 from review feedback. Issue #1 (sessionId data loss) was already fixed in previous commit. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
ashleeradka
added a commit
that referenced
this pull request
Feb 15, 2026
* Add thread drawer mode as alternative to tab layout Implements a NavigationSplitView-based drawer for accessing chat history, with a toggle in Display settings to switch between tab and drawer modes. ## Key Features - New "Show thread list drawer" toggle in Settings > Display - Drawer mode shows threads in a collapsible sidebar (NavigationSplitView) - Thread hiding system: X button hides threads instead of deleting - Hidden threads shown in collapsible "HIDDEN" section at bottom - Restore button (↺) to unhide threads in drawer mode - Consistent button behavior across both modes (icon-only toolbar buttons) ## Implementation Details - ThreadModel: Added `isHidden` property to track hidden threads - ThreadManager: Modified `closeThread()` to mark threads as hidden - ThreadManager: Added `showThread()` to restore hidden threads - MainWindowView: Conditional layout based on `useThreadDrawer` setting - ChatView: Reduced top padding in drawer mode for better spacing - Both modes now filter hidden threads from main list ## Technical Choices - Reuses existing NavigationSplitView component (no custom drawer) - Leverages ThreadManager's existing thread state management - Minimal code duplication between tab and drawer modes - Uses SwiftUI's native collapsible Section for hidden threads Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix thread drawer review issues - Sync selectedThreadId back from activeThreadId to keep sidebar selection in sync - Clean up ChatViewModels when threads are hidden to prevent memory leaks - Recreate ChatViewModels when hidden threads are restored - Use selectThread for visible threads instead of always calling showThread Addresses review feedback from devin-ai-integration. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Address PR review feedback — thread management improvements 1. Add deleteThread() method for permanent thread deletion - Allows users to clean up hidden threads in drawer mode - Prevents indefinite accumulation of hidden threads 2. Fix hidden thread row click behavior - Removed .tag() from hidden thread rows - Only explicit restore button triggers restoration now 3. Optimize duplicate filter computation - Extract hiddenThreads to local variable - Avoid computing filter twice 4. Add delete button to hidden threads UI - Trash icon alongside restore button - Provides permanent cleanup option Fixes issues #2, #3, and #4 from review feedback. Issue #1 (sessionId data loss) was already fixed in previous commit. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix sessionId immutability and improve VSplitView transitions 1. ThreadModel.swift: Change sessionId from let to var - Fixes critical data loss bug where sessionId couldn't be persisted - Enables chat history restoration when threads are unhidden 2. VSplitView.swift: Improve split-view behavior - Change from overlay to side-by-side HStack layout - Panel now pushes content aside instead of covering it - Smoother spring animation (0.3s response, 0.8 damping) - Better padding (vertical + leading only) - Add clarifying comments These changes address the sessionId data loss issue (#1) and improve right panel UX per user feedback. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Restore original tab mode delete behavior Devin correctly identified that closeThread() now hides threads in both modes, but tab mode originally deleted them permanently. This is a behavioral regression. Fix: Make closeThread() conditional based on useThreadDrawer setting: - Tab mode (useThreadDrawer = false): Delete threads permanently (original) - Drawer mode (useThreadDrawer = true): Hide threads (new feature) This preserves the original tab behavior while enabling the new drawer restoration feature only where it's accessible via UI. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Revert to original tab behavior - remove hide/restore feature Remove all hidden thread functionality to match original behavior: - Removed isHidden field from ThreadModel - Removed showThread() and deleteThread() methods from ThreadManager - Restored original simple closeThread() that removes from memory - Removed HIDDEN section from drawer mode UI - Removed isHidden filters from thread lists Original behavior preserved: - X button removes thread from memory - Threads restored on app restart from daemon sessions (up to 5 most recent) - Both tab and drawer modes now have identical thread lifecycle This simplifies the implementation and matches the original behavior exactly. Hide/restore can be added as a separate feature later. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix toolbar flash and address Devin review issues 1. Disable toolbar animations during NavigationSplitView transitions - Add .transaction modifier to disable animations - Wrap columnVisibility changes in withTransaction - Prevents toolbar flash when toggling drawer 2. Hide X button on last thread in drawer mode - Conditionally show close button only when threads.count > 1 - Matches tab mode behavior (ThreadTabBar) - Prevents non-functional button clicks 3. Fix VSplitView trailing padding - Change from .padding(.leading) to .padding(.horizontal) - Ensures right-side rounded corners aren't clipped - Provides proper spacing on both sides Addresses Devin review issues from commit c4cb938. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Enable drawer slide animation while preventing toolbar flash Changed approach to fix toolbar flash: - Removed broad .transaction modifier that disabled all animations - Added .animation(nil, value: columnVisibility) to toolbar HStack only - NavigationSplitView sidebar now slides in/out smoothly - Toolbar no longer flashes during sidebar transitions This gives us the best of both worlds: smooth drawer animation and stable toolbar positioning. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix root cause of toolbar flash - disable automatic sidebar toggle The "extra button" flash was caused by NavigationSplitView automatically adding a native sidebar toggle button to the toolbar. When the sidebar opened/closed, macOS would reposition/show/hide this button, causing our toolbar buttons to shift and flash. Fix: Add .toolbar(removing: .sidebarToggle) to disable the automatic sidebar toggle button that NavigationSplitView adds by default. Also removed the .animation(nil) workaround since we're now fixing the root cause instead of hiding the symptom. This is the proper fix - no animation suppression needed. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Style drawer mode panels and fix layout animations - Add panel styling to chat content (rounded corners, consistent background) - Remove top padding from panels so they extend to button bar - Add smooth spring animation to HStack to prevent jumping when drawer opens/closes - Match ThreadTabBar layout pattern with VStack wrapper Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix drawer reopening and VSplitView padding issues - Reopen thread drawer when closing right-side panel (fixes drawer staying hidden) - Revert VSplitView padding to exclude top padding (panels flush with button bar) Addresses Devin review feedback on PR #2150 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Clean up code quality issues - Remove duplicate .ignoresSafeArea(edges: .top) in drawer mode - Merge duplicate .onAppear blocks into single initialization - Add comment explaining 78pt padding for traffic light buttons - Change ThreadModel.sessionId back to let (no longer mutated) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix shared component regressions and styling issues - Make chat panel styling conditional on drawer mode (fixes tab mode regression) - Revert VSplitView animation to VAnimation.standard (shared component fix) - Extract trafficLightPadding constant with detailed documentation Addresses critical review feedback: tab mode no longer gets unintended background/clipping, and VSplitView animation is consistent across all uses. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix drawer not preserving closed state when panel closes When Settings (or any panel) was opened with drawer already closed, closing the panel would incorrectly reopen the drawer. Now tracks previous drawer state and only reopens if it was open before. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Remove automatic drawer/panel linking behavior Drawer and panels now only open/close via manual toggle - no automatic closing of drawer when panel opens, no automatic reopening. Both are completely independent. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix thread item close button gesture conflict Restructured threadItem to use Button-based approach like ThreadTab, with close button in overlay instead of nested in HStack. This prevents onTapGesture from intercepting close button clicks. Fixes Devin review feedback. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
4 tasks
Jasonnnz
pushed a commit
that referenced
this pull request
Feb 23, 2026
P1 #3: Add conversationId to TaskSubmit so direct CU QA sessions can set reportToSessionId. Client passes active thread ID. Daemon routes it to CU session metadata, enabling attachment creation on finalize. P1 #4: Add retentionDays to TaskRouted from daemon config. Client reads it instead of hardcoding 7 days. Also: create file-backed attachment for recordings without reportToSessionId so cleanup can track orphan files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
6 tasks
Jasonnnz
added a commit
that referenced
this pull request
Feb 23, 2026
…ays (#6951) P1 #3: Add conversationId to TaskSubmit so direct CU QA sessions can set reportToSessionId. Client passes active thread ID. Daemon routes it to CU session metadata, enabling attachment creation on finalize. P1 #4: Add retentionDays to TaskRouted from daemon config. Client reads it instead of hardcoding 7 days. Also: create file-backed attachment for recordings without reportToSessionId so cleanup can track orphan files. Co-authored-by: Vellum Assistant <assistant@vellum.ai> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Jasonnnz
added a commit
that referenced
this pull request
Feb 23, 2026
…ays (#6951) P1 #3: Add conversationId to TaskSubmit so direct CU QA sessions can set reportToSessionId. Client passes active thread ID. Daemon routes it to CU session metadata, enabling attachment creation on finalize. P1 #4: Add retentionDays to TaskRouted from daemon config. Client reads it instead of hardcoding 7 days. Also: create file-backed attachment for recordings without reportToSessionId so cleanup can track orphan files. Co-authored-by: Vellum Assistant <assistant@vellum.ai> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
tkheyfets
pushed a commit
that referenced
this pull request
Mar 2, 2026
* feat: add guardianPrincipalId to schema and storage plumbing (#10965) Co-authored-by: Claude <noreply@anthropic.com> * M2: Backfill + startup invariants for guardian principal (#10969) * feat: backfill guardianPrincipalId and enforce startup invariants Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: restrict backfill expiration to expired requests and tighten desktop rebinding predicate Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * feat: propagate guardianPrincipalId through runtime contexts (#10978) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * M4: Bind principal at all canonical request creation sites (#10980) * feat: bind guardianPrincipalId at all canonical request creation sites Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: add try/catch guards around createCanonicalGuardianRequest in all caller paths Wrap createCanonicalGuardianRequest calls in try/catch in three locations to handle IntegrityError when guardianPrincipalId is missing: 1. daemon/server.ts (makePendingInteractionRegistrar) - prevents crash when session.guardianContext is undefined during tool approval events 2. runtime/routes/conversation-routes.ts (makeHubPublisher) - same pattern for the HTTP hub publisher path 3. runtime/access-request-helper.ts - preserves the intentional no-binding fallback path (documented at file header) where access requests proceed without guardian identity All catch blocks log at debug level matching the existing pattern in daemon/handlers/sessions.ts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: add try/catch in inbound-message-handler and fix misleading notified:true in access-request-helper - Wrap createCanonicalGuardianRequest call in inbound-message-handler.ts ingress escalation path with try/catch to prevent unhandled IntegrityError from crashing the HTTP handler with a 500. On failure, logs a warning and continues to the notification pipeline. - Fix catch block in access-request-helper.ts to return notified: false instead of notified: true, since the emitNotificationSignal call is never reached when the error is caught. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: pass guardianPrincipalId to all createBinding callsites Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * M5: Cutover decision authorization + remove isTrusted (#10990) * feat: cutover decision authorization to principal-based and remove isTrusted Replace the isTrusted compatibility bypass in all runtime types and callsites with principal-based authorization. Decision authorization is now purely principal-based: actor.guardianPrincipalId must match request.guardianPrincipalId for any applied decision. There is no longer a trusted bypass path. Key changes: - Remove isTrusted from ActorContext type, replace with guardianPrincipalId - Add three-step principal validation in applyCanonicalGuardianDecision() - Add guardianPrincipalId filter to canonical guardian store queries - Update guardian-reply-router to use principal-based request discovery - Update all callsites (HTTP routes, daemon handlers, session process, inbound message handler) to resolve and pass guardianPrincipalId - Replace isTrusted-based guardianReplyText conditionals with channel checks Closes #10960 * fix: resolve cross-channel principal mismatch and non-vellum channel fallback - conversation-routes.ts: Fall back to session.guardianContext for verifiedActorExternalUserId and verifiedActorPrincipalId when actorVerification is null (non-vellum channels). - guardian-decision-primitive.ts: Allow cross-channel guardian decisions by checking actor principal against the assistant's canonical vellum binding principal when direct principal match fails. - guardian-reply-router.ts: Add guardianPrincipalId filter to the conversation fallback query in findPendingCanonicalRequests so guardians only see requests they are authorized to act on. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: add cross-channel principal fallback to code-only clarification check Extract isAuthorizedGuardianPrincipal() shared helper from the inline cross-channel fallback logic in applyCanonicalGuardianDecision, and use it in the router's code-only clarification identity check. This ensures a desktop guardian entering a request code for a cross-channel request (e.g. Telegram-originated) sees the clarification context instead of getting "Request not found". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * feat: M6 — tests + guardrails + cleanup for principal-based auth (#10961) (#11002) * feat: M6 — tests + guardrails + cleanup for principal-based auth (#10961) - Update 6 test files to use guardianPrincipalId principal-match pattern instead of legacy isTrusted boolean - Add guard test (no-is-trusted-guard.test.ts) to prevent isTrusted reintroduction in production code - Clean up 3 stale comments referencing trusted bypass and desktop/trusted with principal-based terminology - Verify no production isTrusted references remain (only allowed trust-class variable names: isTrustedActor, isTrustedContact, isTrustedTrustClass) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: correct guard test pipe and identity mismatch test field Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: use JS-level allowlist filtering in guard test to prevent masking --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * fix: address holistic review — decidedByPrincipalId, access_request exempt, backfill expiry, code lookup guard Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: make isAuthorizedGuardianPrincipal symmetric for cross-channel approval Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address reviewer feedback — binding check, pre-bootstrap IPC, migration access_request exclusion - isAuthorizedGuardianPrincipal: verify actor's principal belongs to an active guardian binding before allowing cross-channel approval, closing the asymmetric authorization gap (Devin #3/#4) - local-actor-identity: eagerly create vellum binding via ensureVellumGuardianBinding in pre-bootstrap IPC path so downstream decisionable requests always have a guardianPrincipalId (Devin #5) - Migration 126: exclude access_request from expiry sweep in steps 3a and 3c since access_request is non-decisionable and proceeds via the invite flow (Codex P2 #2) - Update roundtrip tests to supply guardianPrincipalId for decisionable tool_approval requests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: unify channel binding principals + fix test fixtures for IntegrityError guard Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: align migration registry checkpoint key with migration function (_v1 -> _v2) * fix: narrow CanonicalDecisionResult union before accessing resolverReplyText --------- Co-authored-by: Claude <noreply@anthropic.com>
4 tasks
5 tasks
Jasonnnz
pushed a commit
that referenced
this pull request
Apr 2, 2026
…ld auto-focus, remove debug prints - Block keyboard activation (Enter/Space) for disabled menu items by tracking isEnabled state per item in the coordinator (#1, #2) - Include panel level in focusChangeSubject so fallback-path focus changes only affect the correct VMenu instance, preventing parent corruption (#4) - Replace synchronous moveFocus into child (which raced SwiftUI preference propagation) with a pendingChildFocus flag consumed by the child VMenu's .task after layout settles (#3, #6) - Remove all print("[VMenuKeyboard]...") debug statements (#5) - Forward isKeyboardFocused in VNavItemTrailingIcon convenience init (#7) Co-Authored-By: Claude <noreply@anthropic.com>
Jasonnnz
added a commit
that referenced
this pull request
Apr 2, 2026
…3277) * Implement M3 keyboard focus system for VMenu components - Convert VMenuCoordinator to @observable for reactive focus state tracking - Add UUID-based item registration system (VMenuItemRegistrationKey preference key) - Implement keyboard activation (Enter/Space) and submenu opening (right arrow) - Add VoiceOver bridge: post focusedUIElementChanged on keyboard focus moves - Install mouse move monitor to clear keyboard focus (matches native NSMenu) - Add visual keyboard focus highlight to VMenuItem, VSubMenuItem, and VNavItem - Add VMenuItemNSViewCapture (NSViewRepresentable) for VoiceOver NSView tracking - Inject panel level via environment for multi-level keyboard navigation - Add isKeyboardFocused parameter to VNavItem for .regular size menu items Co-Authored-By: Jason Zhou <jasonczhou3@gmail.com> * Fix build: remove #if os(macOS) from function parameter lists Swift does not support conditional compilation directives inside function parameter lists or call sites. Make isKeyboardFocused available on all platforms (defaults to false, no-op on iOS) instead of conditionally compiling it. Use an immediately-invoked closure for the VMenu call site to conditionally pass the value. Co-Authored-By: Jason Zhou <jasonczhou3@gmail.com> * Fix keyboard focus: debounce mouse monitor + fallback item registration Three fixes for arrow keys not highlighting menu items: 1. Mouse move monitor debounce: Ignore mouse movements within 200ms of a keyboard event to prevent trackpad/mouse micro-jitter from clearing the focus highlight before SwiftUI renders it. 2. Belt-and-suspenders item registration: registerItemAction() now also appends the item to itemOrder/itemCounts as a fallback, ensuring moveFocus() has a non-zero count even if onPreferenceChange hasn't fired yet. 3. Timestamp tracking: handleKeyDown() records lastKeyboardEventTime before processing, used by the mouse move debounce guard. Co-Authored-By: Jason Zhou <jasonczhou3@gmail.com> * Add local key event monitor for reliable keyboard focus The panel's keyDown override was not receiving key events — likely because NSHostingView intercepts them in the responder chain, or addChildWindow changes key window status away from the panel. Fix: Install NSEvent.addLocalMonitorForEvents(matching: .keyDown) on the coordinator when the root panel opens. This intercepts key events BEFORE the responder chain, the same approach native NSMenu uses. The monitor returns nil to consume handled events (arrow keys, Enter, Space) and passes through all other keys. Reference: https://developer.apple.com/documentation/appkit/nsevent/addlocalmonitorforevents(matching:handler:) Co-Authored-By: Jason Zhou <jasonczhou3@gmail.com> * Switch to SwiftUI-native .focusable() + .onKeyPress() for keyboard navigation Root cause: previous attempts used AppKit NSEvent monitors to catch key events then tried to bridge state changes back to SwiftUI via @observable or Combine. This AppKit→SwiftUI bridge is fundamentally fragile — SwiftUI may not re-render when coordinator properties change through external event monitors. Fix: Use Apple's recommended approach from WWDC23 'The SwiftUI cookbook for focus' (https://developer.apple.com/videos/play/wwdc2023/10162/): 1. VMenu's VStack is .focusable() with .onKeyPress() handlers — key events are handled entirely within SwiftUI's rendering cycle. 2. Focus state is VMenu's @State focusedItemID — guaranteed re-renders. 3. focusedItemID flows to children via environment (vMenuFocusedItemID). 4. VMenuItem/VSubMenuItem read from environment — no Combine needed. 5. NSEvent keyboard monitor only tracks timing (for mouse-move debounce), does NOT consume events. 6. VMenuPanel.keyDown remains as fallback if VMenu loses focus — it calls coordinator.handleKeyDown which publishes via Combine, and VMenu subscribes to update its @State. References: - https://developer.apple.com/videos/play/wwdc2023/10162/ - https://developer.apple.com/documentation/swiftui/view/focusable(_:interactions:) - https://developer.apple.com/documentation/swiftui/view/onkeypress(_:action:)-14rhi Co-Authored-By: Jason Zhou <jasonczhou3@gmail.com> * Add missing 'import Combine' for Empty publisher fallback Co-Authored-By: Jason Zhou <jasonczhou3@gmail.com> * Fix Enter/Space/arrows: registerRootPanel was wiping actions after .onAppear Root cause: VMenuPanel.show() calls registerRootPanel() AFTER NSHostingView triggers SwiftUI layout, which means .onAppear has already fired and registered item actions. registerRootPanel() then clears itemActions/submenuActions/itemNSViews, permanently wiping the registered actions (.onAppear only fires once). Up/Down worked because VMenu uses its own @State registeredIDs (populated by async onPreferenceChange which fires after registerRootPanel). But Enter/Space/Right/Left depend on coordinator.itemActions which was being wiped. Fix: Only reset transient navigation state (focusedIndex) in registerRootPanel. The coordinator is freshly created per panel tree, so action dictionaries start empty and don't need clearing. Also: Change keyboard focus highlight from Color.accentColor (blue) to VColor.systemPositiveWeak (light green design system token). Co-Authored-By: Jason Zhou <jasonczhou3@gmail.com> * Address review feedback: disabled item guard, level-scoped focus, child auto-focus, remove debug prints - Block keyboard activation (Enter/Space) for disabled menu items by tracking isEnabled state per item in the coordinator (#1, #2) - Include panel level in focusChangeSubject so fallback-path focus changes only affect the correct VMenu instance, preventing parent corruption (#4) - Replace synchronous moveFocus into child (which raced SwiftUI preference propagation) with a pendingChildFocus flag consumed by the child VMenu's .task after layout settles (#3, #6) - Remove all print("[VMenuKeyboard]...") debug statements (#5) - Forward isKeyboardFocused in VNavItemTrailingIcon convenience init (#7) Co-Authored-By: Claude <noreply@anthropic.com> * Address review round 2: block disabled submenus, register VSubMenuItem enabled state, fix mouse monitor - Add isItemEnabled check to right-arrow paths (both primary openMenuSubmenu and fallback handleKeyDown case 124) so disabled VSubMenuItems can't be opened via keyboard - Register VSubMenuItem enabled state with coordinator on appear and onChange(of: isEnabled), matching VMenuItem's existing pattern - Set acceptsMouseMovedEvents = true on VMenuPanel so the coordinator's local mouse-move monitor fires when the cursor is over the panel, allowing keyboard focus to be properly cleared Co-Authored-By: Claude <noreply@anthropic.com> * Use disabled color for menu item icons when isEnabled is false VMenuItem and VSubMenuItem icons now use VColor.contentDisabled when the item is disabled, matching the text color behavior. Previously icons kept their normal color (primaryBase/primaryActive) even when disabled, making it harder to visually identify disabled items. Co-Authored-By: Claude <noreply@anthropic.com> * Guard against Task cancellation after sleep in VMenu .task Add `guard !Task.isCancelled else { return }` after `Task.sleep` to prevent stale state mutation when the view is dismissed before the 50ms delay completes. Without this, a cancelled task could consume the pendingChildFocus flag meant for a subsequent child menu. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Jason Zhou <jasonczhou3@gmail.com> Co-authored-by: Vellum Assistant <assistant@vellum.ai> Co-authored-by: Claude <noreply@anthropic.com>
6 tasks
Merged
5 tasks
This was referenced May 3, 2026
2 tasks
dvargasfuertes
pushed a commit
that referenced
this pull request
May 25, 2026
…ssages (#32018) Occasionally a 'tool_result' SSE event is lost between the daemon and the client (network drop, reconnect race, server-side fanout glitch). The tool call stays 'status: running' forever in the client's DisplayMessage[], even though the assistant clearly continued — there is a subsequent assistant message in the transcript, which is only possible if the LLM provider received the tool result on the server side. The render layer shows these stuck calls as a permanent spinner on an older message bubble, which is misleading: the tool DID complete, the client just never saw the result. This adds 'repairDanglingToolCalls' as Hack #4 in the sanitize pipeline. For every assistant message that is NOT the last assistant in the transcript, any tool call with 'status: running' is rewritten to: status: 'error' isError: true result: '<SYNTHETIC_DANGLING_RESULT>' (explains the client-side data loss for diagnosis) Pipeline placement: AFTER 'removeDuplicateTrailingAssistant' so the dedup filter's pairwise 'result' equality check sees the original (still undefined) values and can correctly identify the duplicate. Why the last assistant is never patched: its dangling tools might still resolve via in-flight stream events. Why a subsequent USER message doesn't qualify as proof: only a later ASSISTANT message proves the LLM provider received the result and continued. A trailing user message could just be a queued send. Tests: 11 new cases under 'sanitizeDisplayMessages · repair dangling tool calls' covering the happy path, last-assistant guard, user-only trailing, sibling tool calls, multiple older assistants, identity preservation, empty input. Integration test extended to assert the patched tool call's result contains 'client-side data loss'. 35/35 tests pass. Lint + audit clean. Typecheck matches the main baseline (10 pre-existing errors in unrelated files). Co-authored-by: vellum-apollo-bot[bot] <242025090+vellum-apollo-bot[bot]@users.noreply.github.com>
vellum-apollo-bot Bot
added a commit
that referenced
this pull request
May 27, 2026
First imperative replacement piece for `useDeprecatedTranscriptScroll`:
when the transcript scroll container is attached to the DOM in the
context of a conversation, snap to bottom. This is the DOM lifecycle
event, not a React state change — `key={conversationId}` ensures the
`<div>` re-attaches on conversation switch and on a fresh detail-page
load, and the callback ref fires at attach time before paint.
This restores the "open every conversation view at latest" behavior
under the controller flag. Without it, the no-op deprecated hook was
leaving the transcript scrolled to the top on both conversation
switch and page refresh.
Consolidation: renamed `transcript-scroll-flag.ts` →
`transcript-scroll.ts` and moved the two new utilities into the same
file. All scroll utilities now live in one place; the gating against
`TRANSCRIPT_SCROLL_CONTROLLER_ENABLED` lives inside each utility's
body so component files import them without branching on the flag.
Tracker: /workspace/scratch/scroll-imperative-tracker.md (feature #4
of 11).
vellum-apollo-bot Bot
added a commit
that referenced
this pull request
May 27, 2026
- PostMessageResult.conversationId is now always string (was string | null in the server-mint flow). The assistant is the authoritative source of truth; drop the redundant resolvedConversationId field on both result shapes. - Always send conversationKey on the wire when the caller has an id so pre-0.8.6 assistants (which don't read conversationId) still resolve the conversation via external-key lookup. On 0.8.6+ assistants, also send conversationId to trigger the strict internal-id lookup. The duplicated id is harmless: pre-0.8.6 ignores the unknown conversationId key; 0.8.6+ reads conversationId first. - Add non-hook assistantSupports(minVersion) to lib/backwards-compat/utils.ts so both the React hook (useAssistantSupports) and event-handler/async call sites share the same semver-comparison core. Refactor supportsServerMintedConversation() to call it instead of duplicating the parse/compare logic. - Replace the invariant throw in use-send-message.ts with a typecheck- enforced contract. postChatMessage's success arm now guarantees a non-empty conversationId so the runtime throw is no longer needed. - Codex P2 — server-mint queue race: add pendingDraftMintRef so the queue path in sendMessage awaits the in-flight first-message POST before posting follow-up messages. Without this, a follow-up submitted during the mint window would 404 on 0.8.6+ assistants (the assistant minted a different id from the still-unresolved local draft key). - Update tests: wire-field cutover now asserts both fields on 0.8.6+; server-mint tests assert the authoritative-id contract; add coverage for caller-supplied id being overridden by the assistant's response. Followup work (NOT in this PR): - Add PostMessageResult to assistant/src/api/responses/*.ts and zod- validate the response shape in postChatMessage (Vargas comments #2 + #4).
dvargasfuertes
pushed a commit
that referenced
this pull request
May 27, 2026
…32239) * refactor(web): scroll to bottom on transcript container DOM attach First imperative replacement piece for `useDeprecatedTranscriptScroll`: when the transcript scroll container is attached to the DOM in the context of a conversation, snap to bottom. This is the DOM lifecycle event, not a React state change — `key={conversationId}` ensures the `<div>` re-attaches on conversation switch and on a fresh detail-page load, and the callback ref fires at attach time before paint. This restores the "open every conversation view at latest" behavior under the controller flag. Without it, the no-op deprecated hook was leaving the transcript scrolled to the top on both conversation switch and page refresh. Consolidation: renamed `transcript-scroll-flag.ts` → `transcript-scroll.ts` and moved the two new utilities into the same file. All scroll utilities now live in one place; the gating against `TRANSCRIPT_SCROLL_CONTROLLER_ENABLED` lives inside each utility's body so component files import them without branching on the flag. Tracker: /workspace/scratch/scroll-imperative-tracker.md (feature #4 of 11). * review iter: split flag file, drop key helper, observe content for seed-then-grow Three review comments addressed: • dvargasfuertes (transcript.tsx:262) — drop `getTranscriptScrollContainerKey()` helper. The scroll container now always remounts on conversation switch via `key={conversationId}`. With the flag OFF the deprecated hook still resets pin state on conversationId change and the items-effect re-fires scrollToLatest inside the auto-pin window, so the OFF path is unchanged behaviorally. • dvargasfuertes (use-deprecated-transcript-scroll.ts:37) — restored `transcript-scroll-flag.ts` as its own file containing only the flag storage + reload-on-toggle logic. `transcript-scroll.ts` imports `TRANSCRIPT_SCROLL_CONTROLLER_ENABLED` from it. When the migration is done, deleting the flag file is one rm. • chatgpt-codex-connector P2 (transcript-scroll.ts:127) — real race: `useViewportMinHeight` seeds LatestTurnRow's `minHeight = clientHeight` in a post-paint effect, growing scrollHeight after the attach-time snap. With the controller flag ON the deprecated hook's ResizeObserver no longer runs, so nothing re-pins. Fix: `attachSnapToLatest` now sets up a `ResizeObserver` on the content wrapper that re-snaps on every content height change until the first `wheel`/`touchmove`/`keydown` on the container. New hook shape: `useTranscriptScrollOnAttach({ scrollContainerRef, contentRef })` returns `{ scrollContainerCallbackRef, contentCallbackRef }`. Wired both into `Transcript` — container callback ref forwards only; content callback ref triggers `attachSnapToLatest` (the pure imperative function the hook delegates to). Six new tests cover initial snap, re-snap on resize, three gesture-disengage paths, teardown, and the no-ResizeObserver fallback. Local: lint clean; tsc same 18 pre-existing errors as main. * review iter: TranscriptProps.conversationId required (no ? no docstring) Per dvargasfuertes (transcript.tsx:46). Type stays `string | null` — production callsite passes `activeConversationId: string | null` and nullable conversations are still a valid state (empty conversation pre-mint). Test callsites updated to pass `conversationId={null}` (transcript.test.tsx ×5, transcript-subagent-inline.test.tsx ×9). --------- Co-authored-by: vellum-apollo-bot[bot] <242025090+vellum-apollo-bot[bot]@users.noreply.github.com>
dvargasfuertes
pushed a commit
that referenced
this pull request
May 27, 2026
…32234) * feat: server-mint vellum conversations on assistant /send and add web compat gate * feat(web): wire postChatMessage server-mint flow into use-send-message postChatMessage(conversationId: null) omits both wire fields so the daemon mints a fresh row on empty-handed sourceChannel="vellum" sends. The response's conversationId becomes resolvedConversationId, and the existing draft-key-resolution path in sendMessage swaps optimistic state and navigates to the canonical URL. Sender side (use-send-message.ts): - sendMessageViaStream now takes isDraft and gates on supportsServerMintedConversation() to pick null vs. requestConversationId. - effectiveConversationId narrowed defensively — postChatMessage's success contract guarantees a non-null id (the early-return on server-mint without a daemon-echoed id surfaces 422 first). API side (messages.ts): - PostMessageResult.conversationId: string | null on success arms. - Body composition omits both wire fields when conversationId is null; pickConversationIdWireField stays in charge of the non-null path. - 422 early-return when server-mint was requested but the daemon accepted the message without echoing a conversation id back. Tests (post-chat-message.test.ts): - Server-mint flow describe block: omit both fields, return minted id as resolvedConversationId, 422 on broken daemon contract, and a guard that the legacy non-null path is unchanged. Queue path (line 519) left unchanged — it requires an existing conversation by construction. The tight race where a draft's second send fires before the first's resolveDraft swap is a known corner case the existing wire-field gate also hits; will revisit if it surfaces. * fix(web): address PR feedback on server-minted conversations - PostMessageResult.conversationId is now always string (was string | null in the server-mint flow). The assistant is the authoritative source of truth; drop the redundant resolvedConversationId field on both result shapes. - Always send conversationKey on the wire when the caller has an id so pre-0.8.6 assistants (which don't read conversationId) still resolve the conversation via external-key lookup. On 0.8.6+ assistants, also send conversationId to trigger the strict internal-id lookup. The duplicated id is harmless: pre-0.8.6 ignores the unknown conversationId key; 0.8.6+ reads conversationId first. - Add non-hook assistantSupports(minVersion) to lib/backwards-compat/utils.ts so both the React hook (useAssistantSupports) and event-handler/async call sites share the same semver-comparison core. Refactor supportsServerMintedConversation() to call it instead of duplicating the parse/compare logic. - Replace the invariant throw in use-send-message.ts with a typecheck- enforced contract. postChatMessage's success arm now guarantees a non-empty conversationId so the runtime throw is no longer needed. - Codex P2 — server-mint queue race: add pendingDraftMintRef so the queue path in sendMessage awaits the in-flight first-message POST before posting follow-up messages. Without this, a follow-up submitted during the mint window would 404 on 0.8.6+ assistants (the assistant minted a different id from the still-unresolved local draft key). - Update tests: wire-field cutover now asserts both fields on 0.8.6+; server-mint tests assert the authoritative-id contract; add coverage for caller-supplied id being overridden by the assistant's response. Followup work (NOT in this PR): - Add PostMessageResult to assistant/src/api/responses/*.ts and zod- validate the response shape in postChatMessage (Vargas comments #2 + #4). * fix(web): simplify wire-field selection and mint race guard Round 3 PR feedback (#32234): 1. messages.ts — pick exactly ONE wire field instead of duplicating conversationKey alongside conversationId on 0.8.6+ assistants. - 0.8.6+ with caller id: conversationId only - 0.8.6+ with null: neither (server-mint branch) - pre-0.8.6 with id: conversationKey only - pre-0.8.6 with null: conversationKey: null (legacy create-or-lookup remains addressed; defense-in-depth for callers that bypass the supportsServerMintedConversation gate) 2. use-send-message.ts — replace the deferred-promise race guard with a simpler "block any send while a mint is in flight" gate. The POST 200s quickly, so rejecting follow-up sends with a brief inline error is simpler than threading an unresolved id through the queue path. - pendingDraftMintRef: useRef<string | null> (was { draftId, promise }) - set before POST in sendMessageViaStream, cleared after resolve/reject - sendMessage entry returns early if the gate matches the active draft Tests updated to match the new contract; added coverage for conversationKey: null on pre-0.8.6 callers. 27/27 pass; tsc + eslint clean on touched files; assistant send-endpoint-busy 15/15 unchanged. --------- Co-authored-by: vellum-apollo-bot[bot] <229272765+vellum-apollo-bot[bot]@users.noreply.github.com> Co-authored-by: vellum-apollo-bot[bot] <242025090+vellum-apollo-bot[bot]@users.noreply.github.com>
This was referenced May 27, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three new GitHub Actions workflows for CI/CD.
Workflows
ci-web.yml- Deploy WebTrigger: Merge to
mainwithweb/**or self changespr-web.yml- PR Web ChecksTrigger: PRs with
web/**or self changespr-platform.yml- PR Platform ChecksTrigger: PRs with
platform/**or self changesRequired Secrets
GCP_SA_KEYDATABASE_URLANTHROPIC_API_KEYRequired Variables
GCP_PROJECT_IDGCP_REGIONGKE_CLUSTER_NAME