feat(measurement): Advanced Measurement v2 draft, discovery, execution and reporting - #916
Merged
Conversation
…n and reporting The services behind the v2 route surface. Slice 1 registered these paths and returned 501; they now answer for real. Draft authoring is a server-side draft per project, mutated only through typed actions guarded by If-Match. A missing precondition is 428 and a stale one is 412, and a stale action writes nothing at all. The ETag is a monotonic counter rather than a content hash, because a hash would silently accept the case where content changes and changes back. Publish runs in one transaction: verify the ETag, verify the expected active revision, recompile server-side, compare the compiled checksum against what was reviewed, write the immutable version, move the pointer, clear the draft. Any failure writes nothing and preserves the draft. Content identical to the active revision is a no-op; content identical to an older revision publishes a new revision, so reverting a plan is expressible. Discovery stays deterministic and review-only, and gains manual identity rebinding that preserves the stable Target key, its assignments and its group membership. The sitemap fetch is hardened against server-side request forgery: scheme allowlist, no embedded credentials, rejection of loopback, private, link-local, unique-local, CGNAT and reserved addresses including IPv6 site-local and zone-scoped forms, connection to the resolved and validated address so a second lookup cannot redirect the dial, every rule re-run on each redirect hop, a redirect cap, an announced-length pre-check, a streaming size cap and a total timeout. Execution materializes the frozen manifest and deduplicates by slot identity, so one question shared across two hundred Properties costs one provider call per provider rather than two hundred. Reporting is revision-scoped. A run named explicitly must be pinned to the active revision or the request is refused, which is how cross-revision joins are rejected; otherwise the latest completed run for that revision is used, and with none every metric is unavailable rather than zero. Denominators count unique expected slots, so Target reuse cannot inflate them. Named share of voice exists only for a group on a non-brand basket and uses one shared denominator. brandPresence is emitted alongside the existing sov, which is retained as a deprecated alias so the browser keeps working until it migrates. The execution graph is now checked for referential integrity when a revision is decoded. Dedup drops a colliding execution node, and a usage edge left pointing at the dropped key would have meant a Target silently absent from a sweep, with no error anywhere. A revision that could produce one no longer parses. Every behaviour is pinned by a test proven by reverting the production change, including each individual request-forgery rule, which were verified a second time by deleting each one and confirming a specific failure. Backend only; nothing under apps/web is touched.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+421
to
+430
| app.get<{ Params: { name: string }; Querystring: PageQuery }>('/projects/:name/measurement-plan/draft/targets', async request => { | ||
| const project = resolveProject(app.db, request.params.name) | ||
| const authoring = parseStoredAuthoring(requireDraft(project).authoringJson) | ||
| return paginate<MeasurementDraftTarget>( | ||
| authoring.targets, | ||
| request.query, | ||
| target => labelSortKey(target.label, target.stableKey), | ||
| target => `${target.label} ${target.stableKey}`, | ||
| ) | ||
| }) |
Comment on lines
+432
to
+441
| app.get<{ Params: { name: string }; Querystring: PageQuery }>('/projects/:name/measurement-plan/draft/assignments', async request => { | ||
| const project = resolveProject(app.db, request.params.name) | ||
| const authoring = parseStoredAuthoring(requireDraft(project).authoringJson) | ||
| return paginate<MeasurementDraftAssignment>( | ||
| authoring.assignments, | ||
| request.query, | ||
| assignment => `${assignment.targetKey} ${assignment.queryId}`, | ||
| assignment => `${assignment.targetKey} ${assignment.queryId}`, | ||
| ) | ||
| }) |
Comment on lines
+443
to
+452
| app.get<{ Params: { name: string }; Querystring: PageQuery }>('/projects/:name/measurement-plan/draft/groups', async request => { | ||
| const project = resolveProject(app.db, request.params.name) | ||
| const authoring = parseStoredAuthoring(requireDraft(project).authoringJson) | ||
| return paginate<MeasurementDraftGroup>( | ||
| authoring.groups, | ||
| request.query, | ||
| group => labelSortKey(group.label, group.stableKey), | ||
| group => `${group.label} ${group.stableKey}`, | ||
| ) | ||
| }) |
Comment on lines
+461
to
+509
| app.post<{ Params: { name: string } }>('/projects/:name/measurement-plan/draft/actions/create', async (request, reply) => { | ||
| const gate = beginMutation(request, reply, 'create') | ||
| if (gate.replay !== null) return gate.replay | ||
| const parsed = measurementDraftCreateRequestSchema.safeParse(request.body) | ||
| if (!parsed.success) throw validationError('Invalid "create" payload', { issues: parsed.error.issues }) | ||
|
|
||
| app.get<{ Params: { name: string } }>('/projects/:name/measurement-plan/draft/assignments', async () => pending('draft assignment paging')) | ||
| // At most one draft per project, and a second create is a conflict rather | ||
| // than a silent reset: the first one holds unreviewed operator work. | ||
| if (draftRow(app.db, gate.project.id)) throw alreadyExists('Measurement plan draft', gate.project.name) | ||
|
|
||
| app.get<{ Params: { name: string } }>('/projects/:name/measurement-plan/draft/groups', async () => pending('draft group paging')) | ||
| const active = activePlanVersionRow(app.db, gate.project.id) | ||
| if ((active?.revision ?? null) !== parsed.data.expectedActiveRevision) { | ||
| throw measurementPlanRevisionConflict(parsed.data.expectedActiveRevision, active?.revision ?? null) | ||
| } | ||
| const authoring = seedAuthoring( | ||
| gate.project, | ||
| active ? parseStoredMeasurementPlanAnyVersion(active.canonicalJson) : null, | ||
| actionContextFor(app.db, gate.project), | ||
| ) | ||
| const draftId = crypto.randomUUID() | ||
| const response = mutationResponse(1, true, [], authoring) | ||
| // Set only once the write has committed, so a failed mutation never comes | ||
| // back carrying an ETag the caller could act on. | ||
| const settled = finishMutation(gate, response, (tx, now) => { | ||
| tx.insert(measurementPlanDrafts).values({ | ||
| id: draftId, | ||
| projectId: gate.project.id, | ||
| schemaVersion: 2, | ||
| baseActiveVersionId: active?.id ?? null, | ||
| baseActiveRevision: active?.revision ?? null, | ||
| authoringJson: JSON.stringify(authoring), | ||
| etagVersion: 1, | ||
| createdBy: serializeActor(gate.actor), | ||
| updatedBy: serializeActor(gate.actor), | ||
| createdAt: now.toISOString(), | ||
| updatedAt: now.toISOString(), | ||
| }).run() | ||
| writeAuditLog(tx, auditFromRequest(request, { | ||
| projectId: gate.project.id, | ||
| actor: 'api', | ||
| action: 'measurement-draft.created', | ||
| entityType: 'measurement-draft', | ||
| entityId: draftId, | ||
| diff: { baseActiveRevision: active?.revision ?? null, etag: response.etag }, | ||
| })) | ||
| }) | ||
| reply.header('etag', response.etag) | ||
| return settled | ||
| }) |
Comment on lines
+512
to
+545
| app.post<{ Params: { name: string } }>(`/projects/:name/measurement-plan/draft/actions/${action}`, async (request, reply) => { | ||
| const gate = beginMutation(request, reply, action) | ||
| if (gate.replay !== null) return gate.replay | ||
| const ifMatch = requireIfMatch(request) | ||
| const row = requireDraft(gate.project) | ||
| assertDraftEtag(row, ifMatch) | ||
|
|
||
| const before = parseStoredAuthoring(row.authoringJson) | ||
| const result = applyDraftAction(action, before, request.body, actionContextFor(app.db, gate.project)) | ||
| // A no-op leaves the counter alone: the ETag must change after every | ||
| // successful MUTATION, and nothing was mutated. | ||
| const changed = authoringIdentity(result.authoring) !== authoringIdentity(before) | ||
| const etagVersion = changed ? row.etagVersion + 1 : row.etagVersion | ||
| const response = mutationResponse(etagVersion, changed, result.warnings, result.authoring) | ||
| const settled = finishMutation(gate, response, (tx, now) => { | ||
| if (!changed) return | ||
| tx.update(measurementPlanDrafts).set({ | ||
| authoringJson: JSON.stringify(result.authoring), | ||
| etagVersion, | ||
| updatedBy: serializeActor(gate.actor), | ||
| updatedAt: now.toISOString(), | ||
| }).where(eq(measurementPlanDrafts.id, row.id)).run() | ||
| writeAuditLog(tx, auditFromRequest(request, { | ||
| projectId: gate.project.id, | ||
| actor: 'api', | ||
| action: `measurement-draft.${action}`, | ||
| entityType: 'measurement-draft', | ||
| entityId: row.id, | ||
| diff: { previousEtag: measurementDraftEtag(row.etagVersion), etag: response.etag }, | ||
| })) | ||
| }) | ||
| reply.header('etag', response.etag) | ||
| return settled | ||
| }) |
Comment on lines
+552
to
+565
| }, async request => { | ||
| requireScope(request, MEASUREMENT_PLAN_WRITE_SCOPE) | ||
| const project = resolveProject(app.db, request.params.name) | ||
| const authoring = parseStoredAuthoring(requireDraft(project).authoringJson) | ||
| const compiled = compileMeasurementDraft(authoring, compileContextFor(app.db, project)) | ||
| if (!compiled.ok) return { ok: false, compiledChecksum: null, checks: compiled.checks } | ||
| return { | ||
| ok: true, | ||
| compiledChecksum: compiled.plan.compiledChecksum, | ||
| checks: compiled.checks, | ||
| counts: draftCounts(authoring), | ||
| plan: compiled.plan, | ||
| } | ||
| }) |
Comment on lines
+569
to
+596
| }, async request => { | ||
| requireScope(request, MEASUREMENT_PLAN_WRITE_SCOPE) | ||
| const project = resolveProject(app.db, request.params.name) | ||
| const authoring = parseStoredAuthoring(requireDraft(project).authoringJson) | ||
| const compiled = compileMeasurementDraft(authoring, compileContextFor(app.db, project)) | ||
| if (!compiled.ok) return { ok: false, compiledChecksum: null, checks: compiled.checks, diff: null } | ||
| const active = activePlanVersionRow(app.db, project.id) | ||
| // A v1 revision has no assignment model, so a diff against it would be | ||
| // invented rather than computed. The candidate is reported whole and the | ||
| // check says why. | ||
| const comparable = active && active.schemaVersion === 2 ? parseV2Plan(active) : null | ||
| const checks = active && !comparable | ||
| ? [...compiled.checks, { | ||
| ruleId: 'active-revision-schema-v1', | ||
| severity: 'warn' as const, | ||
| message: `Active revision ${active.revision} is schema v1, which has no assignment model. Everything below reads as added.`, | ||
| path: [], | ||
| }] | ||
| : compiled.checks | ||
| return { | ||
| ok: true, | ||
| compiledChecksum: compiled.plan.compiledChecksum, | ||
| checks, | ||
| counts: draftCounts(authoring), | ||
| plan: compiled.plan, | ||
| diff: diffCompiledPlans(comparable, compiled.plan, active?.revision ?? null), | ||
| } | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat(measurement): Advanced Measurement v2 draft, discovery, execution and reporting
The services behind the v2 route surface. Slice 1 registered these paths and
returned 501; they now answer for real.
Draft authoring is a server-side draft per project, mutated only through typed
actions guarded by If-Match. A missing precondition is 428 and a stale one is
412, and a stale action writes nothing at all. The ETag is a monotonic counter
rather than a content hash, because a hash would silently accept the case where
content changes and changes back. Publish runs in one transaction: verify the
ETag, verify the expected active revision, recompile server-side, compare the
compiled checksum against what was reviewed, write the immutable version, move
the pointer, clear the draft. Any failure writes nothing and preserves the draft.
Content identical to the active revision is a no-op; content identical to an
older revision publishes a new revision, so reverting a plan is expressible.
Discovery stays deterministic and review-only, and gains manual identity
rebinding that preserves the stable Target key, its assignments and its group
membership. The sitemap fetch is hardened against server-side request forgery:
scheme allowlist, no embedded credentials, rejection of loopback, private,
link-local, unique-local, CGNAT and reserved addresses including IPv6 site-local
and zone-scoped forms, connection to the resolved and validated address so a
second lookup cannot redirect the dial, every rule re-run on each redirect hop, a
redirect cap, an announced-length pre-check, a streaming size cap and a total
timeout.
Execution materializes the frozen manifest and deduplicates by slot identity, so
one question shared across two hundred Properties costs one provider call per
provider rather than two hundred.
Reporting is revision-scoped. A run named explicitly must be pinned to the active
revision or the request is refused, which is how cross-revision joins are
rejected; otherwise the latest completed run for that revision is used, and with
none every metric is unavailable rather than zero. Denominators count unique
expected slots, so Target reuse cannot inflate them. Named share of voice exists
only for a group on a non-brand basket and uses one shared denominator.
brandPresence is emitted alongside the existing sov, which is retained as a
deprecated alias so the browser keeps working until it migrates.
The execution graph is now checked for referential integrity when a revision is
decoded. Dedup drops a colliding execution node, and a usage edge left pointing
at the dropped key would have meant a Target silently absent from a sweep, with
no error anywhere. A revision that could produce one no longer parses.
Every behaviour is pinned by a test proven by reverting the production change,
including each individual request-forgery rule, which were verified a second time
by deleting each one and confirming a specific failure.
Backend only; nothing under apps/web is touched.