fix(api): read contracts by import, and retire two more stale declarations

Greptile flagged the audit's brace counter as blind to braces inside strings,
template literals, regexes and comments. It was, but the bigger problem was that
a text scan can only see contracts whose `method`/`path` are inline literals —
the 70-plus built through `definePostSelector(path, …)` and friends were never
checked at all. Comparing raw `defineRouteContract(` occurrences against parsed
ones showed the scanner silently skipping declarations.

Read the contracts by importing each contract module and inspecting its exported
objects instead, the way `check-route-verbs.ts` already resolves the contract
behind a route. Contract modules are pure Zod so importing them is safe; route
files stay a static scan because importing one drags in `@sim/db`, auth and
`next/server`. Barrels re-export the same object, so entries are keyed by
identity. Coverage goes from 1125 contracts to 1283.

That immediately surfaced two more instances of exactly what this PR retires.
`/api/tools/confluence/page` kept its `PUT` and `DELETE` contracts after #7179
reduced the route to the selector `POST`, so both declared verbs the live route
answers with 405. Neither is fetched — `lib/internal/confluence/execute-tool.ts`
is the only consumer — so they become plain schemas like the knowledge one, and
`executeOperation` now delegates to a schema form rather than growing a second
pattern beside it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: icecrasher321

apps/sim/lib/api/contracts/selectors/confluence.ts

@@ -399,14 +399,16 @@ export const confluencePageSelectorContract = definePostSelector(
   z.object({ id: z.string(), title: z.string() }).passthrough()
 )
 
-export const confluenceUpdatePageContract = defineConfluencePutContract(
-  '/api/tools/confluence/page',
-  confluenceUpdatePageBodySchema
-)
-export const confluenceDeletePageContract = defineConfluenceDeleteContract(
-  '/api/tools/confluence/page',
-  confluenceDeletePageBodySchema
-)
+/**
+ * Page update and delete have no contract because they have no route: the
+ * `PUT`/`DELETE` handlers on `/api/tools/confluence/page` were retired when the
+ * tool moved in process, and the surviving selector `POST` on that path would
+ * answer either verb with 405. `lib/internal/confluence/execute-tool.ts`
+ * validates both against `confluenceUpdatePageBodySchema` /
+ * `confluenceDeletePageBodySchema` directly.
+ */
+export type ConfluenceUpdatePageBody = z.output<typeof confluenceUpdatePageBodySchema>
+export type ConfluenceDeletePageBody = z.output<typeof confluenceDeletePageBodySchema>
 export const confluenceDeleteAttachmentContract = defineConfluenceDeleteContract(
   '/api/tools/confluence/attachment',
   confluenceDeleteAttachmentBodySchema
@@ -562,8 +564,6 @@ export const confluenceUserContract = defineConfluencePostContract(
 
 export type ConfluencePagesBody = ContractBody<typeof confluencePagesSelectorContract>
 export type ConfluencePageBody = ContractBody<typeof confluencePageSelectorContract>
-export type ConfluenceUpdatePageBody = ContractBody<typeof confluenceUpdatePageContract>
-export type ConfluenceDeletePageBody = ContractBody<typeof confluenceDeletePageContract>
 export type ConfluenceDeleteAttachmentBody = ContractBody<typeof confluenceDeleteAttachmentContract>
 export type ConfluenceListAttachmentsQuery = ContractQuery<typeof confluenceListAttachmentsContract>
 export type ConfluenceListBlogPostsQuery = ContractQuery<typeof confluenceListBlogPostsContract>

apps/sim/lib/internal/confluence/execute-tool.ts

@@ -1,5 +1,10 @@
 import { getErrorMessage } from '@sim/utils/errors'
-import type { AnyApiRouteContract, ContractBody, ContractQuery } from '@/lib/api/contracts'
+import type {
+  AnyApiRouteContract,
+  ApiSchema,
+  ContractBody,
+  ContractQuery,
+} from '@/lib/api/contracts'
 import {
   confluenceBlogPostOperationContract,
   confluenceCreateCommentContract,
@@ -10,7 +15,7 @@ import {
   confluenceDeleteBlogPostContract,
   confluenceDeleteCommentContract,
   confluenceDeleteLabelContract,
-  confluenceDeletePageContract,
+  confluenceDeletePageBodySchema,
   confluenceDeletePagePropertyContract,
   confluenceDeleteSpaceContract,
   confluenceGetSpaceContract,
@@ -37,7 +42,7 @@ import {
   confluenceTasksContract,
   confluenceUpdateBlogPostContract,
   confluenceUpdateCommentContract,
-  confluenceUpdatePageContract,
+  confluenceUpdatePageBodySchema,
   confluenceUpdateSpaceContract,
   confluenceUploadAttachmentContract,
   confluenceUserContract,
@@ -94,12 +99,10 @@ import type {
 
 type ContractInput<C extends AnyApiRouteContract> = NonNullable<ContractBody<C> | ContractQuery<C>>
 
-function parsePreparedRequest<C extends AnyApiRouteContract>(
-  contract: C,
+function parsePreparedInput<T>(
+  schema: ApiSchema,
   request: InternalToolOperationCall
-): { success: true; data: ContractInput<C> } | { success: false; response: Response } {
-  const schema = contract.query ?? contract.body
-  if (!schema) throw new Error(`Confluence contract ${contract.path} has no request input`)
+): { success: true; data: T } | { success: false; response: Response } {
   const parsed = schema.safeParse(request.input)
   if (!parsed.success) {
     return {
@@ -110,16 +113,21 @@ function parsePreparedRequest<C extends AnyApiRouteContract>(
       ),
     }
   }
-  return { success: true, data: parsed.data as ContractInput<C> }
+  return { success: true, data: parsed.data as T }
 }
 
-async function executeOperation<C extends AnyApiRouteContract>(
-  contract: C,
+/**
+ * Operations whose HTTP route was retired hold a bare request schema rather than
+ * a contract, so they cannot declare a `method` and `path` nothing serves. The
+ * contract form below feeds this the schema it would have parsed anyway.
+ */
+async function executeSchemaOperation<T>(
+  schema: ApiSchema,
   request: InternalToolOperationCall,
-  execute: (input: ContractInput<C>, context: ConfluenceOperationContext) => Promise<unknown>
+  execute: (input: T, context: ConfluenceOperationContext) => Promise<unknown>
 ): Promise<Response> {
   request.signal?.throwIfAborted()
-  const parsed = parsePreparedRequest(contract, request)
+  const parsed = parsePreparedInput<T>(schema, request)
   if (!parsed.success) return parsed.response
   try {
     const result = await execute(parsed.data, {
@@ -141,6 +149,16 @@ async function executeOperation<C extends AnyApiRouteContract>(
   }
 }
 
+function executeOperation<C extends AnyApiRouteContract>(
+  contract: C,
+  request: InternalToolOperationCall,
+  execute: (input: ContractInput<C>, context: ConfluenceOperationContext) => Promise<unknown>
+): Promise<Response> {
+  const schema = contract.query ?? contract.body
+  if (!schema) throw new Error(`Confluence contract ${contract.path} has no request input`)
+  return executeSchemaOperation<ContractInput<C>>(schema, request, execute)
+}
+
 export const executeConfluenceTool: InternalToolOperationHandler = async (request) => {
   switch (request.toolId) {
     case 'confluence_add_label':
@@ -194,7 +212,11 @@ export const executeConfluenceTool: InternalToolOperationHandler = async (reques
     case 'confluence_delete_label':
       return executeOperation(confluenceDeleteLabelContract, request, executeConfluenceDeleteLabel)
     case 'confluence_delete_page':
-      return executeOperation(confluenceDeletePageContract, request, executeConfluenceDeletePage)
+      return executeSchemaOperation(
+        confluenceDeletePageBodySchema,
+        request,
+        executeConfluenceDeletePage
+      )
     case 'confluence_delete_page_property':
       return executeOperation(
         confluenceDeletePagePropertyContract,
@@ -327,7 +349,11 @@ export const executeConfluenceTool: InternalToolOperationHandler = async (reques
         executeConfluenceSearchInSpace
       )
     case 'confluence_update':
-      return executeOperation(confluenceUpdatePageContract, request, executeConfluenceUpdatePage)
+      return executeSchemaOperation(
+        confluenceUpdatePageBodySchema,
+        request,
+        executeConfluenceUpdatePage
+      )
     case 'confluence_update_blogpost':
       return executeOperation(
         confluenceUpdateBlogPostContract,

scripts/check-api-contract-routes.ts

@@ -15,7 +15,18 @@
  * verb, endpoint is fine" and sends the caller looking in the wrong place. That
  * is the only case this script rejects, so it stays silent on the in-process
  * contracts whose routes were deleted outright.
+ *
+ * Contracts are read by importing each contract module and inspecting its
+ * exported objects, the same way `check-route-verbs.ts` resolves the contract
+ * behind a route. Scanning the source text instead would have to re-implement a
+ * TypeScript lexer to know which braces are code and which sit inside a string,
+ * template literal, regex or comment, and it could only ever see contracts whose
+ * `method`/`path` are inline literals — the 70-plus built through helpers like
+ * `definePostSelector(path, …)` would be invisible. Route files stay a static
+ * scan on purpose: importing one drags in `@sim/db`, auth and `next/server`,
+ * whereas contract modules are pure Zod.
  */
+import { existsSync } from 'node:fs'
 import { readdir, readFile, stat } from 'node:fs/promises'
 import path from 'node:path'
 
@@ -25,52 +36,35 @@ const APP_API_DIR = path.join(ROOT, 'apps/sim/app/api')
 const SKIP_DIRS = new Set(['node_modules', 'dist', '.next', '.turbo', 'coverage', '__tests__'])
 const HTTP_METHODS = ['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'HEAD', 'OPTIONS'] as const
 
+type HttpMethod = (typeof HTTP_METHODS)[number]
+
 interface DeclaredContract {
   name: string
-  method: string
+  method: HttpMethod
   routePath: string
-  file: string
-  line: number
+  module: string
 }
 
-async function walk(dir: string, results: string[] = []): Promise<string[]> {
+async function listContractModules(dir: string, results: string[] = []): Promise<string[]> {
   for (const entry of await readdir(dir, { withFileTypes: true })) {
     if (SKIP_DIRS.has(entry.name)) continue
     const full = path.join(dir, entry.name)
-    if (entry.isDirectory()) await walk(full, results)
+    if (entry.isDirectory()) await listContractModules(full, results)
     else if (/\.tsx?$/.test(entry.name) && !/\.test\.tsx?$/.test(entry.name)) results.push(full)
   }
   return results
 }
 
-/** Reads the balanced object literal passed to each `defineRouteContract(` call. */
-function parseContracts(source: string, file: string): DeclaredContract[] {
-  const found: DeclaredContract[] = []
-  const opener = /defineRouteContract\s*\(\s*\{/g
-  let match: RegExpExecArray | null
-  while ((match = opener.exec(source))) {
-    let cursor = match.index + match[0].length - 1
-    const start = cursor
-    let depth = 0
-    for (; cursor < source.length; cursor++) {
-      const char = source[cursor]
-      if (char === '{') depth++
-      else if (char === '}' && --depth === 0) break
-    }
-    const literal = source.slice(start, cursor + 1)
-    const method = literal.match(/(?:^|[\s,{])method\s*:\s*'([A-Z]+)'/)?.[1]
-    const routePath = literal.match(/(?:^|[\s,{])path\s*:\s*'([^']+)'/)?.[1]
-    if (!method || !routePath) continue
-    const preceding = source.slice(0, match.index)
-    found.push({
-      name: [...preceding.matchAll(/export\s+const\s+([A-Za-z0-9_]+)/g)].pop()?.[1] ?? 'anonymous',
-      method,
-      routePath,
-      file: path.relative(ROOT, file),
-      line: preceding.split('\n').length,
-    })
-  }
-  return found
+function isRouteContract(value: unknown): value is { method: HttpMethod; path: string } {
+  if (typeof value !== 'object' || value === null) return false
+  const candidate = value as Record<string, unknown>
+  return (
+    typeof candidate.method === 'string' &&
+    (HTTP_METHODS as readonly string[]).includes(candidate.method) &&
+    typeof candidate.path === 'string' &&
+    typeof candidate.response === 'object' &&
+    candidate.response !== null
+  )
 }
 
 async function readIfFile(candidate: string): Promise<string | null> {
@@ -98,13 +92,8 @@ async function readRouteFile(routePath: string): Promise<string | null> {
 
   for (let depth = segments.length; depth > 0; depth--) {
     const ancestor = path.join(APP_API_DIR, ...segments.slice(0, depth - 1))
-    let entries
-    try {
-      entries = await readdir(ancestor, { withFileTypes: true })
-    } catch {
-      continue
-    }
-    for (const entry of entries) {
+    if (!existsSync(ancestor)) continue
+    for (const entry of await readdir(ancestor, { withFileTypes: true })) {
       if (!entry.isDirectory()) continue
       if (!entry.name.startsWith('[...') && !entry.name.startsWith('[[...')) continue
       const source = await readIfFile(path.join(ancestor, entry.name, 'route.ts'))
@@ -134,11 +123,42 @@ function exportedMethods(source: string): Set<string> {
   return methods
 }
 
-async function main() {
-  const contracts: DeclaredContract[] = []
-  for (const file of await walk(CONTRACTS_DIR)) {
-    contracts.push(...parseContracts(await readFile(file, 'utf8'), file))
+async function collectContracts(): Promise<DeclaredContract[]> {
+  const modules = await listContractModules(CONTRACTS_DIR)
+  // Barrels re-export the same object, so keying by identity keeps one entry per
+  // contract. Defining modules sort before `index.ts` so the report names them.
+  modules.sort((a, b) => {
+    const aBarrel = path.basename(a) === 'index.ts'
+    const bBarrel = path.basename(b) === 'index.ts'
+    return aBarrel === bBarrel ? a.localeCompare(b) : aBarrel ? 1 : -1
+  })
+
+  const seen = new Map<object, DeclaredContract>()
+  for (const file of modules) {
+    let loaded: Record<string, unknown>
+    try {
+      loaded = (await import(file)) as Record<string, unknown>
+    } catch (error) {
+      console.error(`✗ Could not import ${path.relative(ROOT, file)} to read its contracts:`)
+      console.error(`  ${error instanceof Error ? error.message : String(error)}`)
+      process.exit(1)
+    }
+    for (const [name, value] of Object.entries(loaded)) {
+      if (!isRouteContract(value)) continue
+      if (seen.has(value)) continue
+      seen.set(value, {
+        name,
+        method: value.method,
+        routePath: value.path,
+        module: path.relative(ROOT, file),
+      })
+    }
   }
+  return [...seen.values()]
+}
+
+async function main() {
+  const contracts = await collectContracts()
 
   const violations: Array<DeclaredContract & { served: string[] }> = []
   const inProcess: DeclaredContract[] = []
@@ -149,14 +169,12 @@ async function main() {
       continue
     }
     const served = exportedMethods(routeSource)
-    if (!served.has(contract.method)) {
-      violations.push({ ...contract, served: [...served].sort() })
-    }
+    if (!served.has(contract.method)) violations.push({ ...contract, served: [...served].sort() })
   }
 
   if (process.argv.includes('--list-in-process')) {
     for (const c of [...inProcess].sort((a, b) => a.routePath.localeCompare(b.routePath))) {
-      console.log(`  ${c.method.padEnd(6)} ${c.routePath}  ${c.name} (${c.file}:${c.line})`)
+      console.log(`  ${c.method.padEnd(6)} ${c.routePath}  ${c.name} (${c.module})`)
     }
   }
 
@@ -166,7 +184,7 @@ async function main() {
     )
     for (const v of violations) {
       console.error(`  ${v.method} ${v.routePath}`)
-      console.error(`    contract: ${v.name} (${v.file}:${v.line})`)
+      console.error(`    contract: ${v.name} (${v.module})`)
       console.error(`    route serves: ${v.served.join(', ') || '(no methods)'}`)
       console.error(
         `    fix: export ${v.method} from the route, or drop the declaration if the endpoint is retired\n`