Findings

• [Major] Unknown-extension text files can bypass size enforcement and get silently truncated — this violates the no-silent-fallback constraint and can hide prompt-context loss from users, evidence apps/desktop/src/main/prompt-context.ts:67 and apps/desktop/src/main/prompt-context.ts:99
  Suggested fix:

  const looksText = isProbablyText(probe, extension);
  if (looksText && file.size > MAX_TEXT_ATTACHMENT_BYTES) {
    throw new CodesignError(
      `Text attachment "${file.name}" is too large (${file.size} bytes). Maximum is ${MAX_TEXT_ATTACHMENT_BYTES} bytes.`,
      ERROR_CODES.ATTACHMENT_TOO_LARGE,
    );
  }
  
  if (!looksText) {
    buffer = probe;
  } else {
    const length = Math.max(1, Math.min(file.size || MAX_TEXT_ATTACHMENT_BYTES, MAX_TEXT_ATTACHMENT_BYTES));
    const fullBuffer = Buffer.alloc(length);
    const { bytesRead } = await handle.read(fullBuffer, 0, fullBuffer.length, 0);
    buffer = fullBuffer.subarray(0, bytesRead);
  }

Summary
Review mode: initial
One issue found in the latest diff: unknown text-like attachments with non-text extensions are accepted up to 10MB and then truncated to 256KB without raising an error (apps/desktop/src/main/prompt-context.ts:99). docs/VISION.md and docs/PRINCIPLES.md were not available in this checkout (Not found in repo/docs), so assessment used repo-available constraints (CLAUDE.md).

Testing
Not run (automation). Suggested coverage: add Vitest cases for readAttachment that verify (1) unknown-extension text file >256KB throws ATTACHMENT_TOO_LARGE, and (2) unknown-extension binary file <=10MB returns binary note without content.

open-codesign Bot

Findings

• [Major] Unknown-extension text attachments can still bypass the text-size error via silent truncation — this can hide prompt-context loss and violates the no-silent-fallback constraint, evidence apps/desktop/src/main/prompt-context.ts:103
  Suggested fix:

const looksText = isProbablyText(probe, extension);
if (looksText && file.size > MAX_TEXT_ATTACHMENT_BYTES) {
  throw new CodesignError(
    `Text attachment "${file.name}" is too large (${file.size} bytes). Maximum is ${MAX_TEXT_ATTACHMENT_BYTES} bytes.`,
    ERROR_CODES.ATTACHMENT_TOO_LARGE,
  );
}

if (!looksText) {
  buffer = probe;
} else {
  const length = Math.max(1, Math.min(file.size || MAX_TEXT_ATTACHMENT_BYTES, MAX_TEXT_ATTACHMENT_BYTES));
  const fullBuffer = Buffer.alloc(length);
  const { bytesRead } = await handle.read(fullBuffer, 0, fullBuffer.length, 0);
  buffer = fullBuffer.subarray(0, bytesRead);
}

Summary
Review mode: follow-up after new commits
One issue remains in the latest PR diff: the else branch still truncates unknown-extension files that look like text instead of surfacing ATTACHMENT_TOO_LARGE (apps/desktop/src/main/prompt-context.ts:103). docs/VISION.md and docs/PRINCIPLES.md are not present in this checkout (Not found in repo/docs).

Testing
Not run (automation). Suggested coverage: add a Vitest case where an unknown-extension text file over MAX_TEXT_ATTACHMENT_BYTES throws ATTACHMENT_TOO_LARGE.

open-codesign Bot

Findings

• [Major] Unknown-extension text attachments are silently truncated instead of failing fast — this hides prompt-context loss and violates the no-silent-fallback constraint, evidence apps/desktop/src/main/prompt-context.ts:103
  Suggested fix:

const looksText = isProbablyText(probe, extension);
if (looksText && file.size > MAX_TEXT_ATTACHMENT_BYTES) {
  throw new CodesignError(
    `Text attachment "${file.name}" is too large (${file.size} bytes). Maximum is ${MAX_TEXT_ATTACHMENT_BYTES} bytes.`,
    ERROR_CODES.ATTACHMENT_TOO_LARGE,
  );
}

buffer = looksText
  ? (await handle.read(Buffer.alloc(Math.max(1, Math.min(file.size || MAX_TEXT_ATTACHMENT_BYTES, MAX_TEXT_ATTACHMENT_BYTES))), 0, Math.max(1, Math.min(file.size || MAX_TEXT_ATTACHMENT_BYTES, MAX_TEXT_ATTACHMENT_BYTES)), 0)).buffer
  : probe;

Summary
Review mode: follow-up after new commits
One issue remains in the latest diff: the new else branch for files that look like text but exceed MAX_TEXT_ATTACHMENT_BYTES truncates content instead of throwing ATTACHMENT_TOO_LARGE (apps/desktop/src/main/prompt-context.ts:103).

Testing
Not run (automation). Suggested tests: add a Vitest case where an unknown-extension text file over MAX_TEXT_ATTACHMENT_BYTES throws ATTACHMENT_TOO_LARGE, and a case where a binary file under MAX_BINARY_ATTACHMENT_BYTES returns a filename-only note.

open-codesign Bot

Findings

• [Major] ATTACHMENT_TOO_LARGE is remapped to ATTACHMENT_READ_FAILED for text-like oversized files — this hides the real validation failure and can break UI handling that depends on the correct error code, evidence apps/desktop/src/main/prompt-context.ts:91 and apps/desktop/src/main/prompt-context.ts:113.
  Suggested fix:

} catch (error) {
  if (error instanceof CodesignError) {
    throw error;
  }
  throw new CodesignError(
    `Failed to read attachment "${file.path}"`,
    ERROR_CODES.ATTACHMENT_READ_FAILED,
    {
      cause: error,
    },
  );
}

Summary
Review mode: follow-up after new commits

One major issue found in the newly added oversized-text guard: the thrown CodesignError is currently caught and wrapped as ATTACHMENT_READ_FAILED.

open-codesign Bot

Testing
Not run (automation). Missing/updated Vitest coverage for the new probe path is not included in this diff (especially oversized unknown-extension text should assert ATTACHMENT_TOO_LARGE).

Findings

• [Major] ATTACHMENT_TOO_LARGE is still remapped to ATTACHMENT_READ_FAILED in the new text-probe guard path, which hides validation semantics and can break UI error handling keyed on error code, evidence apps/desktop/src/main/prompt-context.ts:91 and apps/desktop/src/main/prompt-context.ts:113.
  Suggested fix:

} catch (error) {
  if (error instanceof CodesignError) {
    throw error;
  }
  throw new CodesignError(
    `Failed to read attachment "${file.path}"`,
    ERROR_CODES.ATTACHMENT_READ_FAILED,
    {
      cause: error,
    },
  );
}

Summary

• Review mode: follow-up after new commits
• 1 major issue remains in the latest diff; no additional high-confidence issues found on added/modified lines.
• Context docs check: docs/VISION.md and docs/PRINCIPLES.md were not available in this checkout (Not found in repo/docs).

Testing

• Not run (automation)

open-codesign Bot

Findings

• None.

Summary

• Review mode: follow-up after new commits
• No new high-confidence issues found on added/modified lines in the latest diff.
• Follow-up check: prior CodesignError remap issue is resolved in apps/desktop/src/main/prompt-context.ts via explicit rethrow.
• Residual risk/testing gap: behavior changes for text-vs-binary size limits and probe-based classification do not include accompanying Vitest coverage in this PR.
• Context docs: docs/VISION.md and docs/PRINCIPLES.md are not available in this checkout (docs/ is gitignored), so internal-doc conformance could not be re-validated here (Not found in repo/docs).

Testing

• Not run (automation)

open-codesign Bot

Findings

• [Major] Unknown-extension size-limit test is invalid because the fixture file does not exist, so the assertion cannot verify ATTACHMENT_TOO_LARGE and instead triggers read-failure behavior; this can produce false confidence or CI failure depending on matcher behavior. Evidence apps/desktop/src/main/prompt-context.test.ts:48, apps/desktop/src/main/prompt-context.test.ts:52
  Suggested fix:

  it('throws ATTACHMENT_TOO_LARGE for unknown-extension text > 256KB', async () => {
    const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'prompt-context-'));
    const filePath = path.join(dir, 'data.bin');
    await fs.writeFile(filePath, 'a'.repeat(300_000), 'utf8');
  
    await expect(
      preparePromptContext({
        attachments: [{ path: filePath, name: 'data.bin', size: 300_000 }],
      }),
    ).rejects.toMatchObject({
      name: 'CodesignError',
      code: 'ATTACHMENT_TOO_LARGE',
    });
  });

Summary

• Review mode: follow-up after new commits
• 1 issue found on added/modified lines in the latest diff.
• Constraint checks in changed lines: no direct provider SDK import, no silent fallback introduced, no new dependency/license change detected.
• Context docs docs/VISION.md and docs/PRINCIPLES.md: Not found in repo/docs.

Testing

• Not run (automation)

open-codesign Bot

Findings

• [Major] Unknown-extension oversize test never reaches the text-size branch because it uses a non-existent fixture path, so it validates read-failure behavior instead of ATTACHMENT_TOO_LARGE. This can hide regressions in the probe-based size enforcement logic. Evidence apps/desktop/src/main/prompt-context.test.ts:48
  Suggested fix:

  it('throws ATTACHMENT_TOO_LARGE for unknown-extension text > 256KB', async () => {
    const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'prompt-context-'));
    const filePath = path.join(dir, 'data.bin');
    await fs.writeFile(filePath, 'a'.repeat(300_000), 'utf8');
  
    await expect(
      preparePromptContext({
        attachments: [{ path: filePath, name: 'data.bin', size: 300_000 }],
      }),
    ).rejects.toMatchObject({
      name: 'CodesignError',
      code: 'ATTACHMENT_TOO_LARGE',
    });
  });

Summary

• Review mode: follow-up after new commits
• 1 issue found on added/modified lines in the latest diff.
• Constraint checks in changed lines: no direct provider SDK import, no silent fallback introduced, no new dependency/license change detected.
• Required docs docs/VISION.md and docs/PRINCIPLES.md: Not found in repo/docs.

Testing

• Not run (automation)

open-codesign Bot

Findings

• None. No high-confidence issues found on added/modified lines in the latest diff.

Summary

• Review mode: follow-up after new commits
• No blocking/major/minor/nit issues identified in changed lines.
• Constraint checks in changed lines: no direct provider SDK imports, no new silent fallback behavior, no dependency/license/install-size deltas in this diff.
• Required docs docs/VISION.md and docs/PRINCIPLES.md: Not found in repo/docs.

Testing

• Not run (automation).
• Suggested additional coverage: add one test for binary attachment >10MB returning ATTACHMENT_TOO_LARGE, and one test with an existing small binary file asserting it is treated as binary context (filename-only note path).

open-codesign Bot