test(comments): не-вакуумные тесты audit-swallow + dominant-run; CHANGELOG (#512 ревью)
DO1: тест audit-swallow был вакуумен (log() — fire-and-forget void persist(), .not.toThrow() зелен независимо от try/catch). Теперь: после failInsert() шпионим Logger.warn + слушаем unhandledRejection, флашим микро/макротаски, ассертим warn=1 и 0 floating-rejection. Mutation: убрать try/catch у persist → красный. DO2: dominant-run тест не отличал longest от first (самый длинный ран был и первым). Перестроено: короткий plain ран впереди, длинный bold — следом → ассерт bold:true держится только при genuine longest; +тест tie→first. Mutation: reduce→segments[0] → красный. DO3: CHANGELOG [Unreleased] — 3 записи по #496. DO4: убран no-op override insertAttrs.comment (=dominant.markAttrs, а он и есть attributes['comment'] — сегменты фильтруются по нему; {...attributes,comment: attributes.comment}={...attributes}); коммент про 'defensive re-assert' исправлен. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -117,6 +117,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
### Added
|
||||
|
||||
- **A drifted comment suggestion can be re-synced instead of failing forever
|
||||
with a 409.** A suggestion whose stored anchor no longer matched the live
|
||||
document used to reject every apply attempt with an unrecoverable conflict; a
|
||||
new resync path re-reads the live anchor so the suggestion applies against the
|
||||
current text, and orphaned anchors (whose marked run was deleted) are
|
||||
reconciled rather than left blocking. (#496)
|
||||
|
||||
- **Place several images side by side in a row.** A new "Inline (side by
|
||||
side)" alignment mode in the image bubble menu renders consecutive inline
|
||||
images as a row that wraps onto the next line on narrow screens. The row is
|
||||
@@ -302,6 +309,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
### Fixed
|
||||
|
||||
- **Decisions on comment suggestions now leave a durable audit record.**
|
||||
Applying or dismissing a comment suggestion hard-deletes the (childless)
|
||||
subject comment, so the only surviving trace of who decided what is the audit
|
||||
event — but the audit trail was wired to a Noop service that silently
|
||||
swallowed every event. The trail is now DB-backed, so
|
||||
`comment.suggestion_applied` / `comment.suggestion_dismissed` (and the other
|
||||
comment-decision events) persist to the `audit` table and can be reviewed
|
||||
after the comment is gone. A persistence failure is still swallowed with a
|
||||
warning so it never breaks the originating request. (#496)
|
||||
- **Applying a comment suggestion no longer strips the replaced run's inline
|
||||
formatting.** The suggested text was re-inserted carrying only the comment
|
||||
anchor mark, silently dropping bold/italic/code/link on the affected run; the
|
||||
prevailing formatting of the replaced run is now carried onto the applied
|
||||
text. (#496)
|
||||
- **The server no longer runs out of heap during long autonomous agent runs.** A
|
||||
new pnpm patch on `ai@6.0.134` stops the SDK from building a cumulative
|
||||
snapshot of the ENTIRE turn text on every streamed text-delta when no output
|
||||
|
||||
@@ -583,18 +583,20 @@ describe('replaceYjsMarkedText', () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it('mixed formatting under the mark: replacement takes the DOMINANT run marks', () => {
|
||||
// Marked run = "bold" (bold, 4 chars) + "x" (plain, 1 char), same commentId.
|
||||
// The dominant (longer) segment is bold, so the flat replacement is bold.
|
||||
it('mixed formatting under the mark: replacement takes the DOMINANT (longest) run, NOT the leading one', () => {
|
||||
// Leading run is SHORT + plain ("x", 1 char); the following run is LONGER +
|
||||
// bold ("bolded", 6 chars), same commentId. The longest run is deliberately
|
||||
// NOT first: a "first-wins" pick would carry plain (no bold), so asserting
|
||||
// bold on the result only holds if the code genuinely selects the LONGEST run.
|
||||
const { fragment, text } = buildFormatted([
|
||||
{ text: 'x', attrs: { comment: { commentId: 'c1', resolved: false } } },
|
||||
{
|
||||
text: 'bold',
|
||||
text: 'bolded',
|
||||
attrs: { comment: { commentId: 'c1', resolved: false }, bold: true },
|
||||
},
|
||||
{ text: 'x', attrs: { comment: { commentId: 'c1', resolved: false } } },
|
||||
]);
|
||||
|
||||
const result = replaceYjsMarkedText(fragment, 'c1', 'boldx', 'Z');
|
||||
const result = replaceYjsMarkedText(fragment, 'c1', 'xbolded', 'Z');
|
||||
|
||||
expect(result).toEqual({ applied: true, currentText: 'Z' });
|
||||
expect(text.toDelta()).toEqual([
|
||||
@@ -604,4 +606,30 @@ describe('replaceYjsMarkedText', () => {
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it('mixed formatting under the mark: on a length tie the FIRST run wins', () => {
|
||||
// Two equal-length runs (2 chars each) with different formatting, same
|
||||
// commentId. The reduce keeps the accumulator on a tie, so the FIRST run
|
||||
// (italic) prevails over the later bold one.
|
||||
const { fragment, text } = buildFormatted([
|
||||
{
|
||||
text: 'AA',
|
||||
attrs: { comment: { commentId: 'c1', resolved: false }, italic: true },
|
||||
},
|
||||
{
|
||||
text: 'BB',
|
||||
attrs: { comment: { commentId: 'c1', resolved: false }, bold: true },
|
||||
},
|
||||
]);
|
||||
|
||||
const result = replaceYjsMarkedText(fragment, 'c1', 'AABB', 'Z');
|
||||
|
||||
expect(result).toEqual({ applied: true, currentText: 'Z' });
|
||||
expect(text.toDelta()).toEqual([
|
||||
{
|
||||
insert: 'Z',
|
||||
attributes: { comment: { commentId: 'c1', resolved: false }, italic: true },
|
||||
},
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -267,12 +267,11 @@ export function replaceYjsMarkedText(
|
||||
// when the marked run mixes formatting we pick the DOMINANT segment (the one
|
||||
// covering the most characters) and apply its attributes — a v1 that preserves
|
||||
// the common single-format case exactly and, for a mixed run, keeps the
|
||||
// prevailing style rather than losing all of it. The dominant segment already
|
||||
// carries the `comment` mark (every collected segment does), so the anchor is
|
||||
// preserved; we defensively re-assert it in case a future attribute shape
|
||||
// omits it.
|
||||
// prevailing style rather than losing all of it. `attributes` already carries
|
||||
// the `comment` mark (every collected segment is filtered on it above), so the
|
||||
// anchor is preserved by copying the run's attribute set verbatim.
|
||||
const dominant = segments.reduce((a, b) => (b.length > a.length ? b : a));
|
||||
const insertAttrs = { ...dominant.attributes, comment: dominant.markAttrs };
|
||||
const insertAttrs = { ...dominant.attributes };
|
||||
|
||||
node.delete(start, len);
|
||||
node.insert(start, newText, insertAttrs);
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { Logger } from '@nestjs/common';
|
||||
import { DatabaseAuditService } from './database-audit.service';
|
||||
import { AUDIT_CONTEXT_KEY } from '../../common/middlewares/audit-context.middleware';
|
||||
import { AuditEvent, AuditResource } from '../../common/events/audit-events';
|
||||
@@ -115,13 +116,39 @@ describe('DatabaseAuditService', () => {
|
||||
expect(inserted).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('a failed insert never throws to the caller (audit is a side-record)', async () => {
|
||||
const { service, failInsert } = makeService(ctx());
|
||||
failInsert();
|
||||
// Must not reject / throw.
|
||||
expect(() => service.log(applyPayload())).not.toThrow();
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
it('a failed insert is swallowed with a warn and floats no rejection (audit is a side-record)', async () => {
|
||||
// This pins the load-bearing swallow inside persist(). Because log() is
|
||||
// fire-and-forget (`void this.persist(...)`), it always returns synchronously
|
||||
// without throwing — so `not.toThrow()` alone would stay green even if the
|
||||
// try/catch were removed. We instead observe the two effects the catch is
|
||||
// responsible for: a warn IS emitted, and NO unhandled rejection floats.
|
||||
// Removing persist()'s try/catch reddens both assertions (warn count 0 + a
|
||||
// captured rejection).
|
||||
const warnSpy = jest
|
||||
.spyOn(Logger.prototype, 'warn')
|
||||
.mockImplementation(() => undefined as any);
|
||||
const rejections: unknown[] = [];
|
||||
const onRejection = (err: unknown) => rejections.push(err);
|
||||
process.on('unhandledRejection', onRejection);
|
||||
try {
|
||||
const { service, failInsert } = makeService(ctx());
|
||||
failInsert();
|
||||
expect(() => service.log(applyPayload())).not.toThrow();
|
||||
// Flush microtasks so the rejected insert settles, then give any floated
|
||||
// rejection a macrotask tick to be reported by the runtime.
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
await new Promise((r) => setImmediate(r));
|
||||
|
||||
expect(warnSpy).toHaveBeenCalledTimes(1);
|
||||
expect(String(warnSpy.mock.calls[0][0])).toContain(
|
||||
'Failed to persist audit event',
|
||||
);
|
||||
expect(rejections).toHaveLength(0);
|
||||
} finally {
|
||||
process.off('unhandledRejection', onRejection);
|
||||
warnSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it('logWithContext() persists with an explicit (non-request) context', async () => {
|
||||
|
||||
Reference in New Issue
Block a user