Fix/dropdown menu missing portal - #195
Closed
XiuJie2 wants to merge 23 commits into
Closed
Conversation
* build(ui): update TradingGoose UI dependencies Co-authored-by: BWJ2310 <brucewj2310@gmail.com> Co-authored-by: BWJ2310-backup <jun.1216.wei@gmail.com> * refactor(app): modernize UI and failure-state handling Co-authored-by: BWJ2310 <brucewj2310@gmail.com> Co-authored-by: BWJ2310-backup <jun.1216.wei@gmail.com> * docs: update project README Co-authored-by: BWJ2310 <brucewj2310@gmail.com> Co-authored-by: BWJ2310-backup <jun.1216.wei@gmail.com> * changelog: add August 01, 2026 entry detailing migration to Base UI components and related updates --------- Co-authored-by: BWJ2310 <brucewj2310@gmail.com>
* fix(folders): serialize folder parent validation Co-authored-by: BWJ2310 <brucewj2310@gmail.com> Co-authored-by: BWJ2310-backup <jun.1216.wei@gmail.com> * fix(webhooks): harden external callback polling Co-authored-by: BWJ2310 <brucewj2310@gmail.com> Co-authored-by: BWJ2310-backup <jun.1216.wei@gmail.com> * fix(wealthbox): gate dependent fields on credential Co-authored-by: BWJ2310 <brucewj2310@gmail.com> Co-authored-by: BWJ2310-backup <jun.1216.wei@gmail.com> * docs(changelog): document copilot tool changes Co-authored-by: BWJ2310 <brucewj2310@gmail.com> Co-authored-by: BWJ2310-backup <jun.1216.wei@gmail.com> * refactor(listing): separate identity from resolved display data Co-authored-by: BWJ2310 <brucewj2310@gmail.com> Co-authored-by: BWJ2310-backup <jun.1216.wei@gmail.com> * refactor(webhooks): remove legacy webhook test endpoint Co-authored-by: BWJ2310 <brucewj2310@gmail.com> Co-authored-by: BWJ2310-backup <jun.1216.wei@gmail.com> * fix(webhooks): validate Teams notification client state Co-authored-by: BWJ2310 <brucewj2310@gmail.com> Co-authored-by: BWJ2310-backup <jun.1216.wei@gmail.com> --------- Co-authored-by: BWJ2310 <brucewj2310@gmail.com>
Adds a full NVIDIA NIM provider (OpenAI-compatible) alongside admin service entries for DeepSeek, OpenRouter and NVIDIA, so a deployment can supply model keys centrally instead of every request carrying its own. getApiKey() previously threw before any provider code ran when a request had no key, which would have made the new admin fields decorative. It now falls back to the admin-configured key: request key -> rotation slots (per-minute) -> single API key -> throw Rotation was reachable only when isHosted was true, which is a hardcoded hostname allowlist, so the OpenAI and Anthropic rotation slots were never read on self-hosted deployments. The fallback path is not gated on isHosted, so rotation now works anywhere; the hosted branch keeps its existing platform-key-wins behaviour. OpenAI's Default API Key stays reserved for embeddings and is not repurposed for completions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…mpose The image workflow could not run here: it targeted Blacksmith runners this fork has no access to, and hardcoded ghcr.io/tradinggoose, which the fork's GITHUB_TOKEN cannot push to. Because every tag went to one build-push step, that GHCR failure would have taken the Docker Hub push down with it. - run on ubuntu-latest with the upstream docker/* actions, plus a GHA cache to replace the build cache Blacksmith provided - derive the GHCR namespace from the repository owner, lowercased, since GHCR rejects uppercase paths - push all three images to GHCR, and mirror only the app image to Docker Hub, so the schema-carrying migrations image stays private for free - read DOCKERHUB_USERNAME from vars rather than secrets, and fail loudly when it is unset instead of pushing to an empty namespace - drop arm64: QEMU emulation on a standard runner is prohibitively slow docker-compose.prod.yml now resolves images through IMAGE_REGISTRY and publishes Postgres on loopback by default, so a public host does not expose the database. INTERNAL_SOCKET_URL is documented and wired through the compose manifests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Feat/model provider keys and registry
Copilot proxied every turn to the managed service at copilot.tradinggoose.ai, which runs the models on TradingGoose's own accounts. A self-hosted deployment therefore had to buy a Copilot API key even with its own OpenAI, Anthropic, NVIDIA or OpenRouter keys already configured, and was limited to the four models that service exposes. Add a local runtime that serves the same endpoints from inside the deployment. It returns byte-identical SSE, so the chat route, the mark-complete resume path, the abort route and the browser store are untouched: - lib/copilot/local-runtime/runtime.ts runs one model call per request, parks the turn on `awaiting_tools`, and resumes when the browser reports results. Tool execution stays in the browser exactly as before. - llm.ts adapts two wire formats: OpenAI chat-completions (OpenAI, DeepSeek, OpenRouter, NVIDIA, Ollama, xAI, Mistral, Fireworks) and Anthropic messages. - conversation-store.ts holds turn state in Redis, falling back to the in-process cache, with persisted chat history as a backstop when the cache has expired. - prompt.ts carries the system prompt, which previously lived on the service and left lib/copilot/prompts.ts holding a one-line stub. Admin > Services gains a Runtime Mode field (COPILOT_RUNTIME_MODE), defaulting to `local`; `hosted` restores the previous behaviour. The Copilot API key is now optional, since it is only used in hosted mode. The four-model whitelist is replaced by /api/copilot/models, derived from whichever providers have keys configured. OpenRouter models that cannot take tools are filtered out rather than failing at the first tool call. Also make the catalog's `envVar` fallbacks real: COPILOT_API_KEY, COPILOT_API_URL and OLLAMA_URL were documented but read by nothing, so a manifest-configured deployment silently had no Copilot credentials at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
# Conflicts: # apps/tradinggoose/widgets/widgets/copilot/components/user-input/components/model-selector.tsx
Feat/local copilot runtime
Tools like list_workflows and create_workflow take workspaceId as a required argument, and none of the 113 registered tools can look one up. The browser knows the id and sends it to /api/copilot/chat, but the chat route never forwarded it and the local runtime's system prompt never mentioned it, so the model had no source for the value and guessed — typically reusing an id from an attached context, which comes back as "Access denied: You do not have permission to read this workflow" and reads like a permissions problem rather than a wrong id. Forwards the incoming workspaceId through to the runtime, stores it on the conversation so a tool-call resume keeps it, and states it in the system prompt. When a chat has no workspace the prompt now says so explicitly and tells the model to ask rather than invent one. Also tells the model that ids are scoped to one entity kind, so an access error on a borrowed id reads as the wrong id rather than as missing permission. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(copilot): tell the local runtime which workspace the chat is in
The browser appends a new content block for every output item id it has not seen (`ensureStreamingTextBlock` / `ensureStreamingThinkingBlock` in stores/copilot/streaming.ts). The runtime closed its open text item as soon as a reasoning chunk arrived and vice versa, so it minted a fresh id on every switch. Reasoning models interleave `reasoning_content` and `content` throughout an answer rather than emitting all reasoning first, which turned one reply into a long alternating run of thinking and text boxes — visible as the answer flickering while the agent worked. Keeps one item id per kind for the whole model call and finalizes both at the end, so each kind lands in exactly one block. Also sends max_tokens on the OpenAI-compatible path, which it never set. Gateways apply their own default when it is absent and NVIDIA NIM's is low enough to truncate an agent reply mid-sentence; the Anthropic path was already passing it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
NVIDIA NIM and OpenRouter have both been timing out, leaving the local Copilot runtime with no reliable key-backed provider. MiniMax's M-series speaks the OpenAI chat-completions format, so it drops into the existing adapter and gives the runtime a third option. Registered the usual way: a provider in `providers/ai/minimax`, an entry in `PROVIDER_DEFINITIONS`, and a service in Admin > Services with an API key, three rotation slots and an editable base URL. The base URL is a setting rather than a constant because mainland-China accounts are issued keys for `api.minimaxi.com`, not the international host. Two details that are not boilerplate: Thinking cannot be turned off on the M2.x models, and by default it comes back inside `content` wrapped in `<think>` tags, which would render as literal markup in the chat. Both the workflow provider and the Copilot stream now send `reasoning_split: true`, which moves it to `reasoning_content` — the field the delta loop already reads. The models are listed from the static catalog rather than a `/models` round trip. MiniMax publishes a short, stable list, and a dynamically discovered id carries no output ceiling, so it would fall back to the shared 4096 cap and truncate replies. A test pins `max_tokens` at 65536 for `MiniMax-M2.7` to keep that from regressing. `providers/ai/utils-server.test.ts` needed the new resolver in its mock factory: the module destructures the runtime namespace, so a missing export made every system-service key lookup fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(copilot): stop the local runtime splitting a reply into extra blocks
feat(copilot): add MiniMax as a local Copilot provider
`create_workflow` writes its row, calls the socket server, and rolls the row back when that call fails. The failure arrives as `WorkflowRealtimeRequiredError` — a plain `Error` subclass — so `buildCopilotServerToolErrorResponse` did not recognise it and fell through to the catch-all payload: `Server tool execution failed` with `retryable: false` and no cause. That is the worst outcome for this error class. The mutation silently did nothing, the operator got no reason, and the model was explicitly told not to retry, so it stopped and guessed at quota and permission causes that were never involved. Read-only tools were unaffected because they never touch the bridge, which made the failure look tool-specific rather than infrastructural. Map the three realtime-bridge error classes onto structured payloads, and split them by what the socket server actually did: - unreachable or timed out -> 503, retryable, hint points at INTERNAL_SOCKET_URL - answered with 4xx (almost always a mismatched internal secret) -> keep the status, not retryable, hint points at INTERNAL_API_SECRET. Retrying never clears a bad secret, so advertising it as retryable is wrong. Unrecognised errors still collapse to the generic 500 — that redaction is deliberate and remains asserted by the existing test. Matching is by `name` rather than `instanceof`: `snapshot-bridge` and `db-helpers` both import `StructuredServerToolError` from this module, so importing the classes back would close an import cycle. The tests build the real error classes, so a rename fails there instead of silently reverting these to the generic 500. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(copilot): surface realtime bridge failures instead of a generic 500
`readSystemServiceKeys` used NVIDIA as its `default:` branch, so any provider
added to `SYSTEM_SERVICE_KEY_PROVIDERS` without its own `case` silently
authenticated against a different vendor's endpoint with an NVIDIA key. That
fails as a confusing 401 from the wrong service rather than as "no key
configured", and it spends a key the operator never pointed at that provider.
NVIDIA is now named explicitly and the fallback returns no key, so the caller
turns a gap into the accurate "API key is required for X".
Two smaller registry gaps found alongside it:
`providerEndpoints` in `hooks/queries/providers.ts` never gained `nvidia`, so
the `Record<ProviderName, string>` was incomplete — a real type error, and
`useProviderModels('nvidia')` would have fetched `undefined`.
The local runtime's "not supported" message listed the supported providers as
a hand-written sentence that had gone stale: it omitted xAI, Mistral and
Fireworks, telling operators that a model the runtime can actually drive is
unsupported. It is now derived from `LOCAL_COPILOT_PROVIDERS`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…egistry fix(providers): stop the key switch falling back to NVIDIA's credentials
… call Copilot could not create a workflow on a split deployment. `create_workflow` writes the row, applies it through the realtime bridge, and deletes the row when that fails — so every attempt failed with nothing left behind, while `list_workflows` (a plain DB read) kept working. The bridge was calling the wrong service. `getInternalRealtimeUrl()` never read `INTERNAL_SOCKET_URL`, even though `docker-compose.prod.yml` passes it, so it fell through to the browser-facing `NEXT_PUBLIC_SOCKET_URL`. Behind a reverse proxy that address is the Next.js app, which answers `/internal/*` with 200 and its HTML shell rather than a 404. The bridge accepted the 200 and only `response.json()` failed. That parse failure carried no status, no content type and no body, so it reached the operator as "the realtime service did not complete the write, check that it is running and reachable" — pointing at a service that was healthy the whole time. Diagnosing it took a container-by-container comparison of a healthy `/health`, a matching `INTERNAL_API_SECRET` and an `apply-state` route that accepted the secret, before the HTML turned up. Three changes: `INTERNAL_SOCKET_URL` is now first in the resolution chain and declared in the env schema. The public URL stays last so single-origin dev setups keep working, but falling back to it now warns once, because in a split deployment it is always wrong. A 2xx body that is not JSON becomes `SocketServerNonJsonResponseError`, carrying the status, content type and a truncated body, and saying outright when an HTML shell means the call reached the app instead of realtime. It is not retried: a proxy returns the same page every time, so the retry budget only delayed the report. The copilot sanitizer maps it to a distinct `realtime_bridge_misrouted` (502, non-retryable) pointing at `INTERNAL_SOCKET_URL`, instead of the outage payload that told the agent to retry. `WorkflowRealtimeRequiredError` now preserves its cause, since it keeps only the message and the classification needs the type. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…nostics fix(realtime): honour INTERNAL_SOCKET_URL and name a misrouted bridge…
`DropdownMenuContent` accepted `portal={false}` and rendered
`Menu.Positioner` + `Menu.Popup` with no `Menu.Portal` above them. Base UI's
`MenuPositioner` calls `useMenuPortalContext()`, which throws unconditionally
when that context is missing, so the menu crashed on every open with the
minified "Base UI error TradingGoose#32" — the full text being
"Base UI: <Menu.Portal> is missing."
One caller uses it: the sub-block dropdown inside a workflow block
(`sub-block/components/dropdown.tsx:559`), which is why the crash showed up in
the workflow editor right after a block was edited.
Removing `portal={false}` would have been the wrong fix. The flag exists to keep
the popup inside the React Flow canvas — the menu carries a search input, and
portalling to `<body>` takes it out of the transformed container that positions
and scales it. So the portal is kept and pointed at an anchor rendered in the
same spot: the popup lands where it did before, and the context the positioner
requires now exists.
The container is held in state rather than a ref because assigning a ref does
not re-render, and the portal target has to be a mounted node.
Verified by typechecking the component against the real `@base-ui/react@1.6.0`
declarations in an isolated project, since the package is absent from the
repo's installed `node_modules` (the same gap that stops 81 `.tsx` test files
from loading). A negative control confirms that harness rejects a wrong
`container` type. The runtime behaviour still needs a browser check: open a
sub-block dropdown in the workflow editor and confirm it renders, filters, and
stays anchored while the canvas is panned and zoomed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
Too many files changed for review (687 files, 500 file limit). |
|
Important Review skippedToo many files! This PR contains 686 files, which is 386 over the limit of 300. To get a review, reduce the PR to 300 files or fewer by splitting it into smaller PRs or changing its base branch. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (686)
You can disable this status message by setting the Comment |
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
2 participants
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
Why
Affected Areas
apps/tradinggooseapps/docspackages/*Issue Links( if any )
Validation
Risk / Rollout Notes
Config / Data Changes
Screenshots / Video
Checklist