Skip to content

feat(functions): derive default functions domain as function2.insforge.app - #121

Merged
jwfing merged 2 commits into
mainfrom
feat/functions-domain-v2
Jul 31, 2026
Merged

feat(functions): derive default functions domain as function2.insforge.app#121
jwfing merged 2 commits into
mainfrom
feat/functions-domain-v2

Conversation

@jwfing

@jwfing jwfing commented Jul 31, 2026

Copy link
Copy Markdown
Member

Summary

Deno Deploy Classic (v1) is fully sunset, so the legacy {appKey}.functions.insforge.app domain no longer works. This PR points the SDK's derived default functions URL at the v2 CloudFront proxy domain {appKey}.function2.insforge.app, matching the backend's FUNCTIONS_DOMAIN default (backend/src/infra/config/app.config.ts).

Changes

  • src/modules/functions.tsderiveSubhostingUrl() now derives https://{appKey}.function2.insforge.app; doc comment updated.
  • src/types.tsfunctionsUrl @example comment updated.
  • src/modules/__tests__/functions.test.ts — test URLs updated. Note: the in-process dispatch short-circuit requires the configured functionsUrl to strictly equal the derived local URL, so the explicit URLs in tests must use the new domain.

Behavior notes

  • Explicitly configured functionsUrl is unaffected.
  • Non-.insforge.app base URLs (self-hosted) still derive nothing and go through the {baseUrl}/functions/{slug} proxy path.
  • If the direct subhosting URL returns 404, the SDK still falls back to the proxy path as before.

Testing

  • npx vitest run src/modules/__tests__/functions.test.ts — 11/11 pass.
  • Full suite: 6 pre-existing failures in ssr.test.ts / auth.test.ts reproduce on clean main (verified via git stash), unrelated to this change. Same for the 5 pre-existing npm run typecheck errors caused by a local @insforge/shared-schemas version mismatch.

🤖 Generated with Claude Code


Summary by cubic

Switch the SDK’s default functions domain to https://{appKey}.function2.insforge.app to align with Deno Deploy v2 and the backend default. Fallback to the proxy path remains; explicit functionsUrl and non-.insforge.app bases are unchanged.

  • Migration

    • No action for most apps.
    • If you hardcode or test against https://{appKey}.functions.insforge.app, update to https://{appKey}.function2.insforge.app.
  • Dependencies

    • Bump @insforge/sdk to 1.5.2.

Written for commit a1f4314. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • Updated Edge Functions URL routing to use the correct function2.insforge.app hostname.
    • Improved compatibility across subhosting, proxy fallback, in-process, and external deployment scenarios.
  • Documentation

    • Updated configuration examples to reflect the current Edge Functions URL format.
  • Chores

    • Updated the package version to 1.5.2.

Changes since #121 opened

  • Derived default functions domain as function2.insforge.app [a1f4314]
  • Bumped package version from 1.5.1 to 1.5.2 and updated lock file [a1f4314]

…e.app

Deno Deploy Classic (v1) is fully sunset, so the legacy
{appKey}.functions.insforge.app domain no longer works. Point the
SDK's derived default at the v2 CloudFront proxy domain
{appKey}.function2.insforge.app, matching the backend's
FUNCTIONS_DOMAIN default.

Test URLs are updated too: the in-process dispatch short-circuit
requires the configured functionsUrl to equal the derived local URL,
so the explicit URLs in tests must use the new domain.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Changes

Functions hostname update

Layer / File(s) Summary
Update Functions hostname contract
src/modules/functions.ts, src/types.ts, package.json
deriveSubhostingUrl and the functionsUrl example now use function2.insforge.app. The package version changes to 1.5.2.
Align Functions URL tests
src/modules/__tests__/functions.test.ts
Subhosting, proxy fallback, in-process, and foreign-deployment tests now expect the function2 domain.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

Suggested reviewers: fermionic-lyu

Poem

A rabbit checks the hostname trail,
function2 keeps each route on scale.
Tests and docs now match the call,
Version 1.5.2 marks it all.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the change to derive the default functions domain as function2.insforge.app.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/functions-domain-v2

Comment @coderabbitai help to get the list of available commands.

@jwfing jwfing left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Review: feat(functions): derive default functions domain as function2.insforge.app

Summary: A focused, correct one-line-behavior change that repoints the SDK's derived default functions domain from the sunset {appKey}.functions.insforge.app to the v2 CloudFront proxy {appKey}.function2.insforge.app, with matching doc/example and test updates.

Requirements context

No spec/plan under docs/superpowers/ covers this domain migration specifically. The closest document, docs/superpowers/specs/2026-04-15-functions-in-process-dispatch-design.md (+ its plan), was consulted for the surrounding dispatch logic — this PR is a follow-up operational fix (Deno Deploy Classic v1 sunset), assessed against the PR description and the existing code. The PR body's core claim (new domain matches backend FUNCTIONS_DOMAIN default in backend/src/infra/config/app.config.ts) is a cross-repo fact I can't verify from this workspace; see Information below.

Findings

Critical
(none)

Suggestion
(none)

Software engineering / functionality — Information

  • src/modules/functions.ts:53 — The change is internally consistent with the in-process dispatch short-circuit: the equality guard at functions.ts:105-109 compares this.functionsUrl === localFunctionsUrl, and both sides flow through the same deriveSubhostingUrl(), so the derived default and the short-circuit stay in lockstep. Verified: npx vitest run src/modules/__tests__/functions.test.ts → 11/11 pass. Scope is tight (derived default only); explicitly-configured functionsUrl, self-hosted (non-.insforge.app) bases, and the 404→proxy fallback are all correctly untouched.
  • docs/superpowers/specs/2026-04-15-functions-in-process-dispatch-design.md:5,7,25,47 and the companion plan still reference the old {appKey}.functions.insforge.app domain. These are historical design records, so updating them is optional — noting only so the docs aren't mistaken for current guidance.
  • Behavioral caveat (pre-existing, not introduced here): the direct-subhosting path only falls back to the proxy on a 404 (functions.ts:146). A user who explicitly set functionsUrl to the old, now-dead …functions.insforge.app (per the previous @example) could get a DNS/connection error or 5xx instead of a 404 and would not fall back. This PR fixes the derived default (the common path) and doesn't regress anything; flagging only as a migration awareness note for anyone who hardcoded the old value.

Security
No security-relevant changes — no new inputs reaching SQL/shell/HTTP sinks, no auth changes, no secrets/PII, no new dependencies. deriveSubhostingUrl still gates on hostname.endsWith('.insforge.app') before deriving.

Performance
No performance impact — pure string-derivation change on an existing code path; no new I/O, loops, or allocations.

Verdict

approved (informational — the human still gives the explicit GitHub approval). Zero Critical findings; the change is minimal, correct, well-tested, and in-scope.

Fermionic-Lyu
Fermionic-Lyu previously approved these changes Jul 31, 2026

@Fermionic-Lyu Fermionic-Lyu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, Approved.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No issues found across 3 files

Re-trigger cubic

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@Fermionic-Lyu Fermionic-Lyu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, Approved.

@jwfing
jwfing merged commit 1da68e6 into main Jul 31, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants