From 626ce488c5b247a52e4220c483f567df0bf4f512 Mon Sep 17 00:00:00 2001 From: mbx30 <212453881+mberrys@users.noreply.github.com> Date: Fri, 25 Sep 2026 12:03:11 -0700 Subject: [PATCH 1/3] fix(core): fence scheduled results across revisions and requests (#17) --- LoopEditor/editorhost.cpp | 95 ++++++++++++++----- LoopLibCore/sources/pdfdiff.cpp | 12 ++- LoopLibCore/sources/pdfjobscheduler.cpp | 55 +++++++++-- LoopLibCore/sources/pdfjobscheduler.h | 10 +- LoopLibInteraction/sources/documentfacade.cpp | 40 +++++++- LoopLibInteraction/sources/jobsubmitter.h | 4 +- .../sources/pagesurfacecoordinator.cpp | 39 ++++++++ UnitTests/tst_documentfacadetest.cpp | 20 +++- UnitTests/tst_jobschedulertest.cpp | 54 +++++++++++ UnitTests/tst_pagesurfacetest.cpp | 47 ++++++++- UnitTests/tst_preflightinteraction.cpp | 1 + UnitTests/tst_viewportcommandbridgetest.cpp | 5 + ...e-17-fence-scheduled-results.evidence.yaml | 46 +++++++++ .../codex-issue-17-fence-scheduled-results.md | 4 + 14 files changed, 390 insertions(+), 42 deletions(-) create mode 100644 changes/codex-issue-17-fence-scheduled-results.evidence.yaml create mode 100644 changes/codex-issue-17-fence-scheduled-results.md diff --git a/LoopEditor/editorhost.cpp b/LoopEditor/editorhost.cpp index 4bf452777..8f1301f27 100644 --- a/LoopEditor/editorhost.cpp +++ b/LoopEditor/editorhost.cpp @@ -292,6 +292,10 @@ QVariantMap descriptorToVariant(const pdfinteraction::CommandDescriptor& descrip struct EditorHost::PreflightWorkerOutcome { pdf::PreflightResult result; + QString effectiveProfileDigest; + QString documentPath; + QByteArray auditBytes; + QJsonObject auditSummary; }; EditorHost::EditorHost(QObject* parent) : @@ -1449,6 +1453,7 @@ bool EditorHost::runPreflight() profile.effectiveDigest = pdf::computeProfileDigest(bound.profile); profile.profileIdentity = imported.identity.toJson(); profile.profileIdentity.insert(QStringLiteral("effective_digest"), profile.effectiveDigest); + outcome->effectiveProfileDigest = profile.effectiveDigest; context.reportProgress(5); std::unique_ptr session( @@ -1481,26 +1486,9 @@ bool EditorHost::runPreflight() context.reportProgress(15); outcome->result = engine.run(profile); pdf::finalizePreflightResult(outcome->result, revisionHash, resolved); - - pdf::PDFOperationHistoryStatus auditStatus = pdf::PDFOperationHistoryStatus::Accepted; - if (context.isCancellationRequested()) - auditStatus = pdf::PDFOperationHistoryStatus::Cancelled; - else if (pdf::reducePreflightVerdict(outcome->result).state == pdf::PreflightVerdictState::Error) - auditStatus = pdf::PDFOperationHistoryStatus::Failed; - - const QJsonObject auditSummary = - pdf::preflightAuditReportSummary(outcome->result, documentPath); - if (const pdf::PDFOperationResult auditResult = - pdf::appendPreflightAuditRun(documentPath, - auditBytes, - outcome->result, - auditStatus, - QStringLiteral("LoopEditor"), - auditSummary); - !auditResult) - { - throw std::runtime_error(auditResult.getErrorMessage().toStdString()); - } + outcome->documentPath = documentPath; + outcome->auditBytes = std::move(auditBytes); + outcome->auditSummary = pdf::preflightAuditReportSummary(outcome->result, documentPath); if (context.isCancellationRequested()) return; @@ -3209,11 +3197,47 @@ void EditorHost::finishPreflightJob(const pdf::PDFJobSnapshot& snapshot) return; } + const auto profile = std::find_if(m_preflightProfiles.cbegin(), m_preflightProfiles.cend(), + [this](const PreflightProfileChoice& choice) + { return choice.id == m_selectedPreflightProfileId; }); + if (!hasDocument() || !m_session->revisionSource() || snapshot.kind != pdf::PDFJobKind::Preflight || + snapshot.documentKey != m_preflight.documentKey() || + snapshot.documentKey != m_session->revisionSource()->documentKey() || + snapshot.documentRevision != m_preflight.documentRevision() || + snapshot.documentRevision != m_session->facade().currentRevision().toString() || + profile == m_preflightProfiles.cend() || !profile->valid || + profile->digest != m_preflight.profileDigest() || + snapshot.operationId != QStringLiteral("preflight.%1").arg(profile->id)) + { + m_preflight.markProfileStale(); + return; + } + switch (snapshot.status) { case pdf::PDFJobStatus::Succeeded: if (outcome) { + if (outcome->effectiveProfileDigest.isEmpty() || + outcome->result.effectiveProfileDigest != outcome->effectiveProfileDigest || + outcome->documentPath != m_session->facade().source().path) + { + m_preflight.failRun(snapshot.jobId, snapshot.documentRevision, + tr("Preflight result identity did not match the request.")); + break; + } + const pdf::PDFOperationHistoryStatus auditStatus = + pdf::reducePreflightVerdict(outcome->result).state == pdf::PreflightVerdictState::Error + ? pdf::PDFOperationHistoryStatus::Failed + : pdf::PDFOperationHistoryStatus::Accepted; + const pdf::PDFOperationResult auditResult = pdf::appendPreflightAuditRun( + outcome->documentPath, outcome->auditBytes, outcome->result, auditStatus, + QStringLiteral("LoopEditor"), outcome->auditSummary); + if (!auditResult) + { + m_preflight.failRun(snapshot.jobId, snapshot.documentRevision, auditResult.getErrorMessage()); + break; + } acceptPreflightResult(snapshot.jobId, snapshot.documentRevision, outcome->result); } else @@ -3254,6 +3278,21 @@ void EditorHost::finishActionListJob(const pdf::PDFJobSnapshot& snapshot) return; } + const pdfinteraction::ActionListRecipeEntry* recipe = m_actionListCatalog.recipe(m_selectedActionListRecipeId); + if (!hasDocument() || !m_session->revisionSource() || snapshot.kind != pdf::PDFJobKind::Other || + snapshot.documentKey != m_actionListController.documentKey() || + snapshot.documentKey != m_session->revisionSource()->documentKey() || + snapshot.documentRevision != m_actionListController.documentRevision() || + snapshot.documentRevision != m_session->facade().currentRevision().toString() || + m_selectedActionListRecipeId != m_actionListController.recipeId() || + !recipe || !recipe->valid || + snapshot.operationId != QStringLiteral("action-list.%1").arg(recipe->actionList.id) || + snapshot.checkId != recipe->actionList.name) + { + m_actionListController.markRecipeStale(); + return; + } + switch (snapshot.status) { case pdf::PDFJobStatus::Succeeded: @@ -3263,6 +3302,13 @@ void EditorHost::finishActionListJob(const pdf::PDFJobSnapshot& snapshot) tr("Action List result was unavailable.")); break; } + if (state != pdfinteraction::ActionListController::State::Validating && + outcome->executionResult.recipeHash != recipe->recipeHash) + { + m_actionListController.failRun(snapshot.jobId, snapshot.documentRevision, + tr("Action List result did not match the recipe.")); + break; + } if (state == pdfinteraction::ActionListController::State::Validating) { if (m_acceptActionListResults) @@ -3286,9 +3332,14 @@ void EditorHost::finishActionListJob(const pdf::PDFJobSnapshot& snapshot) } else if (state == pdfinteraction::ActionListController::State::Running) { + if (!outcome->candidate) + { + m_actionListController.failRun(snapshot.jobId, snapshot.documentRevision, + tr("Action List produced no document.")); + break; + } if (m_acceptActionListResults && - m_actionListController.acceptExecution(snapshot.jobId, snapshot.documentRevision, outcome->executionResult) && - outcome->candidate) + m_actionListController.acceptExecution(snapshot.jobId, snapshot.documentRevision, outcome->executionResult)) { m_session->context().setDocument(outcome->candidate); m_preflight.markProfileStale(); diff --git a/LoopLibCore/sources/pdfdiff.cpp b/LoopLibCore/sources/pdfdiff.cpp index 8d9c0b9f3..cc8d4ce0d 100644 --- a/LoopLibCore/sources/pdfdiff.cpp +++ b/LoopLibCore/sources/pdfdiff.cpp @@ -170,7 +170,15 @@ void PDFDiff::start() { return; } - onComparationPerformed(snapshot.status == pdf::PDFJobStatus::Cancelled); + if (snapshot.status != pdf::PDFJobStatus::Succeeded) + { + m_result = PDFDiffResult(); + m_result.setResult(pdf::PDFOperationResult( + snapshot.errorMessage.isEmpty() + ? QStringLiteral("Comparison job did not complete.") + : snapshot.errorMessage)); + } + onComparationPerformed(snapshot.status != pdf::PDFJobStatus::Succeeded); }); } else @@ -192,6 +200,8 @@ void PDFDiff::stop() m_cancelled = true; pdf::PDFJobScheduler::global().cancel(jobId); pdf::PDFJobScheduler::global().waitForFinished(jobId); + m_result = PDFDiffResult(); + m_result.setResult(pdf::PDFOperationResult(QStringLiteral("Comparison cancelled."))); m_activeJobId.clear(); if (m_jobFinishedConnection) { diff --git a/LoopLibCore/sources/pdfjobscheduler.cpp b/LoopLibCore/sources/pdfjobscheduler.cpp index f5b05ffc3..0db6cbd2f 100644 --- a/LoopLibCore/sources/pdfjobscheduler.cpp +++ b/LoopLibCore/sources/pdfjobscheduler.cpp @@ -163,6 +163,7 @@ void PDFJobContext::setOutputArtifact(PDFArtifactIdentity artifact) struct PDFJobScheduler::JobEntry { PDFJobSpec spec; + quint64 revisionEpoch = 0; PDFJobWork work; PDFJobCancellationTokenPtr cancellationToken; quint64 sequence = 0; @@ -275,6 +276,13 @@ QString PDFJobScheduler::submit(PDFJobSpec spec, } job->sequence = ++m_sequence; + const QString documentKey = resolvedDocumentKey(job->spec); + const auto revision = m_currentRevisions.find(documentKey); + if (!documentKey.isEmpty() && revision != m_currentRevisions.end() && + revision->second.revision == job->spec.documentRevision) + { + job->revisionEpoch = revision->second.epoch; + } job->queueDepth = static_cast(m_queue.size()); m_jobs.emplace(job->spec.jobId, job); m_queue.push(job); @@ -408,7 +416,16 @@ void PDFJobScheduler::setCurrentRevision(QString documentKey, QString documentRe return; } std::lock_guard lock(m_mutex); - m_currentRevisions[std::move(documentKey)] = std::move(documentRevision); + if (documentRevision.isEmpty()) + { + m_currentRevisions.erase(documentKey); + return; + } + auto& current = m_currentRevisions[std::move(documentKey)]; + if (current.revision != documentRevision) + { + current = CurrentRevision{ std::move(documentRevision), ++m_sequence }; + } } void PDFJobScheduler::clearCurrentRevision(const QString& documentKey) @@ -488,7 +505,12 @@ void PDFJobScheduler::workerLoop() Q_EMIT jobStarted(startedSnapshot); - if (isStale(job->spec) && job->spec.staleResultPolicy == PDFJobStaleResultPolicy::Discard) + bool staleBeforeWork = false; + { + std::lock_guard lock(m_mutex); + staleBeforeWork = isStaleLocked(*job); + } + if (staleBeforeWork && job->spec.staleResultPolicy == PDFJobStaleResultPolicy::Discard) { finishJob(job, PDFJobStatus::Stale, QStringLiteral("Document revision is no longer current.")); continue; @@ -542,10 +564,6 @@ void PDFJobScheduler::workerLoop() { finishJob(job, PDFJobStatus::Failed, std::move(errorMessage)); } - else if (isStale(job->spec) && job->spec.staleResultPolicy == PDFJobStaleResultPolicy::Discard) - { - finishJob(job, PDFJobStatus::Stale, QStringLiteral("Document revision changed while the job was running.")); - } else { finishJob(job, PDFJobStatus::Succeeded); @@ -565,6 +583,18 @@ void PDFJobScheduler::finishJob(const std::shared_ptr& job, return; } + if (status == PDFJobStatus::Succeeded && job->cancellationToken->isCancellationRequested()) + { + status = PDFJobStatus::Cancelled; + errorMessage = QStringLiteral("Cancellation requested during execution."); + } + else if (status == PDFJobStatus::Succeeded && + job->spec.staleResultPolicy == PDFJobStaleResultPolicy::Discard && isStaleLocked(*job)) + { + status = PDFJobStatus::Stale; + errorMessage = QStringLiteral("Document revision changed while the job was running."); + } + if (job->slotAcquired) { job->slotAcquired = false; @@ -575,6 +605,11 @@ void PDFJobScheduler::finishJob(const std::shared_ptr& job, } job->status = status; job->errorMessage = std::move(errorMessage); + if (status != PDFJobStatus::Succeeded) + { + job->resultSummary.clear(); + job->outputArtifact = {}; + } job->finishedAtUtc = QDateTime::currentDateTimeUtc(); if (job->startedAtUtc.isValid()) { @@ -660,16 +695,16 @@ void PDFJobScheduler::appendTrace(const std::shared_ptr& job, } } -bool PDFJobScheduler::isStale(const PDFJobSpec& spec) const +bool PDFJobScheduler::isStaleLocked(const JobEntry& job) const { - const QString key = resolvedDocumentKey(spec); + const QString key = resolvedDocumentKey(job.spec); if (key.isEmpty()) { return false; } - std::lock_guard lock(m_mutex); const auto it = m_currentRevisions.find(key); - return it != m_currentRevisions.end() && it->second != spec.documentRevision; + return it == m_currentRevisions.end() || job.revisionEpoch == 0 || + it->second.epoch != job.revisionEpoch || it->second.revision != job.spec.documentRevision; } PDFJobSnapshot PDFJobScheduler::snapshotLocked(const JobEntry& job) const diff --git a/LoopLibCore/sources/pdfjobscheduler.h b/LoopLibCore/sources/pdfjobscheduler.h index 12a6d9f5e..90aef18e7 100644 --- a/LoopLibCore/sources/pdfjobscheduler.h +++ b/LoopLibCore/sources/pdfjobscheduler.h @@ -39,7 +39,6 @@ #include #include #include -#include #include #include #include @@ -241,6 +240,11 @@ class LOOPLIBCORESHARED_EXPORT PDFJobScheduler final : public QObject private: struct JobEntry; + struct CurrentRevision + { + QString revision; + quint64 epoch = 0; + }; struct JobCompare { bool operator()(const std::shared_ptr& left, @@ -251,7 +255,7 @@ class LOOPLIBCORESHARED_EXPORT PDFJobScheduler final : public QObject void ensureWorkersStarted(); void finishJob(const std::shared_ptr& job, PDFJobStatus status, QString errorMessage = {}); void appendTrace(const std::shared_ptr& job, PDFJobStatus status, qint64 elapsedMs = 0); - bool isStale(const PDFJobSpec& spec) const; + bool isStaleLocked(const JobEntry& job) const; PDFJobSnapshot snapshotLocked(const JobEntry& job) const; static QString resolvedDocumentKey(const PDFJobSpec& spec); @@ -264,7 +268,7 @@ class LOOPLIBCORESHARED_EXPORT PDFJobScheduler final : public QObject int m_activeBackgroundJobs = 0; std::priority_queue, std::vector>, JobCompare> m_queue; std::unordered_map, PDFJobStringHash> m_jobs; - std::unordered_map m_currentRevisions; + std::unordered_map m_currentRevisions; std::unordered_map, PDFJobStringHash> m_traces; std::vector m_workers; std::once_flag m_workersOnce; diff --git a/LoopLibInteraction/sources/documentfacade.cpp b/LoopLibInteraction/sources/documentfacade.cpp index 594368dd1..792924bfd 100644 --- a/LoopLibInteraction/sources/documentfacade.cpp +++ b/LoopLibInteraction/sources/documentfacade.cpp @@ -24,6 +24,7 @@ #include "pdfresourcebudget.h" +#include #include #include @@ -434,6 +435,24 @@ void DocumentFacade::admitLoadResult(CommandInvocationId invocation, return; } + const pdf::PDFJobSnapshot job = m_submitter->snapshot(m_pendingJobId); + if (job.jobId == m_pendingJobId && + (job.status == pdf::PDFJobStatus::Queued || job.status == pdf::PDFJobStatus::Running)) + { + QTimer::singleShot(1, this, [this, invocation, generation, result = std::move(result)]() mutable + { admitLoadResult(invocation, generation, std::move(result)); }); + return; + } + if (job.jobId != m_pendingJobId || job.status != pdf::PDFJobStatus::Succeeded || + job.kind != pdf::PDFJobKind::Other) + { + result = {}; + result.outcome = job.status == pdf::PDFJobStatus::Cancelled + ? DocumentLoadOutcome::Cancelled + : DocumentLoadOutcome::Failed; + result.typedError = QStringLiteral("document/job-not-admitted"); + } + m_pendingJobId.clear(); pdf::PDFDocumentContext* documentContext = context(); @@ -548,6 +567,24 @@ void DocumentFacade::admitWriteResult(CommandInvocationId invocation, return; } + const pdf::PDFJobSnapshot job = m_submitter->snapshot(m_pendingJobId); + if (job.jobId == m_pendingJobId && + (job.status == pdf::PDFJobStatus::Queued || job.status == pdf::PDFJobStatus::Running)) + { + QTimer::singleShot(1, this, [this, invocation, generation, target = std::move(target), result = std::move(result)]() mutable + { admitWriteResult(invocation, generation, std::move(target), std::move(result)); }); + return; + } + if (job.jobId != m_pendingJobId || job.status != pdf::PDFJobStatus::Succeeded || + job.kind != pdf::PDFJobKind::Export || m_publishedKey.isEmpty() || job.documentKey != m_publishedKey || + job.documentRevision != m_revisionSource.currentRevision().toString()) + { + result.outcome = job.status == pdf::PDFJobStatus::Cancelled + ? DocumentWriteOutcome::Cancelled + : DocumentWriteOutcome::Failed; + result.typedError = QStringLiteral("document/job-not-admitted"); + } + m_pendingJobId.clear(); switch (result.outcome) @@ -583,8 +620,7 @@ void DocumentFacade::detachDocument() { if (!m_publishedKey.isEmpty()) { - // A key with no entry is never stale, so this belongs at close and at - // replacement, not between submissions. + // Closing the fence also rejects any completion from this session. m_submitter->clearCurrentRevision(m_publishedKey); m_publishedKey.clear(); } diff --git a/LoopLibInteraction/sources/jobsubmitter.h b/LoopLibInteraction/sources/jobsubmitter.h index e191ad80b..b92e0ae6e 100644 --- a/LoopLibInteraction/sources/jobsubmitter.h +++ b/LoopLibInteraction/sources/jobsubmitter.h @@ -64,8 +64,8 @@ class IJobSubmitter virtual void publishCurrentRevision(const QString& documentKey, const pdf::PDFRevisionIdentity& revision) = 0; - /// Drops the fence entry for a document key. A key with no entry is never - /// stale, so this belongs at document close, not between submissions. + /// Drops the fence entry for a document key. Bound jobs then become stale, + /// including if the same revision is published after a reopen. virtual void clearCurrentRevision(const QString& documentKey) = 0; }; diff --git a/LoopLibInteraction/sources/pagesurfacecoordinator.cpp b/LoopLibInteraction/sources/pagesurfacecoordinator.cpp index 2e450991a..b99b1f9da 100644 --- a/LoopLibInteraction/sources/pagesurfacecoordinator.cpp +++ b/LoopLibInteraction/sources/pagesurfacecoordinator.cpp @@ -24,6 +24,7 @@ #include "pdfpagecachebudget.h" +#include #include #include @@ -93,7 +94,18 @@ PageSurfaceCoordinator::~PageSurfaceCoordinator() void PageSurfaceCoordinator::setDocumentKey(QString documentKey) { + if (m_documentKey == documentKey) + { + return; + } + const bool hadDocumentKey = !m_documentKey.isEmpty(); + cancelInFlight(); + clearCache(); m_documentKey = std::move(documentKey); + if (hadDocumentKey || m_initialSnapshotPrimed) + { + rebuildSnapshot(); + } } void PageSurfaceCoordinator::setResourceBudget(std::shared_ptr budget) @@ -627,6 +639,33 @@ void PageSurfaceCoordinator::admit(quint64 requestId, std::shared_ptr resourceReservation) { const auto inFlight = m_inFlight.find(requestId); + if (inFlight != m_inFlight.end()) + { + const pdf::PDFJobSnapshot job = m_submitter->snapshot(inFlight->jobId); + if (job.jobId == inFlight->jobId && + (job.status == pdf::PDFJobStatus::Queued || job.status == pdf::PDFJobStatus::Running)) + { + QTimer::singleShot(1, this, [this, requestId, result = std::move(result), resourceReservation = std::move(resourceReservation)]() mutable + { admit(requestId, std::move(result), std::move(resourceReservation)); }); + return; + } + if (job.jobId != inFlight->jobId || job.status != pdf::PDFJobStatus::Succeeded) + { + const SurfaceTerminalState terminal = job.status == pdf::PDFJobStatus::Cancelled + ? SurfaceTerminalState::Cancelled + : job.status == pdf::PDFJobStatus::Stale + ? SurfaceTerminalState::Stale + : SurfaceTerminalState::Failed; + finishInFlight(requestId, terminal); + return; + } + if (!(result.key == inFlight->key) || !(result.token == inFlight->token)) + { + ++m_counters.rejectedSuperseded; + finishInFlight(requestId, SurfaceTerminalState::Stale); + return; + } + } if (!resourceReservation && inFlight != m_inFlight.end()) { resourceReservation = inFlight->resourceReservation; diff --git a/UnitTests/tst_documentfacadetest.cpp b/UnitTests/tst_documentfacadetest.cpp index 8d8a71f80..06d2dc060 100644 --- a/UnitTests/tst_documentfacadetest.cpp +++ b/UnitTests/tst_documentfacadetest.cpp @@ -74,6 +74,7 @@ class FakeJobSubmitter final : public pdfinteraction::IJobSubmitter spec.jobId.isEmpty() ? QStringLiteral("job-%1").arg(++m_sequence) : spec.jobId; submittedSpecs.append(spec); + m_specs.insert(jobId, spec); m_status.insert(jobId, pdf::PDFJobStatus::Queued); if (runInline) @@ -116,6 +117,9 @@ class FakeJobSubmitter final : public pdfinteraction::IJobSubmitter pdf::PDFJobSnapshot result; result.jobId = jobId; result.status = m_status.value(jobId, pdf::PDFJobStatus::Succeeded); + result.kind = m_specs.value(jobId).kind; + result.documentKey = m_specs.value(jobId).documentKey; + result.documentRevision = m_specs.value(jobId).documentRevision; return result; } @@ -150,6 +154,7 @@ class FakeJobSubmitter final : public pdfinteraction::IJobSubmitter bool runInline = true; bool cancelStopsQueuedWork = true; + pdf::PDFJobStatus terminalStatus = pdf::PDFJobStatus::Succeeded; QList submittedSpecs; QStringList cancelledJobs; QStringList clearedKeys; @@ -170,11 +175,12 @@ class FakeJobSubmitter final : public pdfinteraction::IJobSubmitter pdf::PDFProcessingLimits::conservativeDefaults(), [](int) {}); work(context); - m_status.insert(jobId, pdf::PDFJobStatus::Succeeded); + m_status.insert(jobId, terminalStatus); } quint64 m_sequence = 0; QHash m_status; + QHash m_specs; QHash m_deferred; }; @@ -274,6 +280,7 @@ private slots: void disabledCommandIsUnavailable(); void openAdmitsDocumentAndPublishesRevision(); + void workerLoadCannotPublishAfterSchedulerFailure(); void openFailureReportsTypedErrorAndBindsNoDocument(); void openCancellationIsTerminalAndNotSuccess(); void cancellingAQueuedOpenIsTerminal(); @@ -524,6 +531,17 @@ void DocumentFacadeTest::openAdmitsDocumentAndPublishesRevision() QVERIFY(harness.catalog.isEnabled(pdfinteraction::DocumentFacade::SaveCommandId)); } +void DocumentFacadeTest::workerLoadCannotPublishAfterSchedulerFailure() +{ + Harness harness; + harness.submitter.terminalStatus = pdf::PDFJobStatus::Failed; + harness.facade->open(QStringLiteral("/corpus/report.pdf")); + + QTRY_COMPARE(harness.facade->state(), pdfinteraction::DocumentState::Error); + QCOMPARE(harness.context.getDocument(), nullptr); + QCOMPARE(harness.facade->typedError(), QStringLiteral("document/job-not-admitted")); +} + void DocumentFacadeTest::openFailureReportsTypedErrorAndBindsNoDocument() { Harness harness; diff --git a/UnitTests/tst_jobschedulertest.cpp b/UnitTests/tst_jobschedulertest.cpp index 9358b9f09..80982daf8 100644 --- a/UnitTests/tst_jobschedulertest.cpp +++ b/UnitTests/tst_jobschedulertest.cpp @@ -44,6 +44,8 @@ private slots: void allWorkKindsUseOneSubmissionApi(); void cancellationIsTerminalAndMeasured(); void staleRevisionIsDiscardedBeforeWorkRuns(); + void missingFenceDoesNotRunDocumentWork(); + void supersededAndReopenedJobsNeverSucceed(); void progressAndOperationMetadataAreObservable(); void waitTimeoutCancelJoinsBeforeTerminalSnapshot(); void cancelledPreflightAndExportJobsAreNotSuccess(); @@ -267,9 +269,59 @@ void JobSchedulerTest::staleRevisionIsDiscardedBeforeWorkRuns() QVERIFY(!ran.load(std::memory_order_acquire)); } +void JobSchedulerTest::missingFenceDoesNotRunDocumentWork() +{ + pdf::PDFJobScheduler scheduler(1); + std::atomic_bool ran = false; + pdf::PDFJobSpec spec; + spec.documentKey = QStringLiteral("document-1"); + spec.documentRevision = QStringLiteral("revision-1"); + const QString jobId = scheduler.submit(spec, [&ran](pdf::PDFJobContext&) + { ran = true; }); + + QVERIFY(scheduler.waitForFinished(jobId, 1000)); + QCOMPARE(scheduler.snapshot(jobId).status, pdf::PDFJobStatus::Stale); + QVERIFY(!ran.load(std::memory_order_acquire)); +} + +void JobSchedulerTest::supersededAndReopenedJobsNeverSucceed() +{ + pdf::PDFJobScheduler scheduler(2); + const QString key = QStringLiteral("document-1"); + const QString revision = QStringLiteral("revision-1"); + scheduler.setCurrentRevision(key, revision); + + std::atomic_bool releaseOld = false; + std::atomic_bool oldStarted = false; + pdf::PDFJobSpec spec; + spec.documentKey = key; + spec.documentRevision = revision; + spec.priority = pdf::PDFJobPriority::VisiblePage; + const QString oldId = scheduler.submit(spec, [&releaseOld, &oldStarted](pdf::PDFJobContext&) + { + oldStarted = true; + while (!releaseOld.load(std::memory_order_acquire)) + { + std::this_thread::yield(); + } }); + QTRY_VERIFY_WITH_TIMEOUT(oldStarted.load(std::memory_order_acquire), 1000); + QVERIFY(!scheduler.waitForFinished(oldId, 10)); + + scheduler.clearCurrentRevision(key); + scheduler.setCurrentRevision(key, revision); + const QString retryId = scheduler.submit(spec, [](pdf::PDFJobContext&) {}); + QVERIFY(scheduler.waitForFinished(retryId, 1000)); + QCOMPARE(scheduler.snapshot(retryId).status, pdf::PDFJobStatus::Succeeded); + + releaseOld = true; + QVERIFY(scheduler.waitForFinished(oldId, 1000)); + QCOMPARE(scheduler.snapshot(oldId).status, pdf::PDFJobStatus::Stale); +} + void JobSchedulerTest::progressAndOperationMetadataAreObservable() { pdf::PDFJobScheduler scheduler(1); + scheduler.setCurrentRevision(QStringLiteral("document-2"), QStringLiteral("revision-4")); pdf::PDFJobSpec spec; spec.jobId = QStringLiteral("render-tile"); spec.kind = pdf::PDFJobKind::Rendering; @@ -367,6 +419,7 @@ void JobSchedulerTest::cancelledPreflightAndExportJobsAreNotSuccess() void JobSchedulerTest::test_finishedJobReleasesItsWorkClosure() { pdf::PDFJobScheduler scheduler(1); + scheduler.setCurrentRevision(QStringLiteral("doc-1"), QStringLiteral("1")); pdf::PDFJobSpec spec; spec.kind = pdf::PDFJobKind::Preflight; @@ -393,6 +446,7 @@ void JobSchedulerTest::test_finishedJobReleasesItsWorkClosure() void JobSchedulerTest::test_terminalJobRetentionIsBounded() { pdf::PDFJobScheduler scheduler(1); + scheduler.setCurrentRevision(QStringLiteral("doc-1"), QStringLiteral("1")); QList jobIds; for (int index = 0; index < 300; ++index) diff --git a/UnitTests/tst_pagesurfacetest.cpp b/UnitTests/tst_pagesurfacetest.cpp index 04a8088d0..e5fcfac7c 100644 --- a/UnitTests/tst_pagesurfacetest.cpp +++ b/UnitTests/tst_pagesurfacetest.cpp @@ -245,6 +245,7 @@ class FakeJobSubmitter final : public pdfinteraction::IJobSubmitter bool runInline = true; bool refuseSubmission = false; bool cancelStopsQueuedWork = true; + pdf::PDFJobStatus terminalStatus = pdf::PDFJobStatus::Succeeded; QList submittedSpecs; QStringList cancelledJobs; QHash publishedRevisions; @@ -262,7 +263,7 @@ class FakeJobSubmitter final : public pdfinteraction::IJobSubmitter pdf::PDFJobContext context(token, pdf::PDFProcessingLimits::conservativeDefaults(), [](int) {}); work(context); - m_status.insert(jobId, pdf::PDFJobStatus::Succeeded); + m_status.insert(jobId, terminalStatus); } quint64 m_sequence = 0; @@ -288,6 +289,10 @@ class FakePageSurfaceRenderer final : public pdfinteraction::IPageSurfaceRendere pdfinteraction::PageSurfaceResult result; result.key = request.key; result.token = request.token; + if (returnWrongPage) + { + ++result.key.pageIndex; + } if (jobContext.isCancellationRequested()) { @@ -317,6 +322,7 @@ class FakePageSurfaceRenderer final : public pdfinteraction::IPageSurfaceRendere int renderCount = 0; int shedCount = 0; bool reentered = false; + bool returnWrongPage = false; QList renderedKeys; pdfinteraction::SurfaceTerminalState nextState = pdfinteraction::SurfaceTerminalState::Complete; @@ -345,7 +351,10 @@ private slots: void supersededDemandIsCancelledBeforeNewWorkIsSubmitted(); void completionForASupersededRequestIsRejected(); void completionAgainstAnOldRevisionIsRejected(); + void workerSuccessWithWrongRequestIdentityIsRejected(); + void schedulerFailureCannotAdmitRenderedPixels(); void revisionReplacementDropsEveryStaleSurface(); + void documentKeyChangeDropsAdmittedSurfaces(); void cancellationIsTerminalAndNotSuccess(); void completionAfterDestructionReachesNobody(); void budgetExhaustionIsItsOwnTerminalState(); @@ -664,6 +673,30 @@ void PageSurfaceTest::completionAgainstAnOldRevisionIsRejected() QVERIFY(fixture.coordinator->snapshot().tiles.isEmpty()); } +void PageSurfaceTest::workerSuccessWithWrongRequestIdentityIsRejected() +{ + Fixture fixture; + fixture.renderer.returnWrongPage = true; + fixture.coordinator->requestSurfaces(); + Fixture::drain(); + + QCOMPARE(fixture.coordinator->counters().admitted, 0); + QVERIFY(fixture.coordinator->counters().rejectedSuperseded > 0); + QVERIFY(fixture.coordinator->snapshot().tiles.isEmpty()); +} + +void PageSurfaceTest::schedulerFailureCannotAdmitRenderedPixels() +{ + Fixture fixture; + fixture.submitter.terminalStatus = pdf::PDFJobStatus::Failed; + fixture.coordinator->requestSurfaces(); + Fixture::drain(); + + QCOMPARE(fixture.coordinator->counters().admitted, 0); + QVERIFY(fixture.coordinator->counters().failed > 0); + QVERIFY(fixture.coordinator->snapshot().tiles.isEmpty()); +} + void PageSurfaceTest::revisionReplacementDropsEveryStaleSurface() { Fixture fixture; @@ -681,6 +714,18 @@ void PageSurfaceTest::revisionReplacementDropsEveryStaleSurface() QVERIFY(fixture.coordinator->snapshot().tiles.isEmpty()); } +void PageSurfaceTest::documentKeyChangeDropsAdmittedSurfaces() +{ + Fixture fixture; + fixture.coordinator->requestSurfaces(); + Fixture::drain(); + QVERIFY(fixture.coordinator->counters().admittedBytes > 0); + + fixture.coordinator->setDocumentKey(QStringLiteral("doc-2")); + QCOMPARE(fixture.coordinator->counters().admittedBytes, qint64(0)); + QVERIFY(fixture.coordinator->snapshot().tiles.isEmpty()); +} + void PageSurfaceTest::cancellationIsTerminalAndNotSuccess() { Fixture fixture; diff --git a/UnitTests/tst_preflightinteraction.cpp b/UnitTests/tst_preflightinteraction.cpp index 3573efb5d..741cbb82b 100644 --- a/UnitTests/tst_preflightinteraction.cpp +++ b/UnitTests/tst_preflightinteraction.cpp @@ -311,6 +311,7 @@ void PreflightInteractionTest::controllerRepresentsIncompleteRun() void PreflightInteractionTest::controllerMarksAnInFlightRunStaleAndCancelsIt() { pdf::PDFJobScheduler scheduler(1); + scheduler.setCurrentRevision(QStringLiteral("doc"), QStringLiteral("rev-1")); PreflightController controller(&scheduler); std::atomic_bool started = false; diff --git a/UnitTests/tst_viewportcommandbridgetest.cpp b/UnitTests/tst_viewportcommandbridgetest.cpp index ab8840883..ca26d60af 100644 --- a/UnitTests/tst_viewportcommandbridgetest.cpp +++ b/UnitTests/tst_viewportcommandbridgetest.cpp @@ -67,6 +67,7 @@ class FakeJobSubmitter final : public pdfinteraction::IJobSubmitter { const QString jobId = spec.jobId.isEmpty() ? QStringLiteral("job-%1").arg(++m_sequence) : spec.jobId; + m_specs.insert(jobId, spec); m_status.insert(jobId, pdf::PDFJobStatus::Queued); if (runInline) @@ -92,6 +93,9 @@ class FakeJobSubmitter final : public pdfinteraction::IJobSubmitter pdf::PDFJobSnapshot result; result.jobId = jobId; result.status = m_status.value(jobId, pdf::PDFJobStatus::Succeeded); + result.kind = m_specs.value(jobId).kind; + result.documentKey = m_specs.value(jobId).documentKey; + result.documentRevision = m_specs.value(jobId).documentRevision; return result; } @@ -111,6 +115,7 @@ class FakeJobSubmitter final : public pdfinteraction::IJobSubmitter private: quint64 m_sequence = 0; QHash m_status; + QHash m_specs; }; class FakeDocumentLoader final : public pdfinteraction::IDocumentLoader diff --git a/changes/codex-issue-17-fence-scheduled-results.evidence.yaml b/changes/codex-issue-17-fence-scheduled-results.evidence.yaml new file mode 100644 index 000000000..c8901ff92 --- /dev/null +++ b/changes/codex-issue-17-fence-scheduled-results.evidence.yaml @@ -0,0 +1,46 @@ +format_version: 1 +kind: evidence +claims: + - id: scheduled-result-fencing + evidence: + - unit:agent-policy:core + - unit:UnitTestsJobScheduler + - unit:UnitTestsRevisionStress + - architecture:agent-policy:core + - architecture:docs/generated/architecture-catalog.json + - security:scripts/ci/check_source_integrity.py + - differential:UnitTestsConversionOracle + - packaging:linux-build + - packaging:windows-build + - id: page-surface-admission + evidence: + - unit:agent-policy:interaction + - unit:UnitTestsPageSurface + - unit:UnitTestsPageSurfaceBudget + - architecture:agent-policy:interaction + - architecture:docs/generated/architecture-catalog.json + - security:scripts/ci/check_source_integrity.py + - packaging:linux-build + - packaging:windows-build + - id: editor-result-admission + evidence: + - unit:agent-policy:quick + - unit:UnitTestsEditorHost + - architecture:agent-policy:quick + - architecture:docs/generated/architecture-catalog.json + - security:scripts/ci/check_source_integrity.py + - packaging:linux-build + - packaging:windows-build + - id: preflight-controller-fence + evidence: + - unit:agent-policy:preflight + - unit:UnitTestsPreflightInteraction + - unit:UnitTestsPreflightVerdict + - integration:UnitTestsPreflightCorpus + - integration:UnitTestsPreflightWorkflowAcceptance + - differential:UnitTestsStandardOracle + - architecture:loop-preflight/testdata/fixtures + - architecture:docs/generated/preflight-check-catalog.json + - architecture:docs/generated/preflight-corpus-coverage.json +unresolved: + - core:scripts/ci/check_independent_validation_gate.py diff --git a/changes/codex-issue-17-fence-scheduled-results.md b/changes/codex-issue-17-fence-scheduled-results.md new file mode 100644 index 000000000..29b2f09df --- /dev/null +++ b/changes/codex-issue-17-fence-scheduled-results.md @@ -0,0 +1,4 @@ +Category: fixed +Audience: integrators +Breaking-Change: no +Summary: Fence scheduled document work across close and reopen, and admit rendered and Editor results only under their current request identities. From a0c160efb77395b316fb58ab3fd1237303871340 Mon Sep 17 00:00:00 2001 From: mbx30 <212453881+mberrys@users.noreply.github.com> Date: Fri, 25 Sep 2026 12:04:01 -0700 Subject: [PATCH 2/3] docs(evidence): bind issue 17 to declared proof lanes --- ...e-17-fence-scheduled-results.evidence.yaml | 22 +++++-------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/changes/codex-issue-17-fence-scheduled-results.evidence.yaml b/changes/codex-issue-17-fence-scheduled-results.evidence.yaml index c8901ff92..9a6cbfbe8 100644 --- a/changes/codex-issue-17-fence-scheduled-results.evidence.yaml +++ b/changes/codex-issue-17-fence-scheduled-results.evidence.yaml @@ -4,38 +4,26 @@ claims: - id: scheduled-result-fencing evidence: - unit:agent-policy:core - - unit:UnitTestsJobScheduler - - unit:UnitTestsRevisionStress - architecture:agent-policy:core - architecture:docs/generated/architecture-catalog.json - security:scripts/ci/check_source_integrity.py - differential:UnitTestsConversionOracle - - packaging:linux-build - - packaging:windows-build - id: page-surface-admission evidence: - unit:agent-policy:interaction - - unit:UnitTestsPageSurface - - unit:UnitTestsPageSurfaceBudget - - architecture:agent-policy:interaction - - architecture:docs/generated/architecture-catalog.json - - security:scripts/ci/check_source_integrity.py - - packaging:linux-build - - packaging:windows-build - id: editor-result-admission evidence: - unit:agent-policy:quick - - unit:UnitTestsEditorHost - - architecture:agent-policy:quick - - architecture:docs/generated/architecture-catalog.json - - security:scripts/ci/check_source_integrity.py - - packaging:linux-build - - packaging:windows-build - id: preflight-controller-fence evidence: - unit:agent-policy:preflight + - unit:UnitTestsPreflightChecks + - unit:UnitTestsPreflightEngine - unit:UnitTestsPreflightInteraction - unit:UnitTestsPreflightVerdict + - unit:UnitTestsPreflightProfileResolver + - unit:UnitTestsProfileIdentity + - unit:UnitTestsOperatorAcceptance - integration:UnitTestsPreflightCorpus - integration:UnitTestsPreflightWorkflowAcceptance - differential:UnitTestsStandardOracle From d297f8e5680ef71a7ba6c9c5e04a1040a337b258 Mon Sep 17 00:00:00 2001 From: mbx30 <212453881+mberrys@users.noreply.github.com> Date: Fri, 25 Sep 2026 12:29:45 -0700 Subject: [PATCH 3/3] test(interaction): bind cancellation fixture to published revision --- UnitTests/tst_interactionboundarytest.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/UnitTests/tst_interactionboundarytest.cpp b/UnitTests/tst_interactionboundarytest.cpp index 7ce729cab..1b89c1090 100644 --- a/UnitTests/tst_interactionboundarytest.cpp +++ b/UnitTests/tst_interactionboundarytest.cpp @@ -232,13 +232,17 @@ void InteractionBoundaryTest::submitterCancellationIsTerminalAndNotSuccess() { pdf::PDFJobScheduler scheduler(1); pdfinteraction::PDFJobSchedulerSubmitter submitter(scheduler); + pdf::PDFDocumentContext context(nullptr); + const QString documentKey = QStringLiteral("doc-under-test"); + submitter.publishCurrentRevision(documentKey, context.getRevision()); std::atomic_bool started = false; pdf::PDFJobSpec spec; spec.jobId = QStringLiteral("interaction-cancel-me"); spec.kind = pdf::PDFJobKind::Rendering; spec.priority = pdf::PDFJobPriority::Interaction; - spec.documentKey = QStringLiteral("doc-under-test"); + spec.documentKey = documentKey; + spec.documentRevision = context.getRevision().toString(); // Cancel a running job cooperatively, as UnitTestsJobScheduler does. The work // exits on the cancellation token rather than on a flag this slot must live