Skip to content

Commit 6e81b79

Browse files
simongdaviessimongdavies
committed
fix: system prompt, edit_handler validation, table contrast, bare exports
- System message: mandatory handler pattern box, edit_handler guidance - edit_handler: validate edited code before applying (closes security bypass) - Validator: handle bare 'export { name }' in .d.ts (fixes getThemeNames) - PPTX tables: always autoTextColor for row text (no overrides) - PDF tables: auto-contrast body text per row, auto-fix poor contrast - Both: dark theme rows get explicit fill for readability on image backgrounds
1 parent da63c56 commit 6e81b79

File tree

9 files changed

+320
-55
lines changed

9 files changed

+320
-55
lines changed

‎builtin-modules/pdf.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
"description": "PDF 1.7 document generation — text, graphics, metadata, standard fonts. Flow-based layout for auto-paginating documents.",
44
"author": "system",
55
"mutable": false,
6-
"sourceHash": "sha256:202d5c76da3d341a",
7-
"dtsHash": "sha256:38f8a8a62174a4f7",
6+
"sourceHash": "sha256:c8716bcb3295f5bc",
7+
"dtsHash": "sha256:f30fba88bfe5f977",
88
"importStyle": "named",
99
"hints": {
1010
"overview": "Generate PDF documents with text, shapes, and metadata. Uses PDF's 14 standard fonts (no embedding required). Coordinates are in points (72 points = 1 inch), with top-left origin.",

‎builtin-modules/pptx-tables.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"description": "Styled tables for PPTX presentations - headers, borders, alternating rows",
44
"author": "system",
55
"mutable": false,
6-
"sourceHash": "sha256:2fd1b0318ee87ab2",
6+
"sourceHash": "sha256:e03a2365c45ab0e6",
77
"dtsHash": "sha256:130d021921083af6",
88
"importStyle": "named",
99
"hints": {

‎builtin-modules/src/pdf.ts‎

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232

3333
import {
3434
autoTextColor,
35+
contrastRatio,
3536
getTheme,
3637
hexColor,
3738
requireArray,
@@ -3004,7 +3005,7 @@ export interface TableStyle {
30043005
headerFg: string;
30053006
/** Header font. */
30063007
headerFont: string;
3007-
/** Body text colour (6-char hex). */
3008+
/** Body text colour (6-char hex). Auto-contrasted if omitted or poor contrast. */
30083009
bodyFg: string;
30093010
/** Body font. */
30103011
bodyFont: string;
@@ -3014,6 +3015,8 @@ export interface TableStyle {
30143015
borderColor: string;
30153016
/** Border line width in points. */
30163017
borderWidth: number;
3018+
/** Page background colour (set internally for contrast checking). */
3019+
_pageBg?: string;
30173020
}
30183021

30193022
/** Built-in table styles matching PPTX table styles. */
@@ -3484,6 +3487,43 @@ function renderTable(
34843487
const rowH = tableRowHeight(fontSize, compact);
34853488
const headerH = rowH;
34863489

3490+
// Ensure page background is available for contrast checking
3491+
if (!style._pageBg) {
3492+
style._pageBg = doc.theme.bg;
3493+
}
3494+
3495+
// ── Contrast auto-correction ─────────────────────────────────────
3496+
// Automatically fix text colors that have poor contrast against the
3497+
// page background. No errors — just silently correct to readable.
3498+
const MIN_CONTRAST = 3.0;
3499+
const pageBg = style._pageBg || "FFFFFF";
3500+
3501+
if (style.bodyFg) {
3502+
const bodyRatio = contrastRatio(style.bodyFg, pageBg);
3503+
if (bodyRatio < MIN_CONTRAST) {
3504+
style.bodyFg = autoTextColor(pageBg);
3505+
}
3506+
}
3507+
3508+
if (style.headerFg && style.headerBg) {
3509+
const headerRatio = contrastRatio(style.headerFg, style.headerBg);
3510+
if (headerRatio < MIN_CONTRAST) {
3511+
style.headerFg = autoTextColor(style.headerBg);
3512+
}
3513+
}
3514+
3515+
if (style.headerBg && style.headerFg) {
3516+
const headerRatio = contrastRatio(style.headerFg, style.headerBg);
3517+
if (headerRatio < MIN_CONTRAST) {
3518+
const suggested = autoTextColor(style.headerBg);
3519+
throw new Error(
3520+
`table: headerFg "${style.headerFg}" has poor contrast (${headerRatio.toFixed(1)}:1) against ` +
3521+
`headerBg "${style.headerBg}". Minimum is ${MIN_CONTRAST}:1. ` +
3522+
`Use "${suggested}" instead.`
3523+
);
3524+
}
3525+
}
3526+
34873527
// Text baseline offset within a row: top padding + font size
34883528
// (drawText Y is the baseline position in top-left coords)
34893529
const padV = compact ? 2 : CELL_PAD_V;
@@ -3574,7 +3614,7 @@ function renderTable(
35743614
doc.drawText(headerText, textX, curY + textYOffset, {
35753615
font: style.headerFont,
35763616
fontSize,
3577-
color: style.headerFg,
3617+
color: style.headerBg ? autoTextColor(style.headerBg) : style.headerFg,
35783618
});
35793619
cellX += colWidths[c];
35803620
}
@@ -3606,6 +3646,13 @@ function renderTable(
36063646
doc.drawRect(x, curY, totalWidth, rowH, { fill: style.altRowBg });
36073647
}
36083648

3649+
// Auto-contrast body text against effective row background
3650+
const isAlt = !!(style.altRowBg && r % 2 === 1);
3651+
const rowBg = isAlt
3652+
? style.altRowBg
3653+
: (style._pageBg || "FFFFFF");
3654+
const rowFg = autoTextColor(rowBg);
3655+
36093656
// Cell text AFTER background
36103657
const isBoldRow = rowBold?.[r] ?? false;
36113658
const cellFont = isBoldRow
@@ -3619,7 +3666,7 @@ function renderTable(
36193666
doc.drawText(cellText, textX, curY + textYOffset, {
36203667
font: cellFont,
36213668
fontSize,
3622-
color: style.bodyFg,
3669+
color: rowFg,
36233670
});
36243671
cellX += colWidths[c];
36253672
}

‎builtin-modules/src/pptx-tables.ts‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ import {
1717
requireArray,
1818
requireNumber,
1919
isDark,
20+
autoTextColor,
21+
contrastRatio,
2022
type Theme,
2123
} from "ha:doc-core";
2224
import {
@@ -279,19 +281,22 @@ export function table(opts: TableOptions): ShapeFragment {
279281

280282
// Dark mode defaults: light text on dark alt-rows
281283
// Light mode defaults: dark text on light alt-rows
282-
const defaultTextColor = darkMode ? theme.fg || "E6EDF3" : "333333";
284+
// Auto-compute readable defaults based on backgrounds
283285
const defaultAltRowColor = darkMode ? "2D333B" : "F5F5F5";
284286
const defaultBorderColor = darkMode ? "444C56" : "CCCCCC";
285287

286288
const headerBg = style.headerBg || "2196F3";
287-
const headerColor = style.headerColor || "FFFFFF";
289+
const headerColor = style.headerColor || autoTextColor(headerBg);
288290
const headerFontSize = style.headerFontSize || 13;
289291
const styleFontSize = style.fontSize || 12;
290-
const textColor = style.textColor || style.themeTextColor || defaultTextColor;
291292
const altRows = style.altRows !== false;
292293
const altRowColor = style.altRowColor || defaultAltRowColor;
293294
const borderColor = style.borderColor || defaultBorderColor;
294295

296+
// ── Contrast enforcement ─────────────────────────────────────────
297+
// ALWAYS auto-contrast. No exceptions. If the color is shit, fix it.
298+
// The LLM's style.textColor is ignored — autoTextColor wins.
299+
295300
// Build grid columns
296301
const gridCols = Array.from(
297302
{ length: colCount },
@@ -317,14 +322,23 @@ export function table(opts: TableOptions): ShapeFragment {
317322
}
318323

319324
// Build data rows
325+
// ALWAYS auto-contrast text against each row's effective background
326+
// to prevent unreadable text on dark themes or image backgrounds.
327+
// If no theme.bg is provided, give non-alt rows an explicit fill
328+
// matching the alt-row scheme so text is always readable.
329+
const slideBg = theme.bg || (darkMode ? "1A1A1A" : "FFFFFF");
330+
const nonAltFill = darkMode ? slideBg : undefined;
320331
const dataRows = rows
321332
.map((row, rowIdx) => {
322333
const isAlt = altRows && rowIdx % 2 === 1;
334+
const rowBg = isAlt ? altRowColor : slideBg;
335+
// ALWAYS auto-contrast — no exceptions, no overrides
336+
const rowTextColor = autoTextColor(rowBg);
323337
const cells = row
324338
.map((cell) =>
325339
cellXml(cell, {
326-
fillColor: isAlt ? altRowColor : undefined,
327-
color: textColor,
340+
fillColor: isAlt ? altRowColor : nonAltFill,
341+
color: rowTextColor,
328342
fontSize: styleFontSize,
329343
borderColor,
330344
}),

‎builtin-modules/src/types/ha-modules.d.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1092,7 +1092,7 @@ declare module "ha:pdf" {
10921092
headerFg: string;
10931093
/** Header font. */
10941094
headerFont: string;
1095-
/** Body text colour (6-char hex). */
1095+
/** Body text colour (6-char hex). Auto-contrasted if omitted or poor contrast. */
10961096
bodyFg: string;
10971097
/** Body font. */
10981098
bodyFont: string;
@@ -1102,6 +1102,8 @@ declare module "ha:pdf" {
11021102
borderColor: string;
11031103
/** Border line width in points. */
11041104
borderWidth: number;
1105+
/** Page background colour (set internally for contrast checking). */
1106+
_pageBg?: string;
11051107
}
11061108
/** Built-in table styles matching PPTX table styles. */
11071109
export declare const TABLE_STYLES: Record<string, TableStyle>;

‎src/agent/index.ts‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -964,6 +964,119 @@ function loadModuleFilesForValidator(
964964
return { sources, dtsSources, moduleJsons };
965965
}
966966

967+
// ── Shared handler validation ──────────────────────────────────────
968+
//
969+
// Validates handler code using the Hyperlight analysis guest.
970+
// Used by both register_handler and edit_handler to ensure all
971+
// code changes go through the same validation pipeline.
972+
973+
interface ValidationResult {
974+
valid: boolean;
975+
isModule: boolean;
976+
error?: string;
977+
validationErrors?: Array<{ type: string; message: string; line?: number }>;
978+
}
979+
980+
async function validateHandlerCode(
981+
name: string,
982+
code: string,
983+
): Promise<ValidationResult> {
984+
const registeredHandlers = sandbox.getHandlers().filter((h) => h !== name);
985+
const availableModules = sandbox.getAvailableModules();
986+
987+
let validationContext: ValidationContext = {
988+
handlerName: name,
989+
registeredHandlers,
990+
availableModules,
991+
expectHandler: true,
992+
};
993+
994+
let isModule = false;
995+
996+
let validation = await validateJavaScriptGuest(code, validationContext);
997+
isModule = validation.isModule;
998+
999+
// Multi-pass module resolution loop
1000+
const maxIterations = 20;
1001+
let iterations = 0;
1002+
while (
1003+
!validation.deepValidationDone &&
1004+
validation.missingSources.length > 0 &&
1005+
validation.errors.length === 0 &&
1006+
iterations < maxIterations
1007+
) {
1008+
iterations++;
1009+
const {
1010+
sources: newSources,
1011+
dtsSources: newDtsSources,
1012+
moduleJsons: newModuleJsons,
1013+
} = loadModuleFilesForValidator(validation.missingSources, pluginManager);
1014+
1015+
if (Object.keys(newSources).length === 0) {
1016+
const unresolvable = validation.missingSources.filter(
1017+
(s) => !s.startsWith("ha:") && !s.startsWith("host:"),
1018+
);
1019+
if (unresolvable.length > 0) {
1020+
const suggestions = unresolvable
1021+
.map((s) => {
1022+
const asHa = `ha:${s}`;
1023+
const asHost = `host:${s}`;
1024+
if (availableModules.includes(asHa)) return `"${s}" → "${asHa}"`;
1025+
if (availableModules.includes(asHost))
1026+
return `"${s}" → "${asHost}"`;
1027+
return `"${s}"`;
1028+
})
1029+
.join(", ");
1030+
return {
1031+
valid: false,
1032+
isModule,
1033+
error: `Invalid import specifiers: ${suggestions}. Modules require "ha:" prefix (e.g., import { x } from "ha:pptx"), plugins require "host:" prefix.`,
1034+
};
1035+
}
1036+
break;
1037+
}
1038+
1039+
validationContext = {
1040+
...validationContext,
1041+
moduleSources: { ...validationContext.moduleSources, ...newSources },
1042+
dtsSources: { ...validationContext.dtsSources, ...newDtsSources },
1043+
moduleJsons: { ...validationContext.moduleJsons, ...newModuleJsons },
1044+
};
1045+
validation = await validateJavaScriptGuest(code, validationContext);
1046+
}
1047+
1048+
if (iterations >= maxIterations) {
1049+
const stillMissing = validation.missingSources.join(", ");
1050+
return {
1051+
valid: false,
1052+
isModule,
1053+
error: `Could not resolve all module dependencies after ${maxIterations} iterations. Still missing: ${stillMissing}.`,
1054+
};
1055+
}
1056+
1057+
// Report warnings to console
1058+
for (const warning of validation.warnings) {
1059+
console.error(` ${C.warn("⚠️")} ${warning.message}`);
1060+
}
1061+
1062+
if (!validation.valid) {
1063+
const errorMessages = validation.errors
1064+
.map((e) => {
1065+
const loc = e.line ? ` (line ${e.line})` : "";
1066+
return `${e.type}: ${e.message}${loc}`;
1067+
})
1068+
.join("\n • ");
1069+
return {
1070+
valid: false,
1071+
isModule,
1072+
error: `Validation failed:\n • ${errorMessages}`,
1073+
validationErrors: validation.errors,
1074+
};
1075+
}
1076+
1077+
return { valid: true, isModule };
1078+
}
1079+
9671080
// ── Tool: register_handler ────────────────────────────────────────────
9681081

9691082
const registerHandlerTool = defineTool("register_handler", {
@@ -1386,6 +1499,70 @@ const editHandlerTool = defineTool("edit_handler", {
13861499
oldString: string;
13871500
newString: string;
13881501
}) => {
1502+
// ── Preview the edit and validate before applying ─────────────
1503+
// Get current source to build the edited version
1504+
const sourceResult = sandbox.getHandlerSource(name, {
1505+
lineNumbers: false,
1506+
}) as { success: true; source: string } | { success: false; error: string };
1507+
1508+
if (!sourceResult.success) {
1509+
console.error(` ${C.err("❌ " + sourceResult.error)}`);
1510+
return { success: false, error: sourceResult.error };
1511+
}
1512+
1513+
const currentSource = sourceResult.source;
1514+
1515+
// Check exact-once match
1516+
const firstIdx = currentSource.indexOf(oldString);
1517+
if (firstIdx === -1) {
1518+
const error =
1519+
"oldString not found in handler. Use get_handler_source to see current code, then copy the EXACT text to replace.";
1520+
console.error(` ${C.err("❌ " + error)}`);
1521+
return { success: false, error };
1522+
}
1523+
const secondIdx = currentSource.indexOf(
1524+
oldString,
1525+
firstIdx + oldString.length,
1526+
);
1527+
if (secondIdx !== -1) {
1528+
const error =
1529+
"oldString matches multiple times. Add more surrounding context to make it unique.";
1530+
console.error(` ${C.err("❌ " + error)}`);
1531+
return { success: false, error };
1532+
}
1533+
1534+
// Build the edited code
1535+
const editedCode =
1536+
currentSource.slice(0, firstIdx) +
1537+
newString +
1538+
currentSource.slice(firstIdx + oldString.length);
1539+
1540+
// Validate the edited code through the same pipeline as register_handler
1541+
try {
1542+
const validation = await validateHandlerCode(name, editedCode);
1543+
if (!validation.valid) {
1544+
console.error(
1545+
` ${C.err("❌ Edit rejected by validation (handler unchanged):")}`,
1546+
);
1547+
console.error(` ${C.err(validation.error ?? "Unknown error")}`);
1548+
return {
1549+
success: false,
1550+
error: `Edit rejected — ${validation.error}`,
1551+
hint: "The edit was NOT applied. The handler is unchanged. Fix the error in your newString and try again.",
1552+
validationErrors: validation.validationErrors,
1553+
};
1554+
}
1555+
} catch (e) {
1556+
const errMsg = e instanceof Error ? e.message : String(e);
1557+
console.error(` ${C.err("❌ Validation error: " + errMsg)}`);
1558+
return {
1559+
success: false,
1560+
error: `Edit rejected — validation failed: ${errMsg}`,
1561+
hint: "The edit was NOT applied. The handler is unchanged.",
1562+
};
1563+
}
1564+
1565+
// Validation passed — apply the edit
13891566
const result = await sandbox.editHandler(name, oldString, newString);
13901567
if (result.success) {
13911568
console.error(

0 commit comments

Comments
 (0)