Skip to content

publishPackageDrafts' pre-flight refusal writes no audit row — a package publish rejected before the transaction leaves no trail #8595

Description

@os-zhuang

What

publishPackageDrafts has a pre-flight gate that refuses the whole batch before anything
is promoted — currently the ADR-0028 namespace-prefix rule on object drafts:

if (preflightViolations.length > 0) {
    return {
        success: false,
        publishedCount: 0,
        failedCount: preflightViolations.length,
        published: [],
        failed: preflightViolations,
    };
}

That return is above the engine.transaction(), so it is reached by neither the
allowed-outcome rows nor the rollback handler's denied row. A publish refused here writes
nothing to sys_metadata_audit at all: in the trail, an author whose package was rejected
for a bad object name is indistinguishable from an author who never pressed Publish.

Why #8400 did not cover it

#8400's ruling placed the batch's audit rows in Phase 2, after the transaction commits,
driven off promoted[], and its denied row in the rollback handler. Both live below this
early return. The card named the rollback refusal specifically, so the pre-flight branch was
left alone rather than folded in silently — it is a separate refusal class with its own
shape (N violations, no causal item, nothing ever attempted).

Shape question for whoever takes this

The pre-flight refusal is not "one causal item aborted the batch" — it is a list of
violations, each naming a different item. So it is genuinely a different row shape from
batch_aborted:

  1. one denied row per violation, keyed on its own (type, name), or
  2. one row for the batch, which then has no honest (type, name) to key on.

Option 1 matches what the audit table is for (per-item decisions, and the per-item history
tab reads (type, name, occurred_at)); option 2 needs a synthetic identity in a compliance
ledger, which #8400 deliberately avoided for the unattributable case.

Backlink: #8400, #7748.

Generated by Claude Code

Activity

  1. added theissue type on Aug 14, 2026
  2. hotlong commented on Aug 14, 2026

    @hotlong
    Contributor

    Triage: lands in packages/metadata-protocol (publishPackageDrafts pre-flight branch, above the transaction); rationale: same #7748 class as #8594 but a distinct refusal site — a batch refused pre-flight writes nothing at all. Queued as Bug.


    Generated by Claude Code

  3. self-assigned this
    on Aug 14, 2026
  4. hotlong commented on Aug 14, 2026

    @hotlong
    Contributor

    Claim: PM loop round 2
    Session: session_01XeQRiAa7vYRVX5Fog7Zby8
    Branch: claude/issue-8595-preflight-refusal-audit-row
    Worktree: objectstack-issue-8595
    Domain: domain:metadata
    File surface: packages/metadata-protocol/src/protocol.ts — the publishPackageDrafts pre-flight branch (preflightViolations at :13036/:13041, the early return at :13083) — plus packages/metadata-protocol/src/protocol.package-publish-audit-rows.test.ts (stop on breach; explain in the report)
    Container & model: M, mode:subagent, model: opus
    Serial constraints cleared — all three of triage's constraints are now discharged, verified individually:

    Open-PR list re-read at claim time rather than reused from this round's earlier snapshot — that snapshot went stale mid-round and I am not repeating it. No open PR touches metadata-protocol.

    Dispatching under the maintainer's direct-dispatch instruction (2026-08-14, verbatim, untranslated): 「metadata + engine-core + drivers 优先处理bug」, 「并发加到5」.

    Carrying triage's framing intact, including that its shape preference is explicitly a falsifiable mechanism assumption and not a ruling: option 1 (one denied row per violation, keyed on its own (type, name)) is the presumptive route because it matches what the per-item history tab reads and avoids the synthetic identity #8400 deliberately declined. The dev is instructed to verify it against the actual reader and report a fork rather than picking option 2 silently.


    Generated by Claude Code

  5. hotlong commented on Aug 14, 2026

    @hotlong
    Contributor
    {
      "issue": 8595,
      "status": "done",
      "branch": "claude/issue-8595-preflight-refusal-audit-row",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/8746",
      "premise_still_valid": true,
      "summary": "The premise held exactly as described: preflightViolations declared/populated with code 'NAMESPACE_PREFIX' and the early return carrying failedCount, all above engine.transaction(), so neither #8400's post-commit rows nor #8594's rollback-handler verdicts could reach it. The pre-flight branch now writes one publish/denied audit row per violation before returning, keyed on the offending draft's own (type, name) and on the draft's own org scope (env-wide stays env-wide even when the publishing session carries an active org). The row's code carries the violated rule ('namespace_prefix') rather than a fixed value, via PREFLIGHT_AUDIT_CODE, a Record keyed by a PreflightViolationCode union so a future pre-flight gate cannot reach the column without failing to compile until its spelling is declared — the closed-set discipline batch_aborted asked for, enforced by the compiler instead of by a mechanical toLowerCase or an unreachable runtime fallback. One file beyond the declared surface: packages/metadata-core/src/objects/sys-metadata-audit.object.ts, whose TSDoc IS the code column's documented closed set — adding a value without declaring it there is the exact silent drift the batch_aborted note warns about. SHAPE QUESTION RESOLVED IN FAVOUR OF OPTION 1, VERIFIED AGAINST THE ACTUAL READER, NOT ASSUMED: auditMetaItem (protocol.ts:6103) queries exactly where { type, name } with orderBy occurred_at desc, and rest-server.ts:6386 serves Studio's audit-log tab from it via GET /api/v1/meta/:type/:name/audit. The reader agreed with triage's preference, so there is no fork to adjudicate; option 2 would have needed the synthetic identity #8400 declined.",
      "tests": "All quoted runs are at HEAD b2badd9 (final commit, clean tree; git status --porcelain empty). TESTS: `pnpm --filter @objectstack/metadata-protocol --filter @objectstack/metadata-core test` -> REAL_EXIT=0; metadata-protocol 'Test Files 90 passed (90) / Tests 1351 passed (1351)', metadata-core 'Test Files 10 passed (10) / Tests 162 passed (162)'. The 5 new cases were confirmed to actually execute via a --reporter=verbose run of the file (20 passed, all five [#8595] cases listed as passing). REVERSE VERIFICATION, direction predicted BEFORE running: removing only the production audit loop should turn the 3 deliverable cases red and leave BOTH controls green, since the controls measure that the fix is a distinction rather than measuring the fix. Measured exactly that — 'Tests 3 failed | 17 passed (20)', headline failure `AssertionError: expected [] to deeply equal [ 'escalation', 'ticket' ]` (the empty trail, i.e. the defect itself); the never-published control and the compliant-batch control stayed green, and all 15 pre-existing #8400/#8594 cases stayed green, so the ablation is scoped to the pre-flight branch. The fix was COMMITTED BEFORE the ablation, so restore was `git checkout HEAD -- protocol.ts` out of a real commit; tree verified clean and ablation marker verified absent afterwards. No dogfood ablation is involved here (no dist-resolved test), so no rebuild claim applies. GATES, exit codes read UNPIPED (the sibling's warning was real — my first build's `EXIT=0` was the pipe's, and is not quoted anywhere): check:cross-package-test-inputs, check:durability-log-level, check:filter-alias-parity, check:error-code-casing (+ --self-test), check:query-options-erasure, check:nul-bytes, check:changeset-gate-self-tests, check:objectui-changeset, check:type-check-coverage, check:type-check-debt, and node scripts/{check-cross-package-test-inputs,check-adr-0087-registration,check-changeset-no-major,check-empty-changeset}.mjs — every one EXIT=0. RE-DERIVED against the actual diff with scripts/pm/dispatch-gates.mjs, which surfaced five families the dispatch list did not name (check:error-code-casing via the metadata-core edit, plus the four changeset-family gates); all were run. check:type-check-debt was NOT skipped and NOT reported as inapplicable: the workspace closure was fully built first (turbo 70/70 successful) and the ratchet then re-measured 33 ledger entries with none above its recorded number; metadata-protocol re-measures at 63, flat against its recorded 63, with zero errors in either file I touched (the package has no typecheck script by design — it is a DEBT ledger entry, so tsc does compile protocol.ts and the compile-time Record guard above is real, not a phantom check). The 1 reported surplus is @objectstack/lint, pre-existing and untouched.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #8747: auditMetaItem's `organizationId` parameter is dead on both ends — never read in the method, never passed by the REST route — while the comment directly above its `where` describes an org-scope `$or` that is not in the query, so the audit read returns every org's rows for a given (type, name); filed unassigned and unlabeled for triage because it is either a real cross-tenant disclosure or a comment that over-claims plus a parameter to delete, and settling which needs a two-org measurement outside this card's scope. Distinct from #8707, which is about write-time stamping rather than the read discarding the stamp."
      ]
    }

    Notes for the PM, outside the JSON:

    • Gate status is in_progress by design — this report is delivered at draft-PR time per the 2026-08-10 ruling; I did not idle-poll CI. Every gate the re-derivation named was run locally and is green at b2badd9, which is the sha quoted in the PR body too.
    • No skip-changeset label applied, deliberately — this PR ships user-visible behaviour and carries .changeset/preflight-refusal-audit-row.md (metadata-protocol + metadata-core, both patch), so the label would be wrong here.
    • Nothing on the PR was set by another actor as of the read-back; body stored intact (generics entity-escaped, no truncation), draft true, head b2badd9.

    Generated by Claude Code

  6. hotlong commented on Aug 14, 2026

    @hotlong
    Contributor

    ACCEPT — PR #8746. The shape question was settled by measuring the reader, which is what I asked for and not what I expected to get back.

    PM session session_01XeQRiAa7vYRVX5Fog7Zby8. Diff read on GitHub. Landing waits on gates.

    The fork closed without needing me

    I dispatched option 1 as a falsifiable assumption, with an explicit instruction to verify it against the actual reader and report a fork rather than fall back to option 2 silently. You went and found the reader: auditMetaItem (protocol.ts) queries exactly where { type, name } ordered by occurred_at desc, and rest-server.ts serves Studio's audit tab from it via GET /api/v1/meta/:type/:name/audit.

    I re-measured both on origin/main rather than taking it: the where really is { type: singular, name: request.name } and nothing else. The reader agrees with the assumption, so there was never a fork to adjudicate — and now the choice rests on a measurement instead of on triage's preference and mine happening to coincide.

    The metadata-core file is in scope, and you were right to touch it

    It was outside the surface I named, and the docs-drift bot flagged the second package before your report arrived — so I had it queued as a possible breach. It is not one: sys-metadata-audit.object.ts's TSDoc is the code column's documented closed set, so minting namespace_prefix without declaring it there is precisely the silent drift the batch_aborted note exists to warn about. Adding the value in one place and not the other would have been the smaller diff and the worse change.

    The closed-set guard is better than what the card asked for

    type PreflightViolationCode = 'NAMESPACE_PREFIX' plus Record<PreflightViolationCode, string> means a future pre-flight gate cannot reach this column without failing to compile until its spelling is declared in both places. The two alternatives you rejected are rejected for the right reasons, and you wrote them down: code.toLowerCase() would turn a documented closed set into an open one that grows with the error catalog, and a runtime fallback would put an unreachable value inside a set documented as closed. The compiler doing the enforcing is the shape this repo keeps asking for.

    The pin set is complete in the way this defect required

    The defect is a distinction — refused looked identical to never-attempted — so "a row appears" was never sufficient. Five cases, and each earns its place:

    • the deliverable (two violations → two rows on their own identities, correct code, not batch_aborted, no commit rows);
    • the opposite-direction control — same drafts, never published, still nothing — which additionally asserts the staging save rows did land, so "no publish row" is a real observation rather than a dead harness;
    • landed vs attempted agree, pinning that this write sits above the transaction and has nothing to survive — a future change that moved it inside would split those numbers, which is the regression worth catching;
    • the read door, asserting through auditMetaItem itself, which is what validates the shape decision rather than restating it;
    • the over-broad guard — a compliant batch under the same namespace writes allowed and no refusal — which is what catches a "fix" that fired whenever a namespace was declared.

    Reverse verification predicted the split before running: 3 deliverable cases red, both controls green, and it landed exactly there — headline expected [] to deeply equal [ 'escalation', 'ticket' ], which is the defect itself stated as a failure. All 15 pre-existing #8400/#8594 cases stayed green, so the ablation is scoped to the pre-flight branch.

    Two smaller things worth naming

    The wire shape is protected. organizationId rides the internal violation record but the failed[] projection drops it explicitly — adding a field to an internal array and letting it reach the response is an easy accidental widening, and you closed it deliberately rather than by luck.

    The org scope is the draft's, not the caller's. d.organizationId ?? null, matching the promoted rows, with the reason recorded: listDrafts surfaces env-wide drafts to a non-null-org caller, so keying on the active org would record the refusal against a partition the item never lived in. Pinned on both sides (toBeNull() and not.toBe(ORG)).

    And the pipe warning was real. You reported that your first build's EXIT=0 came from the pipe and did not quote it anywhere. That warning came from a sibling on this same round; it catching a second false green one dispatch later is the loop working.

    Out-of-scope finding

    #8747 — auditMetaItem's organizationId is dead on both ends. I confirmed both halves independently and added one measurement that narrows it: sys_metadata_audit does declare organization_id and index (organization_id, occurred_at), so Layer 0's field gate can engage and the wall may be carrying the scoping the comment misattributes. That makes it likely a dead parameter plus a misplaced comment rather than the alarming reading — but ⛔ not settled, since the two-org probe you correctly declined to guess at is still the deciding measurement. Recorded on the card for triage.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions