packages/cli/skills/recipe-data-collect.md-44-50 (1)

44-50: ⚠️ Potential issue | 🟡 Minor

File path mismatch will cause command failure.

Step 4 creates /tmp/combined.json (line 41), but step 5 references /tmp/batch.json (line 49), which is never created. Either use the correct file path or add a conversion step.

🔧 Suggested fix

If the combined JSON can be used directly as batch data:

 DOC_ID=$(wafflebase doc create "Combined Report" --format json | jq -r '.id')
 # Convert combined data to batch format and write
-wafflebase cell batch "$DOC_ID" --data "$(cat /tmp/batch.json)"
+wafflebase cell batch "$DOC_ID" --data "$(cat /tmp/combined.json)"

Or, if conversion is needed, add an intermediate step:

# Convert combined data to batch format
jq 'map({range: .range, values: .values})' /tmp/combined.json > /tmp/batch.json
wafflebase cell batch "$DOC_ID" --data "$(cat /tmp/batch.json)"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/cli/skills/recipe-data-collect.md` around lines 44 - 50, Step 5
references /tmp/batch.json which is never created in Step 4 (which writes
/tmp/combined.json), causing the wafflebase cell batch command to fail; fix by
either updating the wafflebase cell batch invocation to consume
/tmp/combined.json or add a conversion step that reads /tmp/combined.json and
writes /tmp/batch.json (so wafflebase cell batch "$DOC_ID" --data "$(cat
/tmp/batch.json)" succeeds). Ensure the patch touches the recipe-data-collect.md
steps around "Optionally write aggregated results to a new document" and the
wafflebase cell batch command to keep paths consistent.

packages/cli/skills/recipe-data-collect.md-1-5 (1)

1-5: ⚠️ Potential issue | 🟡 Minor

Safety metadata contradicts the recipe steps.

The front matter declares safety: read-only, but step 5 (lines 44–50) includes write operations (doc create and cell batch). Either remove the write step or update the safety field to reflect that this recipe can perform writes.

📝 Suggested fix

 ---
 name: recipe-data-collect
 description: Collect and compare data across multiple Wafflebase documents
-safety: read-only
+safety: read-write
 ---

Or, if step 5 should remain optional and the recipe is primarily read-only, consider adding a note in the metadata or description clarifying that writes are optional.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/cli/skills/recipe-data-collect.md` around lines 1 - 5, The front
matter for recipe-data-collect currently sets safety: read-only but step 5 uses
write operations (doc create and cell batch); either change the front-matter
safety to reflect write capability (e.g., safety: read-write) or remove/guard
the write commands in step 5 so the recipe remains read-only; alternatively,
mark the write step as optional in the description/metadata and add a note
clarifying that doc create and cell batch are only executed when explicitly
enabled.

packages/cli/skills/recipe-csv-pipeline.md-3-3 (1)

3-3: ⚠️ Potential issue | 🟡 Minor

Add the missing export step or narrow the recipe promise.

The recipe says it exports results, but the workflow stops at wafflebase cell get in Line 45. Please either add an explicit export step or reword the description so users are not expecting functionality this recipe never demonstrates.

Also applies to: 11-12

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/cli/skills/recipe-csv-pipeline.md` at line 3, The recipe description
claims it "exports results" but the workflow ends at the `wafflebase cell get`
step, so either add an explicit export step after the `wafflebase cell get`
invocation (for example a `wafflebase sheet export` / `wafflebase export`
command that writes CSV/XLSX or an equivalent step that saves the processed
sheet) or change the top-line description to remove "and export results" (narrow
wording to "apply formulas and inspect results" or similar); update the markdown
around the `wafflebase cell get` step and the description line to keep them
consistent.

packages/cli/skills/recipe-csv-pipeline.md-18-20 (1)

18-20: ⚠️ Potential issue | 🟡 Minor

Document the jq prerequisite or avoid requiring it here.

Line 19 depends on jq, but the recipe never tells the reader they need it. On a machine without jq, the very first step fails. While other recipe files also use jq, none of them clearly document it as a prerequisite in the main README.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/cli/skills/recipe-csv-pipeline.md` around lines 18 - 20, The recipe
uses jq to parse the JSON output into DOC_ID (DOC_ID=$(wafflebase doc create "Q1
Analysis" --format json | jq -r '.id')), but jq is not documented as a
prerequisite; either add a short prerequisite note to the recipe (and the main
README) stating that jq is required or replace the pipeline with a jq-free
alternative (e.g., use wafflebase’s built-in flag/output option if available or
a POSIX-safe parser) and update the example to reference DOC_ID and the
wafflebase doc create command so readers know where the value comes from.

packages/cli/src/output/csv.ts-10-10 (1)

10-10: ⚠️ Potential issue | 🟡 Minor

Rename the local escape helper to quoteCsvValue.

Line 10 violates Biome's noShadowRestrictedNames rule—escape shadows a global restricted name. Rename the function and its usages (lines 18, 19) to the more descriptive quoteCsvValue to resolve the violation and clarify intent.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/output/csv.ts` at line 10, The local helper function named
escape shadows a restricted global name; rename the function declaration escape
to quoteCsvValue and update all local references (e.g., the calls currently
written as escape(...)) to quoteCsvValue(...), keeping the same signature and
behavior; also update any local exports or re-uses within the module so the name
change is consistent and Biome's noShadowRestrictedNames rule is satisfied.

packages/cli/src/client/http-client.ts-44-45 (1)

44-45: ⚠️ Potential issue | 🟡 Minor

Type safety: data can be null but is typed as T.

When res.json() fails, data is set to null, but the return type declares it as T. This could cause runtime errors if callers assume data is always the expected type.

🛡️ Proposed fix to update the response type

 export interface ApiResponse<T = unknown> {
   ok: boolean;
   status: number;
-  data: T;
+  data: T | null;
 }

Alternatively, throw an error when JSON parsing fails for non-ok responses, or ensure data is only accessed when ok is true.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/client/http-client.ts` around lines 44 - 45, The variable
data is set to null on JSON parse failure but is typed as T, so update the HTTP
client return typing and handling: change the returned data type from T to T |
null (or make the generic accept nullable like Response<T | null>), or
alternately throw when res.json() fails for non-ok responses and only
parse/return data for successful responses; adjust the function signature and
any callers accordingly to use the nullable type and guard accesses to data.
Reference the data variable and the object returned ({ ok: res.ok, status:
res.status, data }) in packages/cli/src/client/http-client.ts when applying the
change.

packages/cli/src/commands/cell.ts-107-117 (1)

107-117: ⚠️ Potential issue | 🟡 Minor

Add error handling for JSON.parse.

JSON.parse on lines 109 and 116 can throw a SyntaxError if the input is invalid JSON. This exception would propagate uncaught and crash the CLI with an unhelpful stack trace.

🛡️ Proposed fix to wrap JSON.parse in try/catch

       let cells: Record<string, unknown>;
       if (dataStr) {
-        cells = JSON.parse(dataStr);
+        try {
+          cells = JSON.parse(dataStr);
+        } catch {
+          outputError(new Error('Invalid JSON in --data'), opts.quiet);
+          return;
+        }
       } else {
         // Read from stdin
         const chunks: Buffer[] = [];
         for await (const chunk of process.stdin) {
           chunks.push(chunk as Buffer);
         }
-        cells = JSON.parse(Buffer.concat(chunks).toString('utf-8'));
+        try {
+          cells = JSON.parse(Buffer.concat(chunks).toString('utf-8'));
+        } catch {
+          outputError(new Error('Invalid JSON from stdin'), opts.quiet);
+          return;
+        }
       }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/commands/cell.ts` around lines 107 - 117, Wrap both
JSON.parse usages (the one parsing dataStr and the one parsing stdin chunks) in
try/catch blocks so malformed JSON doesn’t throw an uncaught SyntaxError; when
catching, log a clear user-facing error (include the parse error.message and the
raw input or first N chars for context) using the CLI logger or console.error,
and exit with a non-zero status (e.g., process.exit(1)). Update the code around
the cells variable, dataStr and the stdin chunk assembly to validate parsing
inside the try and only assign to cells on success, rethrowing or exiting on
failure to avoid proceeding with an undefined/invalid cells value.

packages/backend/test/api-key-http.e2e-spec.ts-73-134 (1)

73-134: ⚠️ Potential issue | 🟡 Minor

Exercise the slug path in this suite.

Every request here uses workspace.id, so this test never hits the slug→UUID resolution that this PR is fixing in the workspace-scoped guards/controllers. Add at least one happy-path create/list call and one /api/v1/... call with workspace.slug so the regression is covered end to end.

As per coding guidelines "Write tests for critical business logic and edge cases".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/backend/test/api-key-http.e2e-spec.ts` around lines 73 - 134, This
test never exercises slug→UUID resolution because every request uses
workspace.id; add at least one happy-path request that uses workspace.slug for
the workspace-scoped routes. Specifically, in the 'full lifecycle' spec make a
POST to `/workspaces/${workspace.slug}/api-keys` (create API key) and a GET to
`/workspaces/${workspace.slug}/api-keys` (list) or replace one of the existing
create/list calls to use workspace.slug, and also call the v1 documents endpoint
with `/api/v1/workspaces/${workspace.slug}/documents` (use rawKey as Bearer) so
the slug resolution in the workspace-scoped guards/controllers (the create/list
handlers and the GET /api/v1/workspaces/.../documents flow) is exercised
end-to-end; keep assertions identical to the id-based calls.

packages/backend/src/yorkie/yorkie.service.spec.ts-34-37 (1)

34-37: ⚠️ Potential issue | 🟡 Minor

Make the config mock key-specific.

createMockConfigService() returns the Yorkie URL for any get() call, so a typo in the config key inside YorkieService would still make this suite pass. Restrict the mock to the expected key and assert that the service requested that key.

As per coding guidelines "Write tests for critical business logic and edge cases".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/backend/src/yorkie/yorkie.service.spec.ts` around lines 34 - 37, The
mock createMockConfigService currently returns the Yorkie URL for any get()
call, hiding typos in config keys; change createMockConfigService so its get()
is key-specific (e.g., implement jest.fn().mockImplementation(key => key ===
'<expected_config_key>' ? 'http://localhost:8080' : undefined)) so only the
correct key returns the URL, and add an expectation in the spec to assert the
mock was called with that exact key (reference createMockConfigService and the
YorkieService code that calls ConfigService.get).

packages/cli/test/schema.test.ts-25-30 (1)

25-30: ⚠️ Potential issue | 🟡 Minor

Pin the new api-key.* schema entries too.

These assertions never mention the API-key commands added in this PR, so the suite would still pass if one of them were missing from the registry or classified with the wrong safety level.

Suggested assertions

   expect(getCommandSchema('doc.list')!.safety).toBe('read-only');
   expect(getCommandSchema('doc.create')!.safety).toBe('write');
   expect(getCommandSchema('doc.delete')!.safety).toBe('destructive');
+  expect(getCommandSchema('api-key.list')!.safety).toBe('read-only');
+  expect(getCommandSchema('api-key.create')!.safety).toBe('write');
+  expect(getCommandSchema('api-key.revoke')!.safety).toBe('destructive');
   expect(getCommandSchema('cell.set')!.safety).toBe('write');
   expect(getCommandSchema('cell.delete')!.safety).toBe('destructive');

As per coding guidelines "Write tests for critical business logic and edge cases".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/cli/test/schema.test.ts` around lines 25 - 30, The test currently
pins safety levels for several commands but omits the new api-key.* entries; add
assertions using getCommandSchema for each api-key command introduced (e.g.,
getCommandSchema('api-key.create'), getCommandSchema('api-key.list'),
getCommandSchema('api-key.delete')) and assert their expected safety values (for
example: 'write' for create, 'read-only' for list, 'destructive' for delete); if
the PR added additional api-key commands, include analogous assertions for each
to ensure they are registered and have the correct safety classification.

packages/backend/src/api-key/api-key.controller.ts-38-38 (1)

38-38: ⚠️ Potential issue | 🟡 Minor

Unsafe date parsing could produce Invalid Date.

new Date(body.expiresAt) with an invalid string creates an Invalid Date object that won't throw but will cause subtle bugs downstream. If not using a DTO with @IsDateString(), validate the date explicitly.

🛡️ Proposed fix

+    let expiresAt: Date | undefined;
+    if (body.expiresAt) {
+      expiresAt = new Date(body.expiresAt);
+      if (isNaN(expiresAt.getTime())) {
+        throw new BadRequestException('Invalid expiresAt date format');
+      }
+    }
     return this.apiKeyService.create(
       userId,
       resolvedId,
       body.name,
       body.scopes,
-      body.expiresAt ? new Date(body.expiresAt) : undefined,
+      expiresAt,
     );

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/backend/src/api-key/api-key.controller.ts` at line 38, The code
constructs a Date from body.expiresAt using new Date(body.expiresAt) which can
produce an Invalid Date; instead validate the incoming value before creating the
Date: check that body.expiresAt is a string (or undefined), run
Date.parse(body.expiresAt) (or use a strict parser like parseISO/dayjs if
available) and ensure the result is not NaN, then pass new Date(parsed) (or
undefined) to the API key creation; if invalid, return a 400 error or omit the
expiresAt value. Ensure you update the logic around body.expiresAt and new
Date(body.expiresAt) to perform this validation and handle the invalid case.

packages/backend/src/api-key/api-key.strategy.ts-13-28 (1)

13-28: ⚠️ Potential issue | 🟡 Minor

Add error handling for validateKey() exceptions in the strategy.

The apiKeyService.validateKey(token) method throws UnauthorizedException when a key is not found, revoked, or expired. Without error handling in the validate() method, these exceptions propagate unhandled. Passport strategies should return false to indicate authentication failure rather than throw exceptions.

Wrap the key validation in a try-catch block to properly handle failures:

🛡️ Proposed fix

   async validate(req: Request) {
     const authHeader = req.headers.authorization;
     if (!authHeader?.startsWith('Bearer wfb_')) {
       return false;
     }

     const token = authHeader.slice('Bearer '.length);
-    const apiKey = await this.apiKeyService.validateKey(token);
-
-    return {
-      id: apiKey.createdBy,
-      workspaceId: apiKey.workspaceId,
-      scopes: apiKey.scopes,
-      isApiKey: true,
-    };
+    try {
+      const apiKey = await this.apiKeyService.validateKey(token);
+      return {
+        id: apiKey.createdBy,
+        workspaceId: apiKey.workspaceId,
+        scopes: apiKey.scopes,
+        isApiKey: true,
+      };
+    } catch {
+      return false;
+    }
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/backend/src/api-key/api-key.strategy.ts` around lines 13 - 28, The
validate method in api-key.strategy.ts currently calls
this.apiKeyService.validateKey(token) without catching errors causing exceptions
to bubble up; update the async validate(req: Request) function to wrap the call
to this.apiKeyService.validateKey(token) in a try-catch, and on any error return
false (not rethrow) so Passport treats it as authentication failure; reference
the validate method and apiKeyService.validateKey(token) and ensure the function
still returns the same success object on valid key but returns false from the
catch block.

packages/backend/src/api/v1/cells.controller.ts-157-173 (1)

157-173: ⚠️ Potential issue | 🟡 Minor

Consider adding range size limits to prevent resource exhaustion.

The expandRange function can generate very large arrays for wide ranges like A1:ZZ10000, potentially causing memory issues or slow responses. Consider adding reasonable limits.

🛡️ Suggested fix

+const MAX_RANGE_CELLS = 10000;
+
 function expandRange(range: string): string[] | null {
   const match = range.match(/^([A-Z]+)(\d+):([A-Z]+)(\d+)$/i);
   if (!match) return null;

   const startCol = colToIndex(match[1].toUpperCase());
   const startRow = parseInt(match[2], 10);
   const endCol = colToIndex(match[3].toUpperCase());
   const endRow = parseInt(match[4], 10);

+  const numCells = (endCol - startCol + 1) * (endRow - startRow + 1);
+  if (numCells > MAX_RANGE_CELLS) return null;
+
   const refs: string[] = [];
   for (let row = startRow; row <= endRow; row++) {
     for (let col = startCol; col <= endCol; col++) {
       refs.push(indexToCol(col) + row);
     }
   }
   return refs;
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/backend/src/api/v1/cells.controller.ts` around lines 157 - 173, The
expandRange function can produce huge arrays and should guard against overly
large ranges: inside expandRange (and using symbols startCol, endCol, startRow,
endRow, and refs) compute the number of columns and rows (e.g., numCols = endCol
- startCol + 1 and numRows = endRow - startRow + 1), check that numCols *
numRows does not exceed a configurable MAX_CELLS (or reasonable hardcoded
limit), and if it does return null or throw a controlled error; also validate
individual numCols and numRows against per-dimension limits and add tests or a
clear error message so callers of expandRange can handle the limit condition
gracefully.

packages/frontend/src/app/workspaces/workspace-settings.tsx-143-147 (1)

143-147: ⚠️ Potential issue | 🟡 Minor

Query may never fetch if isOwner is initially falsy.

The enabled condition depends on isOwner, which is derived from the me and workspace queries. Since isOwner is computed synchronously and may be undefined initially, this query won't re-evaluate its enabled state when me or workspace data arrives. React Query's enabled is evaluated on each render, but the query won't automatically refetch when isOwner becomes truthy after initial render with enabled: false.

Consider ensuring the query re-runs when ownership is determined:

🐛 Suggested fix

   const { data: apiKeys = [] } = useQuery<ApiKey[]>({
     queryKey: ["workspaces", workspaceId, "api-keys"],
     queryFn: () => fetchApiKeys(workspaceId!),
-    enabled: !!workspaceId && !!isOwner,
+    enabled: !!workspaceId && !!me && !!workspace && workspace.members.some((m) => m.user.id === me.id && m.role === "owner"),
   });

Alternatively, include isOwner in the query key to trigger refetch when ownership status changes.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/src/app/workspaces/workspace-settings.tsx` around lines 143
- 147, The API-keys query may not refetch when ownership is resolved; add
isOwner to the queryKey so React Query treats ownership changes as a key change
and refetches. Modify the useQuery call (useQuery, fetchApiKeys, apiKeys,
workspaceId, isOwner) to use a key like ["workspaces", workspaceId, "api-keys",
isOwner] and keep the enabled check (e.g., enabled: !!workspaceId && !!isOwner)
so the query will rerun when isOwner transitions.

docs/design/rest-api-and-cli.md-663-663 (1)

663-663: ⚠️ Potential issue | 🟡 Minor

Fix: Add language specifier to fenced code block.

The fenced code block at line 663 is missing a language specifier, triggering a markdownlint warning. Since the content describes an agent flow (not executable code), specify text or markdown:

📝 Proposed fix

-```
+```text
 1. Agent loads skill/recipe files (bundled with CLI or fetched from repo)
 2. Reads skill frontmatter to understand safety and available tools

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/design/rest-api-and-cli.md` at line 663, The fenced code block that
starts with "Agent loads skill/recipe files (bundled with CLI or fetched from
repo)" is missing a language specifier and triggers a markdownlint warning;
update that fenced block to include a language tag such as text or markdown
(e.g., change ``` to ```text) so the block is explicitly marked as
non-executable content and the linter warning is resolved.

.github/workflows/npm-publish.yml (1)

35-36: Fail fast if the release tag and package version diverge.

Right now a version mismatch is only discovered inside pnpm publish. Adding a small preflight check before Line 35 makes release failures much easier to diagnose.

Suggested workflow guard

       - name: Run CLI tests
         run: pnpm --filter `@wafflebase/cli` test

+      - name: Verify release tag matches package version
+        run: |
+          TAG="${GITHUB_REF_NAME#v}"
+          VERSION=$(node -p "JSON.parse(require('fs').readFileSync('packages/cli/package.json', 'utf8')).version")
+          test "$TAG" = "$VERSION"
+
       - name: Publish to npm
         run: pnpm --filter `@wafflebase/cli` publish --access public --no-git-checks

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/npm-publish.yml around lines 35 - 36, Add a preflight step
before the existing "Publish to npm" step that fails the job if the git release
tag does not match the package version: read the version from the
`@wafflebase/cli` package.json (the package's version field), normalize both
values (strip a leading "v" from the git tag), compare against the current
GitHub tag (use GITHUB_REF or GITHUB_REF_NAME provided by Actions), and exit
non‑zero with a clear message when they differ so the "Publish to npm" step
never runs on a mismatched tag.

packages/cli/src/config/config.ts (2)

52-57: Consider respecting XDG_CONFIG_HOME.

The function hardcodes ~/.config/wafflebase but the XDG Base Directory Specification recommends using XDG_CONFIG_HOME when set.

💡 Optional: Respect XDG_CONFIG_HOME

 function getConfigPath(): string {
   return (
     process.env.WAFFLEBASE_CONFIG ??
-    join(homedir(), '.config', 'wafflebase', 'config.yaml')
+    join(
+      process.env.XDG_CONFIG_HOME ?? join(homedir(), '.config'),
+      'wafflebase',
+      'config.yaml',
+    )
   );
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/config/config.ts` around lines 52 - 57, The getConfigPath
function currently falls back to homedir()/.config/wafflebase/config.yaml and
ignores XDG_CONFIG_HOME; update getConfigPath to prefer
process.env.WAFFLEBASE_CONFIG, then if unset check process.env.XDG_CONFIG_HOME
and use join(XDG_CONFIG_HOME, 'wafflebase', 'config.yaml') when present,
otherwise keep the existing join(homedir(), '.config', 'wafflebase',
'config.yaml') fallback; ensure you reference the WAFFLEBASE_CONFIG and
XDG_CONFIG_HOME env vars and use the existing join and homedir helpers (function
name: getConfigPath) so behavior is unchanged when XDG_CONFIG_HOME is not set.

---

63-69: Consider logging config file errors in verbose mode.

The empty catch block silently swallows all errors, including permission issues or malformed YAML that the user might want to know about. While graceful degradation is good, users may be confused if their config is silently ignored.

💡 Optional: Log errors when verbose mode is enabled

Since loadProfile doesn't have access to verbose flag, you could return a result object or log a warning:

-  } catch {
-    return {};
+  } catch (err) {
+    // Optionally: console.error(`Warning: Failed to load config from ${configPath}`);
+    return {};
   }

Alternatively, consider accepting a verbose parameter to enable debug logging.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/config/config.ts` around lines 63 - 69, The catch block in
loadProfile (the try that calls readFileSync and parseYaml) silently swallows
all errors; update loadProfile to surface or log errors when verbose is enabled
by either (a) adding an optional verbose boolean parameter to loadProfile and,
on catch, call the existing logger (or console.warn) with the error and file
path when verbose is true, or (b) change the function to return a result object
like { profile: ConfigFile | {}, error?: Error } so callers can log the error in
their verbose flow; reference the functions/read calls readFileSync and
parseYaml and ensure permission/malformed-YAML errors are included in the logged
message while still returning {} for non-verbose runs.

packages/cli/src/commands/root.ts (2)

31-33: New HttpClient created on each call.

getClient creates a new HttpClient instance each time it's called. This is fine for the current use case where commands typically make a single API call, but be aware if commands start making multiple calls.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/commands/root.ts` around lines 31 - 33, getClient currently
constructs a new HttpClient on every call (using getConfig), which can be
wasteful when multiple API calls are made; change it to return a cached/reused
HttpClient instance instead: create a module-level variable (e.g., cachedClient)
and initialize it lazily inside getClient (or expose an initClient function) so
subsequent calls return the same HttpClient; reference getClient, HttpClient and
getConfig when updating the implementation and ensure configuration is applied
when creating the singleton.

---

41-41: Consider reading version from package.json.

The version is hardcoded as '0.1.0'. This may drift out of sync with package.json. Commander supports reading the version dynamically.

💡 Optional: Import version from package.json

+import { readFileSync } from 'node:fs';
+import { fileURLToPath } from 'node:url';
+import { dirname, join } from 'node:path';
+
+const __dirname = dirname(fileURLToPath(import.meta.url));
+const pkg = JSON.parse(readFileSync(join(__dirname, '../../package.json'), 'utf-8'));

 export function createProgram(): Command {
   const program = new Command();

   program
     .name('wafflebase')
     .description('CLI for Wafflebase spreadsheet API')
-    .version('0.1.0')
+    .version(pkg.version)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/commands/root.ts` at line 41, The .version('0.1.0') call is
hardcoded and should be driven from package.json; import or require the
package.json version field (e.g., const { version } =
require('.../package.json') or an ES import if resolveJsonModule is enabled) and
replace the hardcoded string in the .version(...) call with that imported
version. Ensure the import path targets the workspace/package package.json and,
if using TypeScript imports, confirm tsconfig has
resolveJsonModule/allowSyntheticDefaultImports enabled or fall back to require.

packages/cli/src/commands/cell.ts (1)

17-21: Multiple getClient() calls create redundant instances.

The ternary chain calls getClient(opts) up to three times, instantiating a new HttpClient each time. Store the client in a variable for clarity and efficiency.

♻️ Proposed refactor

       try {
+        const client = getClient(opts);
         const res = range?.includes(':')
-          ? await getClient(opts).getCells(docId, tab, range)
+          ? await client.getCells(docId, tab, range)
           : range
-            ? await getClient(opts).getCell(docId, tab, range)
-            : await getClient(opts).getCells(docId, tab);
+            ? await client.getCell(docId, tab, range)
+            : await client.getCells(docId, tab);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/commands/cell.ts` around lines 17 - 21, The ternary in
cell.ts repeatedly calls getClient(opts), creating multiple HttpClient
instances; instead, call getClient(opts) once, assign it to a local variable
(e.g., const client = getClient(opts)), and then use client.getCells(...) or
client.getCell(...) in the existing ternary expression that sets res so you
reuse the same client for all branches (references: getClient, opts, getCells,
getCell, res).

packages/cli/src/output/table.ts (1)

7-8: Consider handling objects with different keys.

The function assumes all objects in the array have the same keys as the first object. If objects have different keys, some columns may be missing data or extra properties will be silently ignored. This is acceptable for CLI output where the API returns homogeneous arrays, but worth documenting.

💡 Optional: Extract all unique keys for heterogeneous arrays

   const rows = data as Record<string, unknown>[];
-  const keys = Object.keys(rows[0]);
+  const keys = [...new Set(rows.flatMap((row) => Object.keys(row)))];

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/output/table.ts` around lines 7 - 8, The code in table.ts
currently grabs columns via Object.keys(rows[0]) which assumes every object in
rows has the same shape; update the logic that builds keys (the variable keys
derived from rows and rows[0]) to compute the union of all property names across
every object in rows (e.g., iterate rows and collect unique keys) so
heterogeneous objects won't drop or ignore fields, and ensure downstream
rendering uses that unified keys list; alternatively, if you want to keep the
current behavior, add a brief comment near the const rows and const keys
declarations documenting the assumption that all array objects are homogeneous.

packages/cli/src/client/http-client.ts (2)

95-120: API key methods duplicate URL construction logic.

The listApiKeys, createApiKey, and revokeApiKey methods manually construct URLs and duplicate the server normalization and fetch logic instead of reusing the generic request method. Consider extracting a helper or adjusting request to support different base paths.

♻️ Proposed refactor to reduce duplication

+  private async requestRaw<T>(
+    method: string,
+    url: string,
+    body?: unknown,
+  ): Promise<ApiResponse<T>> {
+    const res = await fetch(url, {
+      method,
+      headers: this.headers,
+      body: body ? JSON.stringify(body) : undefined,
+    });
+    const data = (await res.json().catch(() => null)) as T;
+    return { ok: res.ok, status: res.status, data };
+  }
+
+  private get apiKeyBase(): string {
+    const server = this.config.server.replace(/\/$/, '');
+    return `${server}/workspaces/${this.config.workspace}/api-keys`;
+  }
+
   // API Keys (management endpoints use different base)
-  async listApiKeys() {
-    const server = this.config.server.replace(/\/$/, '');
-    const url = `${server}/workspaces/${this.config.workspace}/api-keys`;
-    const res = await fetch(url, { headers: this.headers });
-    const data = await res.json().catch(() => null);
-    return { ok: res.ok, status: res.status, data };
+  listApiKeys() {
+    return this.requestRaw('GET', this.apiKeyBase);
   }
-  async createApiKey(name: string) {
-    const server = this.config.server.replace(/\/$/, '');
-    const url = `${server}/workspaces/${this.config.workspace}/api-keys`;
-    const res = await fetch(url, {
-      method: 'POST',
-      headers: this.headers,
-      body: JSON.stringify({ name }),
-    });
-    const data = await res.json().catch(() => null);
-    return { ok: res.ok, status: res.status, data };
+  createApiKey(name: string) {
+    return this.requestRaw('POST', this.apiKeyBase, { name });
   }
-  async revokeApiKey(id: string) {
-    const server = this.config.server.replace(/\/$/, '');
-    const url = `${server}/workspaces/${this.config.workspace}/api-keys/${id}`;
-    const res = await fetch(url, { method: 'DELETE', headers: this.headers });
-    const data = await res.json().catch(() => null);
-    return { ok: res.ok, status: res.status, data };
+  revokeApiKey(id: string) {
+    return this.requestRaw('DELETE', `${this.apiKeyBase}/${id}`);
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/client/http-client.ts` around lines 95 - 120, The three
methods listApiKeys, createApiKey, and revokeApiKey duplicate server
normalization and fetch logic; refactor them to call the existing generic
request helper (or add a small private helper that builds the base URL once)
instead of manually constructing URLs and using fetch in each method. Replace
the manual server.replace(/\/$/, '') +
`${server}/workspaces/${this.config.workspace}/api-keys...` code in listApiKeys,
createApiKey, and revokeApiKey with calls like this.request({ path:
`/workspaces/${this.config.workspace}/api-keys` , method: 'GET'|'POST'|'DELETE',
body: { name } (for create), params or path param for revoke id }) so headers,
JSON parsing, and error handling are centralized.

---

78-83: setCell sends undefined fields in request body.

Both value and formula are included in the request body even when one is undefined. While many servers ignore undefined fields in JSON, it's cleaner to omit them.

💡 Optional: Build body conditionally

   setCell(docId: string, tabId: string, sref: string, value?: string, formula?: string) {
-    return this.request('PUT', `/documents/${docId}/tabs/${tabId}/cells/${sref}`, {
-      value,
-      formula,
-    });
+    const body: { value?: string; formula?: string } = {};
+    if (value !== undefined) body.value = value;
+    if (formula !== undefined) body.formula = formula;
+    return this.request('PUT', `/documents/${docId}/tabs/${tabId}/cells/${sref}`, body);
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/client/http-client.ts` around lines 78 - 83, The setCell
method is sending value and formula even when they are undefined; update setCell
in http-client.ts to build the request body conditionally so only defined fields
are included (e.g., create a body object and add value and/or formula only if
!== undefined) before calling this.request('PUT',
`/documents/${docId}/tabs/${tabId}/cells/${sref}`, body); this keeps the request
JSON clean and avoids sending undefined fields.

packages/cli/test/config.test.ts (1)

4-16: Minor: Environment restoration approach could be fragile.

The process.env = { ...origEnv } assignment replaces the entire process.env object reference. While this works in Node.js (it reassigns the property on the process object), it's safer to restore individual keys or use a helper library. This is acceptable for these isolated tests.

♻️ Alternative approach (optional)

   afterEach(() => {
-    process.env = { ...origEnv };
+    // Clear any test-added keys and restore originals
+    for (const key of Object.keys(process.env)) {
+      if (!(key in origEnv)) delete process.env[key];
+    }
+    Object.assign(process.env, origEnv);
   });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/cli/test/config.test.ts` around lines 4 - 16, The test currently
restores environment by reassigning process.env = { ...origEnv } in the
afterEach block which replaces the env object reference; instead update the
afterEach to restore only the mutated keys (use origEnv to set back
WAFFLEBASE_SERVER, WAFFLEBASE_API_KEY, WAFFLEBASE_WORKSPACE, WAFFLEBASE_CONFIG
and delete any keys that weren't present originally) so you avoid replacing the
process.env reference—modify the afterEach that references origEnv and the
beforeEach that deletes the four WAFFLEBASE_* keys accordingly.

packages/backend/prisma/migrations/20260312025839_add_api_key/migration.sql (1)

18-25: Add an index for workspace-scoped key lookups.

ApiKey management is workspace-scoped, but workspaceId is only a foreign key here. PostgreSQL won't index that automatically, so list/revoke queries will degrade into table scans as this table grows.

Suggested migration addition

 -- CreateIndex
 CREATE UNIQUE INDEX "ApiKey_hashedKey_key" ON "ApiKey"("hashedKey");
+CREATE INDEX "ApiKey_workspaceId_idx" ON "ApiKey"("workspaceId");

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/backend/prisma/migrations/20260312025839_add_api_key/migration.sql`
around lines 18 - 25, Add a non-unique index on the ApiKey.workspaceId column so
workspace-scoped list/revoke queries are indexed; in the migration that
currently creates "ApiKey_hashedKey_key" and the foreign keys, add a CREATE
INDEX for "ApiKey"("workspaceId") (or a composite index on
("workspaceId","hashedKey") if you expect lookups by both) to avoid table scans
on workspace-scoped queries.

packages/backend/prisma/schema.prisma (1)

94-108: Add a composite index for workspace-scoped key listing.

The new table bakes in workspace scoping plus soft revocation, so the common workspaceId + revokedAt filter will otherwise degrade into full scans as API keys accumulate. Declaring the index in the Prisma model also keeps future migrations aligned with the intended access path.

Suggested schema addition

 model ApiKey {
   id          String    `@id` `@default`(uuid())
   name        String
   prefix      String
   hashedKey   String    `@unique`
   workspaceId String
   workspace   Workspace `@relation`(fields: [workspaceId], references: [id], onDelete: Cascade)
   createdBy   Int
   creator     User      `@relation`(fields: [createdBy], references: [id])
   scopes      String[]  `@default`(["read", "write"])
   lastUsedAt  DateTime?
   expiresAt   DateTime?
   revokedAt   DateTime?
   createdAt   DateTime  `@default`(now())
+
+  @@index([workspaceId, revokedAt])
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/backend/prisma/schema.prisma` around lines 94 - 108, Add a composite
index to the ApiKey model to optimize queries filtering by workspaceId and
revokedAt: update the ApiKey model (symbol: ApiKey) to declare an index on the
fields workspaceId and revokedAt (fields: workspaceId, revokedAt) using Prisma's
@@index directive, then run prisma migrate (or generate) to create the migration
so the database and Prisma schema stay in sync.

packages/backend/src/api/v1/workspace-scope.guard.ts (1)

18-37: Add null check for user to improve guard robustness.

If a preceding auth guard fails to populate request.user, accessing user.isApiKey or user.id will throw. While guard ordering should prevent this, adding a defensive check makes the code more resilient.

🛡️ Proposed fix

   async canActivate(context: ExecutionContext): Promise<boolean> {
     const request = context.switchToHttp().getRequest();
     const user = request.user;
+    if (!user) {
+      throw new ForbiddenException('User not authenticated');
+    }
     const workspaceId = request.params.workspaceId;

     const resolvedId = await this.workspaceService.resolveId(workspaceId);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/backend/src/api/v1/workspace-scope.guard.ts` around lines 18 - 37,
In canActivate (workspace-scope.guard.ts) add a defensive null check for
request.user at the start of the method: if user is missing, throw an
UnauthorizedException (or return false) before accessing user.isApiKey or
user.id; keep the rest of the logic (resolveId via
this.workspaceService.resolveId, API key workspaceId comparison, and
this.workspaceService.assertMember) unchanged so subsequent lines can safely
assume user is defined.

packages/cli/src/commands/document.ts (1)

16-16: Consider including response body in error messages for better debugging.

throw new Error(HTTP ${res.status}) discards the response body which may contain useful error details from the server.

♻️ Example enhancement

-        if (!res.ok) throw new Error(`HTTP ${res.status}`);
+        if (!res.ok) throw new Error(`HTTP ${res.status}: ${res.data?.message ?? 'Unknown error'}`);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/commands/document.ts` at line 16, The current error throw
discards the response body; update the error path around the `res` check to read
the response body (e.g., await res.text() or attempt JSON parse) and include
that body in the thrown Error message (for example include `HTTP ${res.status}:
<body>`), ensuring you await the body read before throwing so debugging details
from the server aren’t lost.

packages/cli/src/commands/schema.ts (1)

15-25: Inconsistent error handling pattern.

This error handling differs from other commands (e.g., document.ts) which use the outputError helper. Using outputError here would ensure consistent error formatting across all CLI commands.

♻️ Proposed refactor

+import { output, outputError } from '../output/formatter.js';
-import { output } from '../output/formatter.js';
...
         if (!schema) {
-          console.error(
-            JSON.stringify(
-              { error: { code: 'NOT_FOUND', message: `Unknown command: ${commandName}` } },
-              null,
-              2,
-            ),
-          );
-          process.exitCode = 1;
+          outputError(new Error(`Unknown command: ${commandName}`), opts.quiet);
           return;
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/commands/schema.ts` around lines 15 - 25, Replace the ad-hoc
console.error JSON block that runs when schema is falsy with the shared CLI
error helper: call outputError({ code: 'NOT_FOUND', message: `Unknown command:
${commandName}` }) instead of console.error(...), then set process.exitCode = 1
and return as before; update the branch in the function that checks schema (the
same block referencing schema and commandName) to use outputError to match the
pattern used in document.ts and other commands.

packages/cli/src/schema/registry.ts (1)

152-154: Return a defensive copy to prevent accidental registry mutation.

getAllCommandSchemas() returns the internal registry array directly. External callers could inadvertently mutate it.

♻️ Proposed fix

 export function getAllCommandSchemas(): CommandSchema[] {
-  return registry;
+  return [...registry];
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/schema/registry.ts` around lines 152 - 154,
getAllCommandSchemas currently returns the internal registry array directly
which allows external callers to mutate it; change the return to a defensive
copy (e.g., return registry.slice() or return [...registry]) in
getAllCommandSchemas so callers receive a new array instance and cannot modify
the internal registry; if individual CommandSchema objects must also be
protected, map and shallow-clone each element (e.g., registry.map(s => ({ ...s
}))).

packages/backend/src/api-key/api-key.controller.ts (1)

28-39: Consider using a DTO class with validation decorators.

The inline body type lacks runtime validation. Using a DTO with class-validator decorators ensures proper input validation and clear API contracts.

♻️ Example DTO

// create-api-key.dto.ts
import { IsString, IsOptional, IsArray, IsDateString, MinLength } from 'class-validator';

export class CreateApiKeyDto {
  `@IsString`()
  `@MinLength`(1)
  name: string;

  `@IsOptional`()
  `@IsArray`()
  `@IsString`({ each: true })
  scopes?: string[];

  `@IsOptional`()
  `@IsDateString`()
  expiresAt?: string;
}

Then update the controller:

-    `@Body`() body: { name: string; scopes?: string[]; expiresAt?: string },
+    `@Body`() body: CreateApiKeyDto,

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/backend/src/api-key/api-key.controller.ts` around lines 28 - 39,
Replace the inline body type with a validated DTO: create a CreateApiKeyDto
class (with decorators like `@IsString`, `@MinLength` for name,
`@IsOptional/`@IsArray/@IsString({ each: true }) for scopes, and
`@IsOptional/`@IsDateString for expiresAt), use it as the type for the controller
parameter (replace the inline "{ name: string; scopes?: string[]; expiresAt?:
string }" in the controller method), and enable Nest's validation pipe so the
controller validates input before calling workspaceService.resolveId,
workspaceService.assertOwner, and apiKeyService.create; ensure expiresAt is
still converted to Date when present (new Date(dto.expiresAt)) and keep existing
calls to resolveId/assertOwner/apiKeyService.create.

packages/backend/src/api/v1/documents.controller.ts (1)

56-71: Consider including workspaceId in the update/delete where clauses for defense-in-depth.

While the existence check at lines 62-66 validates the document belongs to the workspace, the actual updateDocument call at line 67-70 only uses id. In a race condition scenario (TOCTOU), the document could theoretically be moved to another workspace between the check and update. Including workspaceId in the operation itself provides an additional safety layer.

♻️ Suggested improvement

     return this.documentService.updateDocument({
-      where: { id: documentId },
+      where: { id: documentId, workspaceId },
       data: body,
     });

Similarly for deleteDocument:

-    return this.documentService.deleteDocument({ id: documentId });
+    return this.documentService.deleteDocument({ id: documentId, workspaceId });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/backend/src/api/v1/documents.controller.ts` around lines 56 - 71,
The update handler currently verifies document existence via
this.documentService.document(...) but then calls
this.documentService.updateDocument({ where: { id: documentId }, ... }) which
omits workspaceId and leaves a TOCTOU gap; modify the update call to include
workspaceId in the where clause (e.g., where: { id: documentId, workspaceId })
and do the same defensive change for the corresponding delete operation
(this.documentService.deleteDocument) so both updateDocument and deleteDocument
use { id: documentId, workspaceId } in their where clauses.

packages/frontend/src/app/workspaces/workspace-settings.tsx (1)

224-228: Handle potential clipboard API failures.

navigator.clipboard.writeText() returns a Promise that can reject (e.g., if clipboard permissions are denied). Unhandled rejections could cause silent failures. Consider adding error handling.

♻️ Suggested improvement for clipboard operations

   const copyInviteLink = (token: string) => {
     const link = `${window.location.origin}/invite/${token}`;
-    navigator.clipboard.writeText(link);
-    toast.success("Invite link copied");
+    navigator.clipboard.writeText(link)
+      .then(() => toast.success("Invite link copied"))
+      .catch(() => toast.error("Failed to copy link"));
   };

Similarly for the API key copy button:

             onClick={() => {
               if (revealedKey) {
-                navigator.clipboard.writeText(revealedKey.key);
-                toast.success("API key copied");
+                navigator.clipboard.writeText(revealedKey.key)
+                  .then(() => toast.success("API key copied"))
+                  .catch(() => toast.error("Failed to copy key"));
               }
             }}

Also applies to: 646-650

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/src/app/workspaces/workspace-settings.tsx` around lines 224
- 228, The copyInviteLink function currently calls
navigator.clipboard.writeText(link) without handling rejections; update
copyInviteLink to check for navigator.clipboard availability and handle the
Promise rejection from writeText (use async/await or .then/.catch) and show a
failure toast (e.g., toast.error("Failed to copy invite link")) on error; apply
the same pattern to the API key copy handler (the API key copy function around
lines 646-650) so both copy operations gracefully handle permission or runtime
failures and provide user feedback.

packages/backend/src/api-key/api-key.service.ts (1)

84-90: Consider logging failed lastUsedAt updates instead of silently swallowing errors.

The fire-and-forget pattern is appropriate for non-critical updates, but silently catching errors could hide database connectivity issues or other problems that might be useful for operational monitoring.

♻️ Suggested improvement

+import { Injectable, UnauthorizedException, Logger } from '@nestjs/common';
+
+@Injectable()
+export class ApiKeyService {
+  private readonly logger = new Logger(ApiKeyService.name);
   // ... existing code ...
 
     // Update lastUsedAt fire-and-forget
     this.prisma.apiKey
       .update({
         where: { id: apiKey.id },
         data: { lastUsedAt: new Date() },
       })
-      .catch(() => {});
+      .catch((err) => this.logger.warn(`Failed to update lastUsedAt for key ${apiKey.id}`, err));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/backend/src/api-key/api-key.service.ts` around lines 84 - 90, The
fire-and-forget prisma.apiKey.update call that sets lastUsedAt currently
swallows errors (.catch(() => {})); change this to log failures instead so
operational issues are visible: replace the empty catch with .catch(err => /*
log */) and emit a structured message including the apiKey.id and err (e.g.,
this.logger.error or console.error) so you capture the failure context for
prisma.apiKey.update and lastUsedAt updates.

packages/backend/src/yorkie/yorkie.service.ts (1)

23-31: Consider adding error handling for client activation/deactivation failures.

If client.activate() fails, the service will be in an inconsistent state where subsequent withDocument calls will fail. Consider logging errors or implementing retry logic for resilience.

♻️ Suggested improvement

   async onModuleInit() {
-    await this.client.activate();
-    this.logger.log('Yorkie client activated');
+    try {
+      await this.client.activate();
+      this.logger.log('Yorkie client activated');
+    } catch (error) {
+      this.logger.error('Failed to activate Yorkie client', error);
+      throw error;
+    }
   }

   async onModuleDestroy() {
-    await this.client.deactivate();
-    this.logger.log('Yorkie client deactivated');
+    try {
+      await this.client.deactivate();
+      this.logger.log('Yorkie client deactivated');
+    } catch (error) {
+      this.logger.error('Failed to deactivate Yorkie client', error);
+    }
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/backend/src/yorkie/yorkie.service.ts` around lines 23 - 31, Wrap
client.activate() in onModuleInit and client.deactivate() in onModuleDestroy
with try/catch blocks to handle failures: catch and log the error via
this.logger.error including error details and a clear context string, set a
boolean flag on the service (e.g., this.clientReady) to reflect success/failure
so methods like withDocument can check and fail fast, and optionally add simple
retry logic with limited attempts/backoff for activate() (use a small loop with
delay) to improve resilience; ensure any caught errors do not leave the process
in an unhandled rejection state.