Skip to content

Commit bf86b21

Browse files
JohnMcLearclaude
andauthored
fix: correct padEditor author-attribute test off-by-one; run backend on PRs (#36)
Two linked problems surfaced when backend-tests finally ran on a main push: 1. Test off-by-one (the actual failure). The "inserts carry an author attribute even when authorId is empty" spec walked atext.attribs and required every op past the insert point to carry an author. But applyEdit splices the new text in *before* the pad's trailing '\n', so that original, AI-untouched newline is shifted to the document tail and correctly stays unattributed. The check skipped the seed prefix but had no upper bound, so it wrongly asserted on that trailing newline. Bound the check to [insertStart, insertEnd) — the span the AI actually wrote. The surgical-attribution code was right; only the assertion over-reached. 2. Why nobody noticed for ~2.5 weeks. backend-tests was skipped for same-repo PRs (it ran only on pushes or fork PRs), and the parent workflow only triggers push on main/master. So same-repo feature-branch PRs got no backend run, and this failure only ever showed up post-merge on main, where nothing blocks on it. Drop the guard so backend runs on every PR; it needs no secrets, so this is safe for forks too. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 29b6f72 commit bf86b21

2 files changed

Lines changed: 18 additions & 8 deletions

File tree

.github/workflows/backend-tests.yml

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,14 @@ on:
66

77
jobs:
88
withplugins:
9-
# run on pushes to any branch
10-
# run on PRs from external forks
11-
if: |
12-
(github.event_name != 'pull_request')
13-
|| (github.event.pull_request.head.repo.id != github.event.pull_request.base.repo.id)
9+
# Always run. The previous guard skipped this job for same-repo pull
10+
# requests (it only ran on pushes or fork PRs), but the parent workflow
11+
# only triggers `push` on main/master — so same-repo feature-branch PRs
12+
# got NO backend run at all, and failures only surfaced post-merge on
13+
# main where nothing blocks on them. A genuine padEditor regression sat
14+
# red on main for weeks as a result. Backend tests need no secrets, so
15+
# running them on every PR (incl. forks) is safe and is what actually
16+
# guards the branch before merge.
1417
name: with Plugins
1518
runs-on: ubuntu-latest
1619
steps:

static/tests/backend/specs/padEditor.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,8 @@ describe('ep_ai_chat - padEditor', function () {
362362

363363
const pad = await padManager.getPad(padId);
364364
const baseLen = pad.text().length; // includes trailing '\n'
365-
const result = await padEditor.applyEdit(pad, {appendText: '\nMore'});
365+
const appendText = '\nMore';
366+
const result = await padEditor.applyEdit(pad, {appendText});
366367
assert.ok(result.success,
367368
`append should succeed without authorId; got: ${JSON.stringify(result)}`);
368369

@@ -388,13 +389,19 @@ describe('ep_ai_chat - padEditor', function () {
388389
// createPad API before this PR's invariant existed.
389390
let cursor = 0;
390391
// applyEdit inserts at currentText.length - 1, i.e. before the
391-
// trailing '\n'. So the new span starts at baseLen - 1.
392+
// trailing '\n'. So the new span starts at baseLen - 1 and spans
393+
// exactly the inserted characters. The pad's original trailing '\n'
394+
// is pushed to the very end of the document and is intentionally
395+
// left unattributed — the AI never wrote it, so only [insertStart,
396+
// insertEnd) is the AI's work and must carry an author attribute.
392397
const insertStart = baseLen - 1;
398+
const insertEnd = insertStart + appendText.length;
393399
for (const op of Changeset.deserializeOps(updated.atext.attribs)) {
394400
const opStart = cursor;
395401
const opEnd = cursor + op.chars;
396402
cursor = opEnd;
397-
if (opEnd <= insertStart) continue; // entirely in the seed
403+
if (opEnd <= insertStart) continue; // entirely in the seed prefix
404+
if (opStart >= insertEnd) continue; // seed's trailing '\n', shifted past our insert
398405
let hasAuthor = false;
399406
for (const [key] of attribsFromString(op.attribs, pool)) {
400407
if (key === 'author') hasAuthor = true;

0 commit comments

Comments
 (0)