feat(webhooks): use WEBHOOK_CALLBACK_BASE_URL as server-side default for webhook management#5
Open
ada-evorada wants to merge 2 commits into
Conversation
Author
CI Failures ResolvedFixes Applied
Verification
|
suda
approved these changes
Apr 9, 2026
Member
suda
left a comment
There was a problem hiding this comment.
Summary
Clean, well-structured implementation. The server-side fallback chain (explicit param -> WEBHOOK_CALLBACK_BASE_URL -> BAD_REQUEST) is consistent across create, delete, and list. Frontend gracefully handles the loading state by falling back to API_URL then window.location.origin. All 7361 unit tests pass and CI is green.
Minor Observation
tests/unit/api/routers/system.test.ts: beforeEach is imported from vitest but never used in the file. No functional impact, just a stale import.
suda
requested changes
Apr 10, 2026
Member
suda
left a comment
There was a problem hiding this comment.
@ada-evorada please rebase on top of fix/dark-mode-css-theme-inline
… for webhook management
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
38d9f78 to
780939b
Compare
Member
|
🔍 Reviewing code — Examining the PR changes for quality and correctness... |
suda
pushed a commit
that referenced
this pull request
Jun 1, 2026
…grel-intelligence#1113) * docs(plans): add spec 002 and plans; lock plan 002/1 Spec 002 (linear-webhook-setup-ux) introduces the Linear wizard UX improvements and save-path fix. Plan 1 locked for execution. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(dashboard): structured error logging for tRPC + Hono Adds src/api/errorLogging.ts with formatters that surface pg driver fields (code, detail, constraint, table) onto server-side logs for both Hono app.onError and the tRPC errorFormatter. Client responses for INTERNAL_SERVER_ERROR now carry a generic message; real details remain in cascade-dashboard-* stdout for operators to grep. plan 002/1 task 1. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(db): allow provider=linear under category=pm in integration check constraint The chk_integration_category_provider check constraint (from 0047_add_alerting_integration.sql) only permitted trello/jira for pm-category integrations. Linear support was introduced without a matching constraint update, so every projects.integrations.upsert for category=pm + provider=linear failed with SQLSTATE 23514 and surfaced as HTTP 500 from the dashboard Linear wizard. Migration 0049 drops and re-creates the constraint with linear added to the pm branch. Forward-only, no data migration required. Reproduced with two new integration tests against the test DB. plan 002/1 tasks 2-3 (diagnose + fix). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test(api): assert plaintext credentials never leak into server logs Integration tests that spy on console.{log,error,warn} around writeProjectCredential, upsertProjectIntegration (happy + FK-violation paths), and the formatTRPCErrorLog formatter. Asserts the sentinel credential value never appears in captured output, while confirming the real PG error (SQLSTATE + constraint name) IS present on the failure path — proving diagnosability without leakage. plan 002/1 task 4. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs(plans): plan 002/1 (save-path-fix) done — unblock Linear wizard Save All 11 per-plan ACs pass. Spec ACs #5-8 delivered: - Linear upsert succeeds on fresh projects and re-configuration - DB error diagnostics (SQLSTATE, constraint, detail) surface in server logs - tRPC clients receive generic 'Internal server error' for unexpected throws - Plaintext credentials never leak into captured stdout/stderr Caveat: CLAUDE.md addition deferred to avoid conflicting with the user's in-progress rewrite of that file. Doc impact met via CHANGELOG. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs(plans): update plan 002/1 progress checkboxes to done Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs(plans): lock plan 002/2 + record testing-approach divergence Plan 2 originally assumed @testing-library/react in web/; project has no such infrastructure and the web/ package has no test script. Plan edited in place to: - use react-dom/server.renderToStaticMarkup (available; no new deps) - test under the existing root unit-core project - drop interactive-mutation tests (would need jsdom); ProjectSecretField internals are out-of-scope per spec non-goal Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs(plans): plan 002/2 frontmatter status -> wip Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(dashboard): linear wizard UX — accurate events list and inline signing secret Plan 002/2 (wizard-webhooks-step). Linear PM wizard Webhooks step now: - lists the three event families CASCADE consumes (Issues, Comments, Issue Labels), each with a one-line rationale tracing to src/triggers/linear/ - renders a ProjectSecretField bound to LINEAR_WEBHOOK_SECRET directly beneath the webhook URL, matching the Sentry alerting tab pattern - drops the 'store as LINEAR_WEBHOOK_SECRET in project credentials' trailing bullet (replaced by the inline input) PMWizard threads projectId + the masked LINEAR_WEBHOOK_SECRET credential meta from projects.credentials.list through WebhookStep, so the field reflects its own stored value on mount and stays in sync with the Credentials tab. Tests: 17 new SSR tests using react-dom/server — no React testing library required. Interactive mutation firing is out of scope (would need jsdom); ProjectSecretField itself is untested today under the same constraint and is spec non-goal to modify. vitest.config.ts gains @/components, @/lib, @/hooks aliases pointing at web/src/ for test-time resolution. src/integrations/README.md Linear section defers to the dashboard wizard as the authoritative setup source. Plan 2 closes out spec 002. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs(claude): streamline CLAUDE.md to essentials Compacts the project's CLAUDE.md from ~820 to ~163 lines by dropping dated setup minutiae, duplicated process docs, and legacy sections. Keeps the load-bearing context: architecture, PR checkout gotcha, testing commands, Zod policy, migrations, GitHub dual-persona rules, trigger system, engines, environment, and git hooks. No code changes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
suda
pushed a commit
that referenced
this pull request
Jun 1, 2026
Closes plan 006/5 — the final cleanup plan of spec 006. Backend: - src/integrations/bootstrap.ts — DELETED. PM registrations flow through src/integrations/pm/index.ts (which also mirrors manifests into integrationRegistry). SCM (GitHub) and alerting (Sentry) self-register via new side-effect modules. - src/github/register.ts + src/sentry/register.ts — new minimal side-effect modules that replace the respective branches of the deleted bootstrap. SCM + alerting stay on the legacy IntegrationModule pattern (out of spec 006 scope). - src/pm/registry.ts — converted to a read-only adapter over pmProviderRegistry. get/getOrNull/all/createProvider/ resolveLifecycleConfig all delegate to the manifest registry. register() is a deprecation warn. The ~9 unmigrated call sites (webhook handlers, manual runner, credential scope, lifecycle, GitHub adapter) keep working unchanged — they now transparently read from the manifest registry, so there is no divergent registration. - src/router/index.ts + src/worker-entry.ts — updated to import the three new side-effect modules instead of the deleted bootstrap. - src/integrations/registry.ts — header comment updated to reflect the new population topology. Tests: - tests/unit/integrations/bootstrap.test.ts — rewritten to cover the new side-effect-module wiring (the file name stays for audit clarity; its content asserts the same end-state invariants). - 8 other test files that imported bootstrap.js for side effect are migrated to import the three new modules (pm barrel + github/register + sentry/register). Docs: - src/integrations/README.md — rewritten. Transitional note + "Legacy path" section removed. The README is now the single canonical author's guide for adding a new PM provider. - CLAUDE.md — integration-abstraction pointer updated to final state. - CHANGELOG.md — entry per plan. Plan-divergence: - AC #2 (delete pm/registry.ts) — became: convert to a read-only delegate. Deleting it would require migrating 9 call sites that are out of spec scope. The delegate preserves the end state (single source of truth = pmProviderRegistry) without downstream churn. - AC #5 (consolidate createXxxLabel tRPC endpoints) — deferred to a follow-up PR. Purely additive cleanup; not required for any spec AC. Both divergences documented in the .done plan. Tests: 7809/7809 pass. Build + lint + typecheck clean. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
suda
pushed a commit
that referenced
this pull request
Jun 1, 2026
…tructured envelope, --comment alias) (mongrel-intelligence#1190) * docs(014): spec + plans for cascade-tools agent ergonomics Adds docs/specs/014-cascade-tools-agent-ergonomics.md plus two plans covering shared-infra and create-pr-review adoption. Prompted by prod run 5d993b04-6e05-4ae1-b7de-8c274cf3496b. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(plan-014): lock plan 1 (shared-infra) * feat(cascade-tools): plan 014/1 shared-infra — truthful prompts + envelope Ships the root-cause fix for prod run 5d993b04-6e05-4ae1-b7de-8c274cf3496b plus the shared infrastructure every future gadget inherits: - System-prompt renderer (src/backends/shared/nativeToolPrompts.ts) stops stripping trailing 's' from array param names and claiming '<string> (repeatable)' for every array. Array-of-object params now render as `--<flag> '<json>'` with aliases appended via `|` and a one-line runnable example from the tool definition. - Factory (src/gadgets/shared/cliCommandFactory.ts) gains oclif flag aliases, JSON parsing for array-of-object flags, file-input JSON parsing, `examples` wired into oclif `--help`, and Levenshtein-based 'did you mean' suggestions for mistyped flags (via fastest-levenshtein). - New shared error envelope (src/gadgets/shared/errorEnvelope.ts) — every CLI failure emits `{"success":false,"error":{type,flag?,message,got?, expected?,hint?,example?}}` on stdout plus a one-line prose summary on stderr. All prior `this.error()` / flat `{success:false,error:"<string>"}` call sites migrated. - Contracts widened: ParameterDefinition gains `cliAliases`, FileInput- Alternative gains `parseAs`, ToolManifest parameters carry `items`, `aliases`, `example`. - Manifest generator threads the new fields through. - bin/cascade-tools.js wraps `run()` to swallow oclif ExitError cleanly so the envelope isn't obscured by Node's default stack dump. Plan-1 ACs #1–mongrel-intelligence#17 all delivered. 8438/8438 unit tests passing. Test surface delta: 57 new unit tests across errorEnvelope.test.ts, shared-nativeToolPrompts.test.ts, and factories.test.ts. Seven legacy assertions encoding the pre-014 error surface updated in cli/cli-command- factory, cli/file-input-flags, cli/scm/create-pr-sidecar, cli/scm/create- pr-review-sidecar, backends/claude-code. Plan 2 adopts the pattern on createPRReviewDef — zero shared-file edits — proving the declarative-metadata invariant. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(plan-014): lock plan 2 (createprreview-adopt) * feat(cascade-tools): plan 014/2 createprreview-adopt + spec done Applies the spec-014 declarative-metadata pattern to createPRReviewDef: - --comment alias for --comments (the exact muscle-memory mistake from prod run 5d993b04-6e05-4ae1-b7de-8c274cf3496b). - --comments-file <path> (and - for stdin) JSON-parsed escape hatch for long payloads that don't survive shell quoting. - Two declarative fields on createPRReviewDef.parameters.comments.cliAliases + createPRReviewDef.cli.fileInputAlternatives. Zero edits to shared infrastructure (cliCommandFactory, manifestGenerator, nativeToolPrompts, errorEnvelope) — proves spec 014's single-entrypoint invariant. Per-plan ACs #1, #2, #3, #5, #6, mongrel-intelligence#7, mongrel-intelligence#8, mongrel-intelligence#9, mongrel-intelligence#11, mongrel-intelligence#12 auto-verified (unit tests + build + lint + typecheck). AC #4 (binary-level smoke) tagged [manual] because vitest fork-pool workers fail to capture stdout/stderr from spawned binaries that do top-level await import(); the six scenarios were verified manually against the built binary and the trace is recorded in the plan. AC mongrel-intelligence#10 n/a — integration test path abandoned for the same reason. All plans done. Spec 014 marked .done (docs/specs/014-*.md → .done). CHANGELOG Unreleased updated with a per-plan entry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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
systemtRPC router — exposesWEBHOOK_CALLBACK_BASE_URLasrouterPublicUrlvia asystem.getPublicUrlquery, giving CLI and frontend a single source of truth for the server-configured public URLcreateanddeleteprocedures now acceptcallbackBaseUrlas optional, falling back toWEBHOOK_CALLBACK_BASE_URLenv var; throwsBAD_REQUESTif neither is providedsystem.getPublicUrland use the server-provided URL instead of deriving from the browser origincascade webhooks create/deletefetch server public URL viasystem.getPublicUrlwhen--callback-urlis not provided;cascade webhooks listuses server URL for Sentry URL displayCLAUDE.mdupdated to clarify the dual purpose ofWEBHOOK_CALLBACK_BASE_URLand its requirement when deployed behind NAT/reverse proxycredential-scoping.test.tsfixed to properly delete env vars instead of setting toundefinedFixes the issue where webhook creation/deletion would use the local internal URL instead of the public URL when CASCADE is deployed behind NAT/reverse proxy.
Card: https://trello.com/c/c2DxzsJC/14-the-scm-and-pm-integrations-are-configuring-webhooks-but-they-are-using-the-local-url-of-cascade-which-wont-work-if-its-behind-n
Test plan
system.getPublicUrl— returns env var when set,nullwhen not set, throwsUNAUTHORIZEDfor unauthenticated requestswebhooks.createandwebhooks.deletewith nocallbackBaseUrl— uses env var when set, throwsBAD_REQUESTwhen neither is providedsystem.getPublicUrland verify new fallback behaviorWEBHOOK_CALLBACK_BASE_URL=https://cascade.example.comand verifycascade webhooks create <project-id>(without--callback-url) registers webhooks pointing to the public URLWEBHOOK_CALLBACK_BASE_URLis set🤖 Generated with Claude Code