Skip to content

fix: review follow-ups from the beta promotion #2643 (JSONL batches, owningApp, 413 spec) - #2707

Merged
WilcoLouwerse merged 4 commits into
developmentfrom
fix/jsonl-batch-followups
Oct 22, 2026
Merged

WilcoLouwerse merged 4 commits into
developmentfrom
fix/jsonl-batch-followups

Conversation

@WilcoLouwerse

@WilcoLouwerse WilcoLouwerse commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Follow-ups from the review of the beta promotion #2643: Robert's five 🟡 and five 🟢 on the bounded JSONL sync (#2690), the owningApp note from my review, and the missing 413 in the spec. They go to the next beta through the next development → beta promotion.

What this fixes

Finding Commit Change
🟡 A failed download on batch 2 or later reschedules the job every 5 minutes 0ecedcc6b resume is reported only when failureReason === 'batch_limit_reached'. A failed batch keeps its cursor and waits for the job's interval.
🟡 A format: "jsonl" synchronization no longer refuses a .tar.gz endpoint 0ecedcc6b fetchJsonLinesBatch() checks isTarGzEndpoint() before downloading, logs the same warning and returns an incomplete empty batch that keeps the cursor.
🟡 The disappearance pass never runs for a JSONL file larger than one batch 0ecedcc6b Option (a): REQ-006 says a JSONL source larger than one batch never runs the disappearance pass. A batch_limit_reached run no longer logs the fetch-incomplete warning or dispatches the deletion-guarded event; its guard reads batch_run.
🟡 The batch cursor is an offset into a file that is downloaded again every tick 0ecedcc6b The download's ETag (else Last-Modified with Content-Length) is kept in the new field currentPageVersion (synchronization schema 1.2.1, both registers). A batch that finds another version logs it and reads batch 1 of the new file in the same run. The same applies when a cycle resumes after a gap. A source that sends neither header cannot show a change, and its cursor is used as before.
🟡 An upgrade does not disable the ten external feeds 0ecedcc6b The ten job objects carry version 1.0.1, so the import writes isEnabled: false on an upgrade. This also overwrites an administrator's own edits to those ten jobs. The fragment test asserts the version, the native-data-gathering-provider spec has an upgrade scenario, and the bounded-jsonl-sync proposal says so.
🟢 Text that still describes the Austria job as enabled, single-request and whole-file 0ecedcc6b The job and synchronization descriptions, the $comment memory sentence, the test method name (…AndDisabledJob) and proposal.md are reworded.
🟢 The currentPage docblock contradicts the JSONL condition 0ecedcc6b The two orphaned docblocks are gone. persistPageCursor() names both conditions and keeps the measurement.
🟢 "MUST NOT hold the response body in memory" has a branch where it does 0ecedcc6b The sentence is scoped to streamed responses, in the spec and the proposal.
🟢 The scheduling tests cite the ingestion requirement 0ecedcc6b JobServiceTest and SynchronizationActionTest cite the job-scheduling requirement.
🟢 The live-pass task is still open 0ecedcc6b Answer: no, the live pass has not run. The task stays open and now says the batched Austria import is unverified on a live instance.
🟢 sourceApp is a free-text label; any app can answer for any source 79f64b107 A source can name its app in configuration.owningApp. The ZDS case client refuses a command from another app, and the provider receiver restricts the query event to that app before dispatching it (restrictToApp(), set once). Without owningApp, nothing changes.
#2694 heads-up: the 413 on the HMAC bridge is not in the spec e2e208d44 The stuf-message-admin-only delta of REQ-020 states the 413 above 2 MiB, with a scenario naming the existing test.

SynchronizationOutcomeWriteBackTest::testBothRegistersDeclareWriteBack pinned the synchronization schema at exactly 1.2.0; it now asserts at least 1.2.0.
00c2aa891 adds the two new schema strings (currentPageVersion title and description) to l10n/en.json and l10n/nl.json, for check:schema-l10n.

Related, not in this PR

Checks run on this branch

  • bash hydra/scripts/run-hydra-gates.sh . --scope-to-diff --base origin/development, with NODE_PATH pointing at an ajv install → 48 of 48 applicable gates ran and are green (49 not applicable to this diff).
  • phpunit -c phpunit-unit.xml --no-coverage, whole suite: 5,586 tests, 11 errors and 1 failure. That is the same list of 12 as on origin/development (0a678ee5d, 5,578 tests) in the same environment; the lists were diffed. The touched test files (SynchronizationServiceTest, …FetchCompletenessTest, Wave0GatheringConnectorRegisterFragmentTest, JobServiceTest, SynchronizationActionTest, StufZkn/*): OK.
  • Every new test fails with this PR's lib/ and fragment changes reverted, except testJsonlBatchOfTheSameFileVersionReadsOn and testTheOwningAppsCommandIsSent, which pin unchanged behaviour.
  • phpcs, phpstan and phpmd on the changed lib/ files: no errors. phpcs shows 9 warnings in SynchronizationService.php, the same 9 as on development.
  • Not run: the full composer check:strict, and a live cron pass with the Austria job.

🤖 Generated with Claude Code

WilcoLouwerse and others added 3 commits October 19, 2026 11:47
- A failed download on batch 2 or later keeps its batch and no longer
  reports `resume`, so its job waits for its own interval instead of calling
  the failing source again every five minutes.
- A `format: "jsonl"` synchronization refuses a `.tar`/`.tar.gz` endpoint
  before downloading, with the same warning as the non-JSONL path.
- A batch run (`batch_limit_reached`) no longer logs the fetch-incomplete
  warning or dispatches the deletion-guarded event once per batch; its guard
  reads `batch_run`. REQ-006 now says a JSONL source larger than one batch
  never runs the disappearance pass.
- A cycle is tied to one version of the file: the download's ETag (else
  Last-Modified with Content-Length) is kept in the new synchronization field
  `currentPageVersion` (schema 1.2.1). A later batch that finds another
  version starts the cycle again at batch 1 of the new file.
- The ten external gathering jobs carry version 1.0.1, so an upgrade
  disables a job an earlier version imported enabled; the fragment test
  asserts it and the spec has an upgrade scenario.
- Text that still described the Austria job as enabled, single-request and
  whole-file is updated, the orphaned currentPage docblock is folded into
  persistPageCursor(), the streaming sentence is scoped to streamed
  responses, and the scheduling tests cite the job-scheduling requirement.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ers from that app only

A StUF-ZKN source can name its app in `configuration.owningApp`. The ZDS
case client then refuses a case command whose `sourceApp` is another app,
and the provider receiver restricts the query event to that app before
dispatching it, so another app's answer or refusal does not count and a
listener cannot move the restriction. A source without `owningApp` behaves
as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/integriq @ 9647830

Check PHP Vue Security License Tests
lint ✅
phpcs ✅
phpmd ✅
psalm ✅
phpstan ✅
phpmetrics ✅
eslint ✅
stylelint ✅
build ✅
build ✅
check-specs ✅
test-l10n ✅
format ✅
check-schema-l10n ✅
check-l10n-js ✅
composer ✅ ✅ 143/143
npm ✅ ✅ 646/646
app:check-code ⏭️
info.xml ✅
REUSE ❌
lockfile sync ✅
PHPUnit ✅ 5565/5565 · 1 skipped
Newman ✅ 130/130
Playwright ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it locally with npx playwright test, or from the Actions tab on a branch with no open pull request into development.
Hydra gates ✅
Hydra gates (axe) ✅

Quality workflow — 2026-10-19 10:30 UTC

Download the full PDF report from the workflow artifacts.

@WilcoLouwerse
WilcoLouwerse merged commit 78f2e08 into development Oct 22, 2026
48 checks passed
@WilcoLouwerse
WilcoLouwerse deleted the fix/jsonl-batch-followups branch October 22, 2026 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant