code review fixes

gallayl · gallayl · commit 9a4220fc5bce · 2026-02-21T09:46:46.000+01:00
diff --git a/frontend/src/components/service-table.tsx b/frontend/src/components/service-table.tsx
@@ -100,12 +100,7 @@ export const ServiceTable = Shade<ServiceTableProps>({
                 />
               </NestedRouteLink>
               <NestedRouteLink href={`/services/${entry.id}?${serializeToQueryString({ edit: true })}`}>
-                <Button
-                  variant="text"
-                  size="small"
-                  title="Edit"
-                  startIcon={<Icon icon={icons.edit} size="small" />}
-                />
+                <Button variant="text" size="small" title="Edit" startIcon={<Icon icon={icons.edit} size="small" />} />
               </NestedRouteLink>
             </div>
           ),
diff --git a/frontend/src/pages/dashboard/index.tsx b/frontend/src/pages/dashboard/index.tsx
@@ -108,7 +108,7 @@ export const Dashboard = Shade<DashboardProps>({
           title={currentStack?.displayName ?? props.stackName}
           description={currentStack?.description}
           actions={
-            <NestedRouteLink href="/stacks/:stackName/edit" params={{ stackName: props.stackName! }}>
+            <NestedRouteLink href="/stacks/:stackName/edit" params={{ stackName: props.stackName }}>
               <Button variant="outlined" startIcon={<Icon icon={icons.edit} size="small" />}>
                 Edit Stack
               </Button>
@@ -161,7 +161,7 @@ export const Dashboard = Shade<DashboardProps>({
               </Button>
             ) : null}
             <div style={{ flex: '1' }} />
-            <NestedRouteLink href="/services/wizard/:stackName" params={{ stackName: props.stackName! }}>
+            <NestedRouteLink href="/services/wizard/:stackName" params={{ stackName: props.stackName }}>
               <Button variant="contained" startIcon={<Icon icon={icons.plus} size="small" />}>
                 Create Service
               </Button>
@@ -189,7 +189,7 @@ export const Dashboard = Shade<DashboardProps>({
             }}
           >
             <h3 style={{ margin: '0', fontSize: '16px' }}>Repositories ({repos.length})</h3>
-            <NestedRouteLink href="/repositories/create/:stackName" params={{ stackName: props.stackName! }}>
+            <NestedRouteLink href="/repositories/create/:stackName" params={{ stackName: props.stackName }}>
               <Button variant="outlined" startIcon={<Icon icon={icons.plus} size="small" />}>
                 Add Repository
               </Button>
diff --git a/frontend/src/pages/services/service-detail.tsx b/frontend/src/pages/services/service-detail.tsx
@@ -38,7 +38,7 @@ const eventLabels: Record<string, string> = {
   'build-completed': 'Build completed',
   'build-failed': 'Build failed',
   'pull-completed': 'Pull completed',
-  'imported': 'Imported',
+  imported: 'Imported',
 }
 
 type ServiceDetailProps = {
diff --git a/frontend/src/pages/stacks/create-stack.tsx b/frontend/src/pages/stacks/create-stack.tsx
@@ -40,11 +40,7 @@ export const CreateStack = Shade({
       <PageContainer>
         <PageHeader icon="➕" title="Create Stack" description="Set up a new stack to manage your services." />
         <Paper>
-          <StackForm
-            mode="create"
-            onSubmit={(data) => void handleSubmit(data)}
-            cancelHref="/"
-          />
+          <StackForm mode="create" onSubmit={(data) => void handleSubmit(data)} cancelHref="/" />
         </Paper>
       </PageContainer>
     )
diff --git a/frontend/src/pages/stacks/edit-stack.tsx b/frontend/src/pages/stacks/edit-stack.tsx
@@ -146,12 +146,7 @@ export const EditStack = Shade<EditStackProps>({
           }
         />
         <Paper>
-          <StackForm
-            mode="edit"
-            initial={stack}
-            onSubmit={(data) => void handleSave(data)}
-            cancelHref="/"
-          />
+          <StackForm mode="edit" initial={stack} onSubmit={(data) => void handleSave(data)} cancelHref="/" />
         </Paper>
         {isConfirmingDelete ? (
           <ConfirmDialog
diff --git a/service/src/app-models/data-store/setup-data-store.ts b/service/src/app-models/data-store/setup-data-store.ts
@@ -20,7 +20,7 @@ import type { Options, Sequelize } from 'sequelize'
 import sqlite from 'sqlite3'
 import { join } from 'path'
 
-import { authorizedDataSet, dataDir } from '../../config.js'
+import { authorizedDataSet, dataDir, ensureDataDir } from '../../config.js'
 
 // --- Sequelize Model classes ---
 
@@ -389,6 +389,7 @@ async function initAllModels(sequelize: Sequelize): Promise<void> {
 }
 
 export const setupDataStore = async (injector: Injector) => {
+  ensureDataDir()
   const logger = getLogger(injector).withScope('DataStore')
   const dbOptions = getDbOptions()
 
diff --git a/service/src/app-models/stacks/setup-stacks-rest-api.ts b/service/src/app-models/stacks/setup-stacks-rest-api.ts
@@ -117,8 +117,14 @@ export const setupStacksRestApi = async (injector: Injector) => {
           const svcs = await svcDs.find(i, { filter: { stackName: { $eq: id } }, select: ['id'] })
           const svcIds = svcs.map((svc) => svc.id)
           if (svcIds.length > 0) {
-            await repo.getDataSetFor(ServiceStatus, 'serviceId').remove(i, ...svcIds).catch(() => {})
-            await repo.getDataSetFor(ServiceConfig, 'serviceId').remove(i, ...svcIds).catch(() => {})
+            await repo
+              .getDataSetFor(ServiceStatus, 'serviceId')
+              .remove(i, ...svcIds)
+              .catch(() => {})
+            await repo
+              .getDataSetFor(ServiceConfig, 'serviceId')
+              .remove(i, ...svcIds)
+              .catch(() => {})
             await svcDs.remove(i, ...svcIds).catch(() => {})
           }
 
@@ -127,15 +133,21 @@ export const setupStacksRestApi = async (injector: Injector) => {
             .find(i, { filter: { stackName: { $eq: id } }, select: ['id'] })
           const repoIds = repos.map((r) => r.id)
           if (repoIds.length > 0) {
-            await repo.getDataSetFor(GitHubRepository, 'id').remove(i, ...repoIds).catch(() => {})
+            await repo
+              .getDataSetFor(GitHubRepository, 'id')
+              .remove(i, ...repoIds)
+              .catch(() => {})
           }
 
           const deps = await repo
             .getDataSetFor(Dependency, 'id')
             .find(i, { filter: { stackName: { $eq: id } }, select: ['id'] })
           const depIds = deps.map((d) => d.id)
           if (depIds.length > 0) {
-            await repo.getDataSetFor(Dependency, 'id').remove(i, ...depIds).catch(() => {})
+            await repo
+              .getDataSetFor(Dependency, 'id')
+              .remove(i, ...depIds)
+              .catch(() => {})
           }
 
           await repo
diff --git a/service/src/config.ts b/service/src/config.ts
@@ -10,7 +10,10 @@ import { mkdirSync } from 'fs'
 import { join } from 'path'
 
 export const dataDir = process.env.STACK_CRAFT_DATA_DIR || join(process.cwd(), 'data')
-mkdirSync(dataDir, { recursive: true })
+
+export const ensureDataDir = () => {
+  mkdirSync(dataDir, { recursive: true })
+}
 
 export const authorizedOnly = async (options: { injector: Injector }): Promise<AuthorizationResult> => {
   const isAllowed = await isAuthenticated(options.injector)
diff --git a/service/src/mcp/mcp-server.ts b/service/src/mcp/mcp-server.ts
@@ -237,7 +237,7 @@ export const createMcpRequestHandler = (injector: Injector, sessionManager: McpS
   return async (req: IncomingMessage, res: ServerResponse) => {
     try {
       const authHeader = req.headers.authorization
-      const user = await resolveTokenUser(injector, authHeader)
+      const user = await resolveTokenUser(injector, authHeader, elevated)
       if (!user) {
         res.writeHead(401, { 'Content-Type': 'application/json' })
         res.end(JSON.stringify({ error: 'Unauthorized. Provide a valid Bearer token.' }))
diff --git a/service/src/mcp/setup-mcp.ts b/service/src/mcp/setup-mcp.ts
@@ -51,12 +51,14 @@ export class McpHttpServer {
   }
 }
 
+export const getMcpPort = (env = process.env) => parseInt(env.MCP_PORT as string, 10) || 9091
+
 /**
  * Sets up the MCP endpoint on a separate port.
  * MCP clients connect to this endpoint using Streamable HTTP transport.
  */
 export const setupMcp = (injector: Injector) => {
-  const port = parseInt(process.env.MCP_PORT as string, 10) || 9091
+  const port = getMcpPort()
   const mcpServer = injector.getInstance(McpHttpServer)
   mcpServer.listen(injector, port)
 }
diff --git a/service/src/middleware/bearer-token-auth.ts b/service/src/middleware/bearer-token-auth.ts
@@ -10,16 +10,23 @@ import { useSystemIdentityContext } from '@furystack/core'
  * Extracts a Bearer token from the Authorization header,
  * looks up the matching ApiToken, and returns the associated user.
  * Returns null if no token or token not found.
+ *
+ * When an existing elevated injector is provided it will be reused
+ * instead of creating (and disposing) a temporary one per call.
  */
-export const resolveTokenUser = async (injector: Injector, authHeader: string | undefined): Promise<User | null> => {
+export const resolveTokenUser = async (
+  injector: Injector,
+  authHeader: string | undefined,
+  existingElevated?: Injector,
+): Promise<User | null> => {
   if (!authHeader?.startsWith('Bearer ')) {
     return null
   }
 
   const plainToken = authHeader.slice(7)
   const tokenHash = createHash('sha256').update(plainToken).digest('hex')
 
-  const elevated = useSystemIdentityContext({ injector })
+  const elevated = existingElevated ?? useSystemIdentityContext({ injector })
   try {
     const repository = getRepository(elevated)
     const tokenDs = repository.getDataSetFor(ApiToken, 'id')
@@ -40,6 +47,8 @@ export const resolveTokenUser = async (injector: Injector, authHeader: string |
     const users = await userDs.find(elevated, { filter: { username: { $eq: token.username } }, top: 1 })
     return users[0] ?? null
   } finally {
-    await elevated[Symbol.asyncDispose]()
+    if (!existingElevated) {
+      await elevated[Symbol.asyncDispose]()
+    }
   }
 }
diff --git a/service/src/services/log-storage-service.ts b/service/src/services/log-storage-service.ts
@@ -76,14 +76,24 @@ export class LogStorageService {
     const elevated = this.getElevatedInjector()
     const ds = getRepository(elevated).getDataSetFor(ServiceLogEntry, 'id')
 
-    const entries = await ds.find(elevated, {
-      filter: { serviceId: { $eq: serviceId } },
-      select: ['id'],
-    })
+    const BATCH_SIZE = 1_000
+    let totalRemoved = 0
+
+    for (;;) {
+      const batch = await ds.find(elevated, {
+        filter: { serviceId: { $eq: serviceId } },
+        select: ['id'],
+        top: BATCH_SIZE,
+      })
+
+      if (batch.length === 0) break
+
+      await ds.remove(elevated, ...batch.map((entry) => entry.id))
+      totalRemoved += batch.length
+    }
 
-    if (entries.length > 0) {
-      await ds.remove(elevated, ...entries.map((entry) => entry.id))
-      await this.logger.information({ message: `Cleared ${entries.length} log entries for service ${serviceId}` })
+    if (totalRemoved > 0) {
+      await this.logger.information({ message: `Cleared ${totalRemoved} log entries for service ${serviceId}` })
     }
   }
 

Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,7 @@ const eventLabels: Record<string, string> = {`
`38`	`38`	`'build-completed': 'Build completed',`
`39`	`39`	`'build-failed': 'Build failed',`
`40`	`40`	`'pull-completed': 'Pull completed',`
`41`		`- 'imported': 'Imported',`
	`41`	`+ imported: 'Imported',`
`42`	`42`	`}`
`43`	`43`
`44`	`44`	`type ServiceDetailProps = {`
Original file line number	Diff line number	Diff line change
`@@ -51,12 +51,14 @@ export class McpHttpServer {`
`51`	`51`	`}`
`52`	`52`	`}`
`53`	`53`
	`54`	`+export const getMcpPort = (env = process.env) => parseInt(env.MCP_PORT as string, 10) \|\| 9091`
	`55`	`+`
`54`	`56`	`/**`
`55`	`57`	`* Sets up the MCP endpoint on a separate port.`
`56`	`58`	`* MCP clients connect to this endpoint using Streamable HTTP transport.`
`57`	`59`	`*/`
`58`	`60`	`export const setupMcp = (injector: Injector) => {`
`59`		`- const port = parseInt(process.env.MCP_PORT as string, 10) \|\| 9091`
	`61`	`+ const port = getMcpPort()`
`60`	`62`	`const mcpServer = injector.getInstance(McpHttpServer)`
`61`	`63`	`mcpServer.listen(injector, port)`
`62`	`64`	`}`