diff --git a/DECISIONS.md b/DECISIONS.md index d68143df..50aefa5f 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1475,6 +1475,7 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-09-01] mcp.tool_called gets errorCause = errorCauseTag(err) on the two execution catch paths only (#2051): SQLSTATE or coded-error code, else the error class name, capped at 64 chars; a plain Error deliberately tags null because the class name 'Error' is noise, and pre-execution denials pass nothing since their errorCode already IS the vocabulary. Raw driver messages stay out of event_log on purpose: a constraint-violation message can quote row values. [2026-09-01] counterparty_aliases joins the categorization_templates audit-trigger strip list (20260901200000) instead of staying logged: prod falsified the original exclusion list within 30 minutes of 20260901103000 going live (15 of the first 16 UPDATE audit rows were alias+learning noise, ~800/day projected vs ~50/day of real rule changes), because the learning path merges aliases in the same write that bumps occurrence_count. Explicit trade-off: a human editing ONLY aliases is no longer logged; accepted since alias growth is overwhelmingly automatic and any change also touching accounts/VAT/pattern/active still logs (first real one, 19:02:17Z same day, captured correctly). Pre-fix noise rows stay in audit_log (append-only) and the read model stops labelling the column so they render as no-ops. [2026-09-01] MCP catalog budget attacked at the duplicated staged envelope rather than by demoting more reads: measuring the payload by segment showed outputSchema is 38 % of the whole catalog (23 290 tokens) and STAGED_OPERATION_SCHEMA alone 14 736 of it, the same envelope transmitted 58 times, while descriptions (what the three previous rounds trimmed) are only 10 %. period_status now carries its shape in one sentence instead of declared JSON Schema, matching actor/approve/preview which were always bare objects; 2 552 tokens reclaimed with no tool demoted and no field removed. Every edit is in the LOOSER direction because the server emits structuredContent for every tool and the documented failure mode is a declaration too tight making a strict client reject a successful call. next kept additionalProperties: false: staging.test.ts pins it closed and a guard whose reason is not in front of you is not one to loosen for 420 tokens. Ceiling ratcheted to 60 000 rather than the usual ~300 margin, leaving ~1 070 deliberate working margin: server.ts took 70 commits in 14 days and the previous 116-token margin is what starts the ratchet-block-bump-demote cycle visible in the bench log. +[2026-09-01] Checklist "Anslut till Claude" done-signal = unrevoked api_keys row named MCP-klient (OAuth) for the USER, not per company: the OAuth token route is the only writer of that name and the key company_id is whatever was active at sign-in (null for companyless signups), so a company filter would miss real connections; the AI-profile flag it replaced never meant "connected to Claude" (#2133). Counted through the service client with an explicit user_id filter, not the user client: api_keys' SELECT policy is company-scoped (20260330130000), which hides companyless and archived-company keys and left the step open for exactly the user who had just connected (skeptic refutation on PR #2147). The manual create route reserves the name (400) rather than adding a source column: a migration for a cosmetic tick is not worth it, and the name is already the only marker every reader of that row uses. The consent-page default (all scopes pre-selected, founder decision 2026-08-26) is described, not changed; the compliance swarm's GDPR Art.25(2) finding on this PR targets that decision and is Emil's call, not this docs fix. [2026-09-01] PR #2130 CodeRabbit P1 (org-number twin inherits a grant): the ombud path binds SKV system-credential access to an org number, and org numbers are public and tenant-editable, so while more than one live (non-archived) company claims the same 12-digit org number NO company may verify, mint a deep link, or be granted by the nightly sync on it (409 ORG_NUMBER_CONTESTED; cron counts them as contested and changes nothing). This does not re-add the company-creation org-number guard (org-number reuse stays allowed); it only fences the one feature where the org number is the authority boundary. Also: the cron honours summarizeGrants.recognized (unknown role codes = pinning problem, never a denial), mirroring probeViaOmbudsregister. [2026-09-01] ENABLE_BANKING_SANDBOX removed from the enable-banking manifest and the index.ts header (#2131): the variable was declared as optional but never read anywhere; sandbox vs production is decided by ENABLE_BANKING_API_URL (api.tilisy.com vs api.enablebanking.com, api-client.ts derives isSandbox from the host). A dead variable declared in the manifest is what the self-hosting docs would otherwise have copied. The manifest now lists the two optional variables the code actually reads (API_URL, PSU_TYPE); the _PRODUCTION aliases stay undeclared on purpose, they are a hosted Vercel convention, not an operator contract. diff --git a/app/(dashboard)/hem-sections.tsx b/app/(dashboard)/hem-sections.tsx index cdca1ac7..1192dd38 100644 --- a/app/(dashboard)/hem-sections.tsx +++ b/app/(dashboard)/hem-sections.tsx @@ -62,7 +62,7 @@ export async function HemChecklistSection({ userId, now, initialSetup, - agentBuilt, + hasMcpKey, vatRegistered, momsPeriod, }: { @@ -70,7 +70,8 @@ export async function HemChecklistSection({ userId: string now: Date initialSetup: InitialSetupState - agentBuilt: boolean + /** Live OAuth-minted MCP key exists for this user: see claudeStepDone(). */ + hasMcpKey: boolean vatRegistered: boolean momsPeriod: MomsPeriod | null }) { @@ -160,7 +161,7 @@ export async function HemChecklistSection({ hasBankConnected={onboardingProgress.hasBankConnected} hasSkatteverketConnected={onboardingProgress.hasSkatteverketConnected} hasInboxItems={onboardingProgress.hasInboxItems} - hasAgentBuilt={agentBuilt} + hasMcpKey={hasMcpKey} vatLine={vatLine} sieSweep={ sieSweep diff --git a/app/(dashboard)/page.tsx b/app/(dashboard)/page.tsx index e286d688..bf6018ec 100644 --- a/app/(dashboard)/page.tsx +++ b/app/(dashboard)/page.tsx @@ -5,6 +5,9 @@ import DashboardContent from '@/components/dashboard/DashboardContent' import { ChecklistSkeleton, PanesSkeleton } from '@/components/dashboard/HemSkeletons' import { COMPANY_PICKED_COOKIE } from '@/lib/company/context' import { isCockpitLandingRole } from '@/lib/company/home-domain' +import { OAUTH_MCP_KEY_NAME } from '@/lib/auth/api-keys' +import { claudeStepDone } from '@/lib/onboarding/checklist' +import { createServiceClient } from '@/lib/supabase/server' import { getDashboardAuthContext, getDashboardCompanyId, @@ -71,7 +74,22 @@ export default async function DashboardPage() { const now = new Date() - const [settingsRes, { data: profile }, agentProfile, { count: skatteverketTokenCount }] = + // Service role for the OAuth-key count below: api_keys' SELECT policy is + // company_id IN user_company_ids() (20260330130000), so through the user + // client a key minted companyless (company_id NULL, the connect-before- + // signup flow) or bound to a company the user has since archived or left is + // invisible, and the step would stay open for exactly the user who just + // connected. The query filters on user_id explicitly, so no other user's + // rows are reachable. + const serviceClient = await createServiceClient() + + const [ + settingsRes, + { data: profile }, + agentProfile, + { count: skatteverketTokenCount }, + { count: oauthKeyCount, error: oauthKeyError }, + ] = await Promise.all([ getDashboardSettings(), // First name for the greeting. @@ -80,6 +98,18 @@ export default async function DashboardPage() { // The Skatteverket promo below the panes needs this flag in the shell; // the checklist section reads it again for its own step (cheap head count). supabase.from('skatteverket_tokens').select('*', { count: 'exact', head: true }).eq('user_id', user.id).eq('company_id', companyId), + // The checklist's "Anslut till Claude" step is done when the MCP OAuth + // token route has minted a key for this user (claudeStepDone). Keyed on + // the user, not the company: the Claude connection follows the person, + // and the key's company_id is whatever was active at sign-in (or null + // for a companyless signup), so a company filter would miss real + // connections. Revoked rows do not count. + serviceClient + .from('api_keys') + .select('*', { count: 'exact', head: true }) + .eq('user_id', user.id) + .eq('name', OAUTH_MCP_KEY_NAME) + .is('revoked_at', null), ]) // A FAILED settings read must not masquerade as "onboarding not done": @@ -90,6 +120,12 @@ export default async function DashboardPage() { if (settingsError) { throw new Error(`company_settings fetch failed: ${settingsError.message}`) } + // Same rule for the OAuth-key count: a failed query answers count null, + // which claudeStepDone would read as "never connected" and re-open the + // Claude step for a connected user. Surface it instead of guessing. + if (oauthKeyError) { + throw new Error(`api_keys count failed: ${oauthKeyError.message}`) + } // If onboarding is not complete, redirect to onboarding. Exception: a byrå // member who did NOT explicitly pick this company this session goes to the @@ -110,6 +146,7 @@ export default async function DashboardPage() { } const agentBuilt = Boolean(agentProfile?.verified_at) + const hasMcpKey = claudeStepDone({ oauthKeyCount }) const userFirstName = profile?.full_name?.trim().split(/\s+/)[0] ?? null const initialSetup = { path: settings.initial_setup_path ?? null, @@ -137,7 +174,7 @@ export default async function DashboardPage() { userId={user.id} now={now} initialSetup={initialSetup} - agentBuilt={agentBuilt} + hasMcpKey={hasMcpKey} vatRegistered={settings.vat_registered} momsPeriod={settings.moms_period ?? null} /> diff --git a/app/api/mcp-oauth/token/route.ts b/app/api/mcp-oauth/token/route.ts index a0f9e5bb..a2600fd5 100644 --- a/app/api/mcp-oauth/token/route.ts +++ b/app/api/mcp-oauth/token/route.ts @@ -2,6 +2,7 @@ import { NextResponse } from 'next/server' import { decryptAuthCode, verifyPkce, hashAuthCode } from '@/lib/auth/oauth-codes' import { generateApiKey, + OAUTH_MCP_KEY_NAME, generateRefreshToken, hashRefreshToken, createServiceClientNoCookies, @@ -161,7 +162,7 @@ async function handleAuthorizationCodeGrant(params: URLSearchParams) { company_id: companyId, key_hash: hash, key_prefix: prefix, - name: 'MCP-klient (OAuth)', + name: OAUTH_MCP_KEY_NAME, scopes: grantedScopes, refresh_token_hash: refresh.hash, }) diff --git a/app/api/settings/api-keys/__tests__/route.test.ts b/app/api/settings/api-keys/__tests__/route.test.ts index 31b73f04..eff16540 100644 --- a/app/api/settings/api-keys/__tests__/route.test.ts +++ b/app/api/settings/api-keys/__tests__/route.test.ts @@ -108,6 +108,24 @@ describe('POST /api/settings/api-keys', () => { expect(body.error.code).toBe('API_KEY_SCOPE_INVALID') }) + it('returns 400 for the reserved OAuth marker name (would fake a Claude connection)', async () => { + const { insertSpy } = setupFrom({ count: 0 }) + const res = await POST( + createMockRequest('/api/settings/api-keys', { + method: 'POST', + body: { name: ' MCP-klient (OAuth) ', scopes: ['reports:read'] }, + }), + { params: Promise.resolve({}) }, + ) + const { status, body } = await parseJsonResponse<{ + error: { code: string; details: { field: string; reason: string } } + }>(res) + expect(status).toBe(400) + expect(body.error.code).toBe('VALIDATION_ERROR') + expect(body.error.details).toMatchObject({ field: 'name', reason: 'reserved' }) + expect(insertSpy).not.toHaveBeenCalled() + }) + it('returns 409 API_KEY_SOD_CONFLICT for stage+approve without acknowledgement', async () => { setupFrom({ count: 0 }) const res = await POST( diff --git a/app/api/settings/api-keys/route.ts b/app/api/settings/api-keys/route.ts index a623d657..cd95e8ed 100644 --- a/app/api/settings/api-keys/route.ts +++ b/app/api/settings/api-keys/route.ts @@ -2,6 +2,7 @@ import { NextResponse } from 'next/server' import { generateApiKey, DEFAULT_SCOPES, + OAUTH_MCP_KEY_NAME, validateScopes, findStageApproveConflict, } from '@/lib/auth/api-keys' @@ -68,6 +69,17 @@ export const POST = withRouteContext( // Empty body: use defaults. } + // The OAuth token route's key name is the marker the Hem checklist reads + // as "connected to Claude" (there is no source column). A hand-minted key + // with that name would tick the step without any connection, so the name + // is reserved for the OAuth path. + if (name.trim() === OAUTH_MCP_KEY_NAME) { + return errorResponseFromCode('VALIDATION_ERROR', log, { + requestId, + details: { field: 'name', reason: 'reserved', reserved: OAUTH_MCP_KEY_NAME }, + }) + } + // Both live and test keys bind to the active company. A test key is // simulation-only (the v1 wrapper forces dry-run on every write) so it can // safely point at the real company without ever persisting anything. diff --git a/components/onboarding/NewUserChecklist.tsx b/components/onboarding/NewUserChecklist.tsx index 922683b6..26adc689 100644 --- a/components/onboarding/NewUserChecklist.tsx +++ b/components/onboarding/NewUserChecklist.tsx @@ -3,7 +3,7 @@ import { useEffect, useRef, useState } from 'react' import Link from 'next/link' import { useRouter } from 'next/navigation' -import { useTranslations } from 'next-intl' +import { useLocale, useTranslations } from 'next-intl' import { Check } from 'lucide-react' import posthog from 'posthog-js' import { Badge } from '@/components/ui/badge' @@ -13,7 +13,12 @@ import { cn } from '@/lib/utils' import { useErrorToast } from '@/lib/hooks/use-error-toast' import { useFormat } from '@/lib/hooks/use-format' import { isAnalyticsEnabled } from '@/lib/analytics/enabled' -import { checklistNumbers, type VatDeadlineLine } from '@/lib/onboarding/checklist' +import { + checklistNumbers, + claudeConnectorLink, + completionPatchBody, + type VatDeadlineLine, +} from '@/lib/onboarding/checklist' import { ENABLED_EXTENSION_IDS } from '@/lib/extensions/_generated/enabled-extensions' import { useCapability } from '@/contexts/CompanyContext' import { CAPABILITY } from '@/lib/entitlements/keys' @@ -27,7 +32,11 @@ interface NewUserChecklistProps { hasBankConnected?: boolean hasSkatteverketConnected?: boolean hasInboxItems?: boolean - hasAgentBuilt?: boolean + /** The user holds a live OAuth-minted MCP key, i.e. a Claude (or other + * MCP client) connection completed its first sign-in. This is the only + * signal that means "connected to Claude"; the in-app AI-profile flag + * used to tick this step and never corresponded to it (issue #2133). */ + hasMcpKey?: boolean /** Personalized VAT-deadline line for the Skatteverket step (null = say nothing). */ vatLine?: VatDeadlineLine /** Latest SIE reconciliation-sweep outcome: surfaces "X matchade, Y att @@ -74,11 +83,12 @@ export default function NewUserChecklist({ hasBankConnected = false, hasSkatteverketConnected = false, hasInboxItems = false, - hasAgentBuilt = false, + hasMcpKey = false, vatLine = null, sieSweep = null, }: NewUserChecklistProps) { const t = useTranslations('initial_setup') + const locale = useLocale() const { appName } = useBranding() const router = useRouter() const showError = useErrorToast() @@ -92,6 +102,12 @@ export default function NewUserChecklist({ // step (companies whose completedAt arrives from the server never see it). const [retiring, setRetiring] = useState<'verdict' | 'closing' | 'done' | null>(null) const retireStartedRef = useRef(false) + // A completion PATCH the server rejected (4xx: no write role, no settings + // row) must not be retried in a loop: `saving` is a dependency of the + // completion effect, so without this latch every rejection re-armed the + // effect and re-raised the error toast forever. The next visit tries once + // more from server truth. + const completeRejectedRef = useRef(false) // The ChatGPT side door on the Claude step: collapsed by default so the // one-click Claude path stays the visual primary. const [chatGptOpen, setChatGptOpen] = useState(false) @@ -134,7 +150,7 @@ export default function NewUserChecklist({ // Companies built without the skatteverket/inbox extensions skip those steps. const step3Done = !hasSkatteverket || hasSkatteverketConnected const step4Done = !hasInbox || hasInboxItems - const step5Done = hasAgentBuilt + const step5Done = hasMcpKey useEffect(() => { // The block retires itself once every step is done; Dölj remains the @@ -143,14 +159,18 @@ export default function NewUserChecklist({ if ( !state.completedAt && step1Done && step2Done && step3Done && step4Done && step5Done && - saving === null + saving === null && + !completeRejectedRef.current ) { if (!retireStartedRef.current) { retireStartedRef.current = true setRetiring('verdict') } - void persist({ completed: true }, 'complete').then((updated) => { + // completionPatchBody supplies a path when none was recorded: the route + // refuses completed:true without one, and this cohort looped on a 400. + void persist(completionPatchBody(state.path), 'complete').then((updated) => { if (updated) captureSetup('onboarding_setup_completed', { path: updated.path }) + else completeRejectedRef.current = true }) } // persist intentionally stays out: its identity follows the toast hook and @@ -175,8 +195,9 @@ export default function NewUserChecklist({ if (state.dismissedAt) return null // After the beat, stay retired even while the completion PATCH is still in // flight or retrying: falling through to the full checklist here would - // flash it after the verdict already played. A failed PATCH keeps retrying - // invisibly; the next visit renders from server truth either way. + // flash it after the verdict already played. A rejected PATCH is not + // retried this session (completeRejectedRef); the next visit renders from + // server truth either way. if (retiring === 'done') return null if (state.completedAt && !retiring) return null @@ -231,9 +252,11 @@ export default function NewUserChecklist({ // page origin so self-hosted and white-label domains link to themselves. const goClaude = () => { captureSetup('onboarding_setup_step_started', { step: 'claude' }) - const serverUrl = `${window.location.origin}/api/extensions/ext/mcp-server/mcp?tool_namespace=accounted` - const link = `https://claude.ai/customize/connectors?modal=add-custom-connector&connectorName=${encodeURIComponent(appName)}&connectorUrl=${encodeURIComponent(serverUrl)}` - window.open(link, '_blank', 'noopener') + window.open( + claudeConnectorLink({ origin: window.location.origin, appName }), + '_blank', + 'noopener', + ) } // ChatGPT has no add-connector deep link (the user pastes the server URL // into Developer mode manually), so the side door copies the URL instead. @@ -428,6 +451,22 @@ export default function NewUserChecklist({ )} footnote={
+ {/* What the click leads to on Claude's side. Tools list before + any sign-in (lazy auth, by design), so without this line a + "connected" status with an unanswered first question reads + as a broken connection (issue #2133). The guide carries the + full sequence; one language per URL, see ApiKeysPanel. */} +

+ {t('step_claude_expectation')}{' '} + + {t('step_claude_guide_link')} + +