feat: add break-on-record-write triage

[ianrayianray]

Summary

This PR implements roadmap item 7, Break-on-record-write triage.

• Adds bcdev_record_writes_start, bcdev_record_writes_status, and bcdev_record_writes_finish.
• Arms Business Central's global record-write debugger switch for one selected session, identifies each paused write's runtime table ID, and automatically continues the workload.
• Returns exact target-table writers grouped by operation, receiver, and complete AL call stack while counting proven unrelated writes separately.
• Supports next-session, user-filtered, and exact-session targeting through the existing debugger attachment contract.
• Excludes temporary-record writes by default, with an explicit option to include them.
• Uses deployed source first; local source can establish exact evidence only after the caller asserts changesDeployed: true.
• Bounds collection with maxObservedWrites and releases the workload when the cap is reached.

Failure and lifecycle behavior

• complete: true applies only to the observed writes in one attached-session capture window; it is not a tenant-wide claim.
• Missing source or statement spans, unsupported or ambiguous receivers, watch failures, malformed runtime identities, unexpected breaks, early session detach, truncation, or debugger failures keep the report incomplete.
• Exact groups retained before a session detach remain available, but the report warns that later writes may be missing.
• Explicit with <record> do contexts resolve the actual receiver, including nested blocks.
• Valid parenthesis-free Insert, Modify, Delete, and DeleteAll calls are classified.
• Spans containing multiple write candidates fail closed with the stable multipleWriteCandidates reason.
• Rejected post-bind debugger configuration and unexpected clean hub closure fail the retained report closed.
• Finish keeps event intake open across a best-effort ReleaseConnection barrier, drains in-flight asynchronous Break callbacks, and avoids dropping or double-releasing the paused workload.
• Record-write triage and manual debugging share one atomic singleton debugger slot.

Compatibility

• Existing manual bcdev_debug_attach { breakOnRecordWrite: ... } behavior remains available.
• Existing debugger event and stack-frame fields remain intact; the full optional statement span is additive.
• Raw stack evidence is retained, and reports do not store record values, source bodies, credentials, or authenticated URLs.

Validation

• bun test: 598 passed, 0 failed
• bun run typecheck: passed
• bun run build: passed
• Embedded-skill drift and diff checks: passed

Validated against a disposable Business Central SaaS Sandbox fixture:

• The full capture observed 13 writes: 10 exact target writes in 9 groups, 3 proven unrelated writes, and 0 unresolved writes.
• All six documented write operations, repeated and distinct caller stacks, runtime RecordRef, deployed-source classification, and full statement spans were exercised.
• Enabling temporary writes added exactly one target write.
• Local-source fallback remained unresolved without the deployment assertion and classified successfully with it.
• A one-write cap released the workload and returned a truncated, incomplete report.
• An explicit with Target inside a different table's method classified Target, not the host Rec; parenthesis-free Insert, Modify, Delete, and DeleteAll all passed with zero unresolved writes.
• The idiomatic if not Record.Insert(false) then Record.Modify(true) produced individual write-call spans; all 5 observed writes classified exactly with 0 unresolved.
• A session detach before finish retained its exact groups while returning complete: false, stopReason: "sessionDetached", and a warning.
• Finish cleared the shared debugger slot and an immediate manual attach/detach succeeded.

Redacted live evidence is recorded in scripts/e2e-break-on-record-write-triage-2026-07-25.md.

No Production or on-premises live calls were made.

[SShadowS]

Reviewed the full diff against bc9c6ae, ran it in an isolated worktree, probed the parser and collector lifecycle, and checked the load-bearing wire claims against the decompiled reference.

Verified locally: 594 pass / 0 fail, typecheck clean, build 1.36 MB, tree clean after build. CI is green on the PR.

Design check: the wire forces this approach

Worth recording so nobody "simplifies" it later. Break(ApplicationObjectIdWrapper objectId, StackFrame[] stack, string? errorMessage) carries no written-table identity, only the code object. Source parsing plus a receiver watch is the only route to a table ID. Also confirmed against the reference:

• BreakOnRecordWriteBehaviour is Unspecified, None, All, ExcludeTemporary, so 3 is correct, and BreakOnErrorBehaviour ends in ExcludeTry, also 3. The shared breakBehaviour mapping is right for both callers.
• SourcePosition is ushort, so the >= 0 guards in normalizeStatementSpan are harmless belt and braces, and an all-zero span really is SourceSpan.Default, meaning unset.

Should fix

1. A mid-capture detach still reports complete: true

record-write-triage.ts:632-639. The detached handler sets stopReason = "sessionDetached" but never sets evidenceLost and pushes no warning. Probed:

### mid-capture detach
complete: true | stopReason: sessionDetached | outcome: completed | warnings: []

A detached during triage is always externally initiated, since the tool never detaches itself (finish uses release plus stop). So the capture window was cut short by something the caller did not choose, the workload may have kept writing undebugged, and the report still says the writer list is complete. nextSteps then says "Review the grouped writer stacks" with no caveat.

Compare finish() at L567-570, which correctly sets evidenceLost when no session ever bound. Same class of evidence truncation, handled there but not here. The test at record-write-triage.test.ts:570 asserts outcome, stopReason, and summary for the detach case, and pointedly not complete.

Fix: on detached, push a warning and set evidenceLost = true, or expose a distinct windowEndedEarly flag.

2. The idiomatic AL upsert lands in writeStatementUnrecognized

inspectStatement at L291 requires exactly one write candidate in the span. Probed with correctly computed multi-line spans:

"if not Customer.Insert(false) then\n            Customer.Modify(true);"
  -> {"statement":null,"reason":"writeStatementUnrecognized"}
"Customer.Insert(false)"                   -> insert/Customer   OK
"if Customer.Insert(false) then exit;"     -> insert/Customer   OK
"begin\n  Customer.Modify();\nend;"        -> modify/Customer   OK
"Customer.Modify(); // Customer.Insert();" -> modify/Customer   OK (comments ignored)

If Business Central reports the enclosing statement span for if not X.Insert(false) then X.Modify(true);, which is the single most common AL write idiom, then both breaks land unresolved and complete goes false. Which way it goes depends entirely on the server's span granularity, and the live fixture does not appear to contain a conditional two-write statement. The evidence lists parenthesis-free forms, all six operations, and RecordRef, but no if X.Insert() then Y.

Two asks. Add that shape to the live checklist so the span granularity is known rather than assumed. And give it its own reason, something like multipleWriteCandidates, instead of writeStatementUnrecognized, which reads as a parse failure. If every candidate resolves to the same receiver and table, classifying it is defensible.

Notes

• RecordWriteCollectorOptions.includeTemporary is dead. Filtering happens at attach (record-write-tools.ts:213 sets breakOnRecordWrite: "nonTemporary"); the collector never reads its copy. Drop it or assert on it, otherwise someone later "fixes" filtering in the wrong layer.
• Dotted chains take the last segment. Outer.Inner.Insert() resolves to receiver Inner (probed). Function-call and indexed receivers correctly fail closed with receiverUnsupported, but a chain does not. If Inner happens to resolve to a different table, the write is silently counted as unrelated, which is a false negative rather than an unresolved. Barely reachable in valid AL, but worth failing closed on chain length above 1.
• explicitWithContext re-lexes the whole object on every break. Measured 21 ms per classification on a 20k-line object. Only the implicit-receiver path pays it; explicit receivers skip it. At the 500 default cap that is roughly 10 s of lexing with the workload held paused. Caching tokens alongside sourceCache would mirror what the source cache already does.
• An all-zero span still yields line: 1. debugger-hub.ts:258: when normalizeStatementSpan rejects the unset value-type span, the fallback computes 0 + 1. That is pre-existing behaviour, but the code now explicitly knows the span is unset, and the project's own breakpoint rule is that all-zero means unset, not line 1. A non-integer from.line yields line: 0.
• No on-prem validation, which the PR states honestly. The record-write switch itself was BC28-validated in PR feat(debugger): power controls v2 — precision breaks, source download, abort/release, SQL insight #6, so the risk sits mostly in the new tool flow. I have a BC28 container available and can run the three tools end to end there, including the if X.Insert() then fixture from finding 2, if that is useful to you.

Solid, no change needed

Stale-callback guards (this.hub !== hub) on Break and OnFatalDebuggerException close a real hole. The slot claim is synchronous from assertDebugSlotAvailable through claimDebugSlot in both handlers, so it is genuinely atomic. Span-end handling tolerates plus or minus one column, so the inclusive versus exclusive question cannot bite. The lexer ignores comments and string literals. Redaction holds: no record values, no source bodies, safeDetail runs through redactAuthorization, and a test asserts a bearer token does not leak. The release barrier in finish is reasoned about correctly, and its double-release assumption matches ReleaseConnection semantics.

Test coverage is the strongest part of this PR. The MCP-layer tests cover rejected post-bind configuration, unexpected clean hub close, and out-of-band user-filter rejection, which are exactly the paths that would otherwise fail open.

Verdict

Ship it after finding 1. That one is a false all-clear on a scenario that will happen in normal use (session ends, browser closed, server recycles the session) and it is a two-line fix. Finding 2 matters more for day-to-day usefulness, but it needs a live answer before you know which fix is right, so it is a reasonable follow-up rather than a blocker, provided the checklist box goes in now.

[ianrayianray]

Addressed in afa6ca5.

• A detach received before bcdev_record_writes_finish now retains collected exact groups but sets complete: false, records stopReason: "sessionDetached", and adds an early-window warning. A detach caused by finish's own release barrier remains a deliberate finished window.
• Added deterministic coverage for external detach with both terminateSession values, finish-induced detach, and MCP next-step guidance.
• Added the conditional-upsert shape to the disposable SaaS fixture. Business Central returned individual write-call spans for Insert and Modify: 5 observed, 5 matched, 0 unresolved.
• Added stable multipleWriteCandidates fail-closed reporting for any enclosing span that does contain more than one write.
• Dotted receiver chains now fail closed as receiverUnsupported, and the unused collector-level includeTemporary copy was removed. Temporary filtering remains at debugger attach, where the wire option is configured.

Validation:

• bun test: 598 passed, 0 failed
• bun run typecheck: passed
• bun run build: passed (1.36 MB)
• Embedded-skill regeneration and diff checks: passed
• Full SaaS Sandbox matrix rerun; no Production or on-premises call made

The redacted live evidence and checklist are updated in the branch.

[SShadowS]

Verified afa6ca5 in an isolated worktree: 598 pass / 0 fail, typecheck clean, build 1.36 MB.

Probed the detach split directly. An external detach retains its exact groups and reports complete: false, stopReason: "sessionDetached", and the early-window warning; a detach caused by finish's own release barrier stays a deliberate finished window; the happy path is unchanged. Statement shapes now report multipleWriteCandidates for two writes in one span, and dotted chains, indexed receivers, and function-return receivers all fail closed. Making an unsupported receiver win over a valid match in the same span is a tightening beyond what I asked for, and it is the right call.

Thanks for taking the conditional upsert to the live fixture. An individual write-call span per operation is exactly the fact that needed to be evidence rather than assumption.

One non-blocking note for a later doc pass. Your evidence records that SaaS Background completion emits OnDetachedFromConnection before finish, so a Background workload that ends normally will always report complete: false. That is the correct conservative result: OnDetachedFromConnection(bool terminateSession) only maps to a DAP Terminated event in the official adapter, so the wire carries no "the workload finished" semantics and a clean end cannot be distinguished from an early death. Worth one line in the skill so agents do not read complete: false on every Background capture as a malfunction. Related nit: nextSteps for that case says "Review unresolved groups and warnings" even when there are zero unresolved groups.

Still open from the earlier review, all filed as notes rather than asks: the per-break re-lex cost in explicitWithContext, an all-zero span still reporting line: 1, and no on-prem validation.