Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 41 additions & 5 deletions src/persistent-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,26 @@ export function normalizeSqliteBindings(values: unknown[]): Array<number | strin
return values.map(normalizeSqliteValue);
}

/**
* Unescape backslash escape sequences in a plain-text string before persisting.
* Converts common two-character escape artifacts (e.g. backslash-n from CLI
* argument passing) into their actual character equivalents so stored text is
* human-readable and free of accidental escape artifacts.
*
* Only the following sequences are converted (single-pass, left-to-right):
* \n -> newline
* \t -> tab
* \r -> carriage return
* \\ -> single backslash
*
* All other characters (including quotes and backticks) are left unchanged.
* This function must NOT be applied to JSON strings or structured fields.
*/
export function unescapeText(s: string): string {
const map: Record<string, string> = { '\\': '\\', n: '\n', t: '\t', r: '\r' };
return s.replace(/\\(\\|n|t|r)/g, (_, c: string) => map[c]);
}

export class SqlitePersistentStore {
private db: Database.Database;
private dbPath: string;
Expand Down Expand Up @@ -337,14 +357,27 @@ export class SqlitePersistentStore {
// runtime normalization elsewhere.
const normalizedStatus = normalizeStatusValue(item.status) ?? item.status;

// Unescape plain-text fields so backslash escape artifacts (e.g. \n from
// CLI argument passing) are stored as the intended characters.
// Structured/JSON fields (tags, refs, audit JSON) must NOT be unescaped here.
const titleVal = unescapeText(item.title ?? '');
const descriptionVal = unescapeText(item.description ?? '');
const deleteReasonVal = unescapeText(item.deleteReason ?? '');
// Unescape only the plain-text field within the structured audit object.
let auditVal: string | null = null;
if (item.audit) {
const auditCopy = { ...item.audit, text: unescapeText(item.audit.text ?? '') };
auditVal = JSON.stringify(auditCopy);
}

// Ensure we never pass `undefined` into better-sqlite3 bindings (it only
// accepts numbers, strings, bigints, buffers and null). Normalize tags to
// a JSON string and convert any undefined to null before running.
const tagsVal = Array.isArray(item.tags) ? JSON.stringify(item.tags) : JSON.stringify([]);
const values: any[] = [
item.id,
item.title,
item.description,
titleVal,
descriptionVal,
normalizedStatus,
item.priority,
item.sortIndex,
Expand All @@ -357,14 +390,14 @@ export class SqlitePersistentStore {
item.issueType ?? '',
item.createdBy ?? '',
item.deletedBy ?? '',
item.deleteReason ?? '',
deleteReasonVal,
item.risk ?? '',
item.effort ?? '',
item.githubIssueNumber ?? null,
item.githubIssueId ?? null,
item.githubIssueUpdatedAt ?? null,
item.needsProducerReview ? 1 : 0,
item.audit ? JSON.stringify(item.audit) : null,
auditVal,
];

const normalized = normalizeSqliteBindings(values);
Expand Down Expand Up @@ -601,11 +634,14 @@ export class SqlitePersistentStore {
// Pre-construction: stringify references, coerce optional fields.
// Preserve existing || behavior for githubCommentUpdatedAt so that
// falsy values (including empty string) become null.
// Unescape the comment body so backslash escape artifacts are stored as
// the intended characters. The refs JSON and other structured fields are
// intentionally left unchanged.
const values: unknown[] = [
comment.id,
comment.workItemId,
comment.author,
comment.comment,
unescapeText(comment.comment),
comment.createdAt,
JSON.stringify(comment.references),
comment.githubCommentId ?? null,
Expand Down
156 changes: 154 additions & 2 deletions tests/normalize-sqlite-bindings.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
/**
* Tests for normalizeSqliteValue and normalizeSqliteBindings
* Tests for normalizeSqliteValue, normalizeSqliteBindings, and unescapeText
* (WL-0MLRSV1XF14KM6WT)
*/

import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { normalizeSqliteValue, normalizeSqliteBindings } from '../src/persistent-store.js';
import { normalizeSqliteValue, normalizeSqliteBindings, unescapeText } from '../src/persistent-store.js';
import { WorklogDatabase } from '../src/database.js';
import { createTempDir, cleanupTempDir, createTempJsonlPath, createTempDbPath } from './test-utils.js';

Expand Down Expand Up @@ -287,3 +287,155 @@ describe('SQLite binding round-trip', () => {
expect(outbound[0].toId).toBe(b.id);
});
});

// ---------------------------------------------------------------------------
// Unit tests for unescapeText
// ---------------------------------------------------------------------------

describe('unescapeText', () => {
it('returns an empty string unchanged', () => {
expect(unescapeText('')).toBe('');
});

it('passes through plain text with no escape sequences', () => {
expect(unescapeText('Hello World')).toBe('Hello World');
});

it('converts \\n to a real newline', () => {
expect(unescapeText('Line\\nBreak')).toBe('Line\nBreak');
});

it('converts \\t to a real tab', () => {
expect(unescapeText('Col\\tValue')).toBe('Col\tValue');
});

it('converts \\r to a real carriage return', () => {
expect(unescapeText('Foo\\rBar')).toBe('Foo\rBar');
});

it('converts \\\\ to a single backslash', () => {
expect(unescapeText('path\\\\file')).toBe('path\\file');
});

it('handles multiple escape sequences in a single string', () => {
expect(unescapeText('a\\nb\\tc\\\\d')).toBe('a\nb\tc\\d');
});

it('does not double-decode when a backslash precedes a backslash-n', () => {
// Input: 4 chars: \ \ n -> backslash + n (not a newline)
expect(unescapeText('\\\\n')).toBe('\\n');
});

it('preserves double quotes unchanged', () => {
expect(unescapeText('say "hello"')).toBe('say "hello"');
});

it('preserves backticks unchanged', () => {
expect(unescapeText('use `code`')).toBe('use `code`');
});

it('preserves unrecognised backslash sequences unchanged', () => {
// \x is not a recognised sequence; the backslash is kept as-is
expect(unescapeText('foo\\xbar')).toBe('foo\\xbar');
});
});

// ---------------------------------------------------------------------------
// Integration round-trip tests: unescaping applied on DB write
// ---------------------------------------------------------------------------

describe('unescapeText round-trip via DB', () => {
let tempDir: string;
let dbPath: string;
let jsonlPath: string;
let db: WorklogDatabase;

beforeEach(() => {
tempDir = createTempDir();
dbPath = createTempDbPath(tempDir);
jsonlPath = createTempJsonlPath(tempDir);
db = new WorklogDatabase('UT', dbPath, jsonlPath, true, true);
});

afterEach(() => {
db.close();
cleanupTempDir(tempDir);
});

it('stores description with real newline when input contains \\n escape artifact', () => {
const created = db.create({
title: 'Escape test',
description: 'Line\\nBreak',
});

const loaded = db.get(created.id);
expect(loaded).toBeDefined();
// Stored text must contain a real newline, not the two-char sequence \n
expect(loaded!.description).toBe('Line\nBreak');
expect(loaded!.description).not.toContain('\\n');
});

it('stores title with real newline when input contains \\n escape artifact', () => {
const created = db.create({
title: 'Title\\nWith Escape',
});

const loaded = db.get(created.id);
expect(loaded).toBeDefined();
expect(loaded!.title).toBe('Title\nWith Escape');
expect(loaded!.title).not.toContain('\\n');
});

it('stores comment body with real newline when input contains \\n escape artifact', () => {
const item = db.create({ title: 'Escape comment test' });

db.createComment({
workItemId: item.id,
author: 'tester',
comment: 'First\\nSecond',
references: [],
});

const comments = db.getCommentsForWorkItem(item.id);
expect(comments).toHaveLength(1);
expect(comments[0].comment).toBe('First\nSecond');
expect(comments[0].comment).not.toContain('\\n');
});

it('unescapes audit text field but leaves audit JSON structure intact', () => {
const created = db.create({
title: 'Audit escape test',
audit: {
time: '2026-01-01T00:00:00.000Z',
author: 'tester',
text: 'Ready to close: Yes\\nExtra detail',
status: 'Complete',
},
});

const loaded = db.get(created.id);
expect(loaded).toBeDefined();
expect(loaded!.audit).toBeDefined();
// audit.text should have a real newline
expect(loaded!.audit!.text).toBe('Ready to close: Yes\nExtra detail');
expect(loaded!.audit!.text).not.toContain('\\n');
// Structured audit fields must remain intact
expect(loaded!.audit!.author).toBe('tester');
expect(loaded!.audit!.status).toBe('Complete');
});

it('does not alter tags (JSON field) when description contains escape artifacts', () => {
const created = db.create({
title: 'Tags intact',
description: 'Desc\\nValue',
tags: ['tag\\none', 'normal'],
});

const loaded = db.get(created.id);
expect(loaded).toBeDefined();
// Description should be unescaped
expect(loaded!.description).toBe('Desc\nValue');
// Tags are JSON-structured; the raw tag values are preserved as-is
expect(loaded!.tags).toEqual(['tag\\none', 'normal']);
});
});
Loading