fix: apply the adversarial-review findings for the dev train promoted by #272

[mberrys]

fix: apply the adversarial-review findings for the dev train promoted by #272

What changed

An adversarial review of the dev changeset promoted by #272 (five independent reviewers, one
subsystem slice each, findings verified against the source before classification) produced 31
findings; this PR fixes the accepted ones with regression tests, dismisses two with evidence and
defers two with reasons. The full labelled finding set lives in
docs/reviews/2026-10-11-dev-review-272.md; the change fragment for this branch is
changes/fix-dev-review-272.md. dev is not the default branch; this PR closes no issue.

Bug fixes and changes

Area	Severity	Finding (review #)	Fix	Regression test
Core	critical	Named-destination outline items resolved to page 0, so a bookmark toggle deleted them, export rewrote them as page 1 and auto-generate skipped page 1 (1)	resolveDestinationPage refuses a named or unparsed destination before the reference/index lookup; toggle no longer matches them, export refuses them with bookmark/unsupported-outline-item	namedDestinationOutlineIsNeverAPageTarget
Core	critical	The XMP merge could emit undeclared namespace prefixes, duplicate a property that lived in another rdf:Description, and never replace attribute-form properties, all while reporting success (2)	Presence pre-scan (element and attribute occurrences), attribute substitution, the namespace is declared on replaced and appended properties, and an ambiguous duplicate refuses with metadata/xmp-unreadable	metadataReplacesAPropertyCarriedInASecondDescription, metadataDeclaresAppendedPropertyNamespaces, metadataSubstitutesAttributeFormProperties, metadataRefusesAmbiguousPropertyOccurrences
Core	warning	Attachment add/remove silently unlisted a file specification stored as a direct object (3)	Both paths refuse with the new additive code attachment/specification-not-addressable instead of dropping the entry	attachmentInlineSpecificationsAreRefusedNotDropped
Core	warning	A form value edit updated only the first same-named field; the canonical path propagates to all (4)	The value propagates to every same-named field with scope Internal, exactly as PDFFormManager::setFormFieldValue does; a widget-bearing match is preferred over a widget-less parent	sameNamedFormFieldsAreUpdatedTogether, widgetlessFormFieldMatchFallsBackToTheWritableTwin
Core	warning	The outline rewrite guard accepted a GoTo whose destination the writer nulls (/Dest null state loss) (5)	The guard requires a valid destination; a named destination stays accepted (it round-trips)	unparsedDestinationOutlineRefusesAnOutlineRewrite
Core	nit	PDFRevisionIdentity::fromString accepted a corrupted hash as a valid identity (6)	The hash field is validated as an even-length hex run before decoding (the empty run stays valid: that is what toString() produces for an in-memory document, and rejecting it broke every post-edit revision round trip)	revisionIdentityRoundTripsThroughItsStringForm (corrupted shapes rejected, empty-hash identity round-trips)
Interaction	critical	The HiDPI screenshot composed every tile at device-pixel-ratio-times its placed rect (7)	Tiles draw at their placed rect in viewport units; the image's own ratio maps to device pixels	canvasCompositionDrawsPlacedTilesInViewportUnitsOnHiDpi (ratio 2.0)
Interaction	critical	Image extraction ignored the document's CopyContent permission while PdfTool refuses the same operation (8)	extractPage refuses with the CLI's own sentence; the host gates actionExtractImage on the copy permission at the UI seam	imageExtractionRefusesWhenCopyContentIsDenied (real encrypted fixture, user vs owner authorization)
Interaction	warning	Copy text ignored the CopyContent permission (9)	copySelectedText refuses before the clipboard; actionCopyText joins the export command's permission gate	host gate plus the catalog-state assertions in UnitTestsEditorHost
Interaction	warning	The no-text-layer refusal was one-shot: the empty layout was cached before the check and later calls reported an available empty selection (10)	The cached path re-runs the empty-blocks check	selectionSourceRefusesAPageWithNoTextLayerOnEveryCall
Interaction	warning	A refused signature-check completion wedged m_signatureCheckRunning, leaving a signed document "not-signed" and every later check refused (11)	The refused current check releases the running state (a superseded completion cannot release a newer check)	refusedSignatureCheckCompletionReleasesTheRunningCheck, supersededSignatureCheckCompletionDoesNotReleaseTheNewerCheck
Interaction	warning	The annotation cap plus the finding cap equalled the frame budget, so annotations could starve the steering primitives (12)	maxAnnotationPrimitives 1000 to 500: findings plus annotations can never consume more than 3500 of 4000 slots	findingAndAnnotationCapsLeaveHeadroomForSteeringPrimitives
Interaction	nit	Unreachable supersede branch in the document-edit family (13)	Dead branch removed; the busy refusal above it is the policy	n/a (dead code)
Interaction	warning	The annotation-family tests were driven by the table they should pin, so a dropped row shipped a dead "implemented" command invisibly (14)	The 18-id table is pinned against a literal list	catalogFamilyMatchesTheLiteralCommandList
Editor	warning	Send by E-Mail announced a mail hand-off that no receiver performs (16)	The Quick shell opens the mail client with the published file and revision	covered by the shell receiver and the existing signal test
Editor	warning	Bookmark import/export finished Completed while doing nothing; their file-dialog half had no receiver (17)	Main.qml receivers answer the host invokables, mirroring the screenshot dialogs	covered by the strengthened bookmark round trip in UnitTestsEditorHost
Editor	warning	The soft-proof state label bound a property as a function call, so it never rendered (18)	Property binding (root.host.softProofSummary)	seen by the localization probe's label coverage
Editor	warning	The ink-coverage readout published numbers for a stale job and outlived its revision (19)	Only Succeeded publishes; the readout is dropped on revision change	inkCoverageRefusesSupersededJobsAndDropsTheReadoutOnRevisionChange
Editor	nit	The annotations tab button lacked the wrapping content item of its three siblings (21)	Wrapper copied verbatim in both QML copies	pseudo-locale layout probe
Gates	critical	check_quick_localization.py was red on its own dev tip: actionExportDocument used a label form the checker rejects (23)	The MenuModel label uses the ordinary switch arm, like actionPrint; the extraction is factored and covered	test_check_quick_localization.py (+3 cases, pre-fix replay of the missing set)
Gates	warning	The #126 checker and the help-strings staleness check ran in no gate (24)	Wired into ci.yml and the whole-tree guards in check-change.py	the checker unit tests run in both lanes
Gates	warning	The handler-coverage ratchet could be grown by its own tool; "may only shrink" was enforced nowhere (25)	Recording refuses additions unless --accept-new-gaps <reason> records a deliberate gap; the reason is stored beside the ids	test_command_catalog.py (growth refused, opt-in recorded, shrink)
Gates	warning	The identity checker could not see packaging templates; a template naming the product by its pre-rebrand name sat in the tree (26)	.in/.ui/.qrc are scanned; the Wix installer template names the real LOOP artifacts	test_check_loop_identity.py (a legacy token in a .in file is a finding)
Gates	warning	The action disposition vocabulary was validated only by the PowerShell lane (27)	verify-command-catalog.py validates disposition and target against the schema vocabulary; the Linux twin mirrors it	test_command_catalog.py (typo'd disposition and bad target are RED)
Gates	nit	ADR-014 pinned counts that disagree with the shipped registers (28)	113 actions, 35 unbound-at-authoring ids, ratchet claim requalified against the real register	n/a (docs)
Tests	warning	The bookmark import/export test could not fail if the bookmark command no-oped	The custom record is asserted before export and the invocation result is read	bookmarkImportExportRoundTripsAndRefusesAStaleRevision
Tests	warning	The claimed screenshot overwrite refusal had no test	New test with a positive control (a first capture must succeed) then a planted file must be preserved byte-for-byte	screenshotCaptureRefusesToOverwriteAnExistingFile
Tests	warning	The only test of the ten shipping catalogs was always skipped; nothing wired the env var	UnitTestsLoopStateVisual gets LOOP_TRANSLATION_TEST_DIRECTORY from the ctest registration	the catalog test now runs against all ten compiled catalogs

Dismissed and deferred (recorded in the review report)

• Dismissed: the signature badge after an in-place edit (15). The facts describe the source
  bytes by documented contract; the operator-visible defect was the wedge (11), fixed.
• Dismissed: the ../../ include of shellglossary.h in loopstatevisual.cpp (22). The
  relative include is load-bearing: UnitTestsLoopStateVisual compiles the file directly without
  LoopLibInteraction/sources on its include path, so the plain include fails the build
  (reproduced).
• Deferred: history re-verification on the interactive thread (20) and the self-referential
  accessible-readout test (32). Both need structure work (the worker pattern; the host
  composition extracted), tracked in the review report.

Proof

python scripts/agent/check-change.py --base origin/dev --head-branch fix/dev-review-272 --build-dir C:/.dev/repos/loop-build-fixdev reports pass at the head commit, 6d5eebd8: 127/127 checks,
modules build_policy, core, documentation, interaction, quick, risk high.

• Every mapped target built (80 build checks across the 7 target groups: LoopEditor, LoopLibCore,
  LoopLibInteraction, LoopLibQuick, PdfTool, ProductQuickAccessibilitySmoke, UnitTests) and every
  mapped suite ran green: 73 tests, including the ten-suite sweep that caught two integration
  defects before the gate run (the reported relative-include premise, disproved by the build; and
  the fromString empty-hash round trip, which broke every post-edit revision and is pinned by a
  new case).
• clang-format green on all 19 changed C/C++ files and clang-tidy 19.1.0 green on the 9 changed
  translation units.
• Whole-tree checkers green: source integrity, architecture catalog, policy adapters, loop identity,
  governance registers (and its negative cases), command catalog and its cases, Quick localization
  and its cases, help-strings staleness, QML mirror parity, search budget, independent validation
  gate, architecture contracts, Windows Qt test runtime, changelog.
• Direct pre-gate runs: UnitTestsEditorHost 84/84, UnitTestsDirectEdit 20/20.

Not run locally: the pwsh/WSL-dependent lanes (Windows installer, Flatpak, release gate) and the
packaging lanes, which run in CI; no release-profile build on this host.

Quality pass

The diff carries no change-narrating comments, no weakened assertions, and no suppressions. Two
comments are kept on purpose: the replaced-XMP-element namespace rationale (the binding the
replaced element carried is why the declaration exists) and the release condition on the
signature-check flag (why a superseded completion must not clear a newer check). Formatting is
clang-format clean on every touched file; whole-file reformat churn is zero.

Security and rollback

The copy-permission refusals close a real gap (extraction and clipboard bypassed CopyContent
while the CLI refused the identical operations). No new unsafe construct; the XMP merge parses at
the boundary and refuses shapes it cannot merge. Rollback: revert this branch. No persisted format
changed; the only interface movement is one additive DirectEditRefusal enum value
(attachment/specification-not-addressable, appended after WriteRefused so existing values keep
their numbers), named with the LoopLibCore/sources/** and UnitTests/CMakeLists.txt
protected-path edits in the change fragment.

Docs

docs/reviews/2026-10-11-dev-review-272.md (the finding set, dismissals, verified-clean list),
changes/fix-dev-review-272.md + .evidence.yaml, and the corrected ADR-014.

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}