Address PR #197 review: test coverage + dedup + CI log capture

Code-review follow-ups (Approve-with-comments) for batch #197
(context badge #189 / e2e in CI #187 / inline MCP test #170):

- server: extract the duplicated chatContextWindow ::text->positive-int
  coercion (resolve() + getMasked()) into an exported parsePositiveInt
  helper and unit-test its branches (200000/1.9/0/-5/""/abc/undefined),
  closing the untested read-path gap.
- client: merge the two backward scans over messageRows into one pure,
  exported selectContextBadge helper (numerator and denominator still
  taken from the most recent row carrying EACH value) and unit-test the
  different-rows and fresh-zero-doesn't-shadow cases.
- client: extract the MCP "Test" button tristate presentation into a pure
  mcpTestButtonView helper (collapses the two parallel if/else chains) and
  unit-test idle/ok-with-tools/ok-no-tools/failed label+tooltip branches.
- ci: redirect the backgrounded prod server's stdout/stderr to a log file
  in e2e-mcp and cat it on failure, so a start-up crash is diagnosable
  instead of surfacing only as the generic health timeout.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
claude code agent 227
2026-06-26 17:24:29 +03:00
parent 9b61024b95
commit ba5cd02439
9 changed files with 357 additions and 85 deletions

View File

@@ -0,0 +1,43 @@
import { parsePositiveInt } from './ai-settings.service';
/**
* Round-trip coercion for numeric `::text` provider settings (e.g.
* chatContextWindow). Values are stored as text and read back as strings, so
* this guards the read path the DTO write-validation does not cover: a silent
* loss of `Math.floor` or a `> 0` → `>= 0` drift would otherwise go unnoticed.
*/
describe('parsePositiveInt', () => {
it('keeps a valid positive integer string', () => {
expect(parsePositiveInt('200000')).toBe(200000);
});
it('floors a fractional string', () => {
expect(parsePositiveInt('1.9')).toBe(1);
expect(parsePositiveInt('1.0')).toBe(1);
});
it('returns undefined for zero', () => {
expect(parsePositiveInt('0')).toBeUndefined();
});
it('returns undefined for a negative value', () => {
expect(parsePositiveInt('-5')).toBeUndefined();
});
it('returns undefined for an empty string', () => {
expect(parsePositiveInt('')).toBeUndefined();
});
it('returns undefined for a non-numeric string', () => {
expect(parsePositiveInt('abc')).toBeUndefined();
});
it('returns undefined for undefined / null', () => {
expect(parsePositiveInt(undefined)).toBeUndefined();
expect(parsePositiveInt(null)).toBeUndefined();
});
it('accepts a real number too (not only ::text strings)', () => {
expect(parsePositiveInt(42)).toBe(42);
});
});

View File

@@ -18,6 +18,18 @@ import {
PROVIDER_SETTINGS_KEYS,
} from './ai.types';
/**
* Coerce a raw provider value (stored as `::text`, so it arrives as a string —
* see workspace.repo.ts) into a positive integer, or `undefined` when it is not
* a finite number greater than zero. Used for numeric `::text` settings such as
* `chatContextWindow`. Fractions are floored: `"1.9" → 1`, `"0"`/`"-5"`/`""`/
* `"abc"`/`undefined` → `undefined`.
*/
export function parsePositiveInt(raw: unknown): number | undefined {
const n = Number(raw);
return Number.isFinite(n) && n > 0 ? Math.floor(n) : undefined;
}
/**
* Shape of the partial update accepted by `update`. Mirrors the validated
* controller DTO. `apiKey` / `embeddingApiKey` are write-only: undefined =
@@ -159,20 +171,12 @@ export class AiSettingsService {
const provider = await this.readProvider(workspaceId);
if (!provider.driver) return null;
// Provider values are stored as ::text (see workspace.repo.ts), so
// chatContextWindow arrives as a string here; parse it back to a positive
// integer or undefined.
const ctxWindow = Number(provider.chatContextWindow);
const config: ResolvedAiConfig = {
driver: provider.driver,
chatModel: provider.chatModel,
// Max context window for the chat header badge denominator. 0/unset = no
// limit.
chatContextWindow:
Number.isFinite(ctxWindow) && ctxWindow > 0
? Math.floor(ctxWindow)
: undefined,
// Max context window for the chat header badge denominator. Stored as
// ::text; 0/unset/invalid = no limit (undefined).
chatContextWindow: parsePositiveInt(provider.chatContextWindow),
// Plain passthrough; getChatModel defaults unset to 'openai-compatible'.
chatApiStyle: provider.chatApiStyle,
// Cheap model id for the anonymous public-share assistant; reuses the chat
@@ -232,14 +236,9 @@ export class AiSettingsService {
async getMasked(workspaceId: string): Promise<MaskedAiSettings> {
const provider = await this.readProvider(workspaceId);
// Provider values are stored as ::text (see workspace.repo.ts), so
// chatContextWindow arrives as a string; coerce it to a positive integer or
// undefined so the client receives a real number.
const ctxWindow = Number(provider.chatContextWindow);
const chatContextWindow =
Number.isFinite(ctxWindow) && ctxWindow > 0
? Math.floor(ctxWindow)
: undefined;
// Stored as ::text; coerce to a positive integer (or undefined) so the
// client receives a real number.
const chatContextWindow = parsePositiveInt(provider.chatContextWindow);
let hasApiKey = false;
let hasEmbeddingApiKey = false;