Uh oh!
There was an error while loading. Please reload this page.
fix(app-shell): unwrap the dispatcher envelope before reading duplicatePackage's verdict - #6604
Merged
os-sales merged 1 commit intoAug 27, 2026
Conversation
…tePackage's verdict
`duplicatePackage()` read `success` at the top level of the response body. That
is the runtime dispatcher's envelope (`deps.success(result)` answers
`{ success: true, data }`), so on every HTTP 200 the flag it read was `true` by
construction. The operation's own verdict lives one level down in `data` and is
a three-state, computed server-side as `failed.length === 0 && copied.length > 0`.
Two outcomes therefore answered 200 with `envelope.success: true` while the
duplicate had not succeeded, and both were reported to the Studio author as a
complete success:
- partial: some items failed to copy, with `failed[].error` the only place
the reason is ever stated;
- empty: nothing was copied at all (e.g. an all-env-wide source package under
a session that resolves no active organization).
Unwrap `data` first, then read the operation flag -- the order `revertCommit`
already uses for the sibling commit-revert route in `preview/commitHistory.ts`.
A false verdict now rejects with the copied/failed counts plus each
`failed[].error` (first five by name, then a `+N more` tail); a generic
`HTTP nnn` is not sufficient for the partial arm. The non-2xx arm is unchanged.
Both reachable false states are pinned; a suite covering only `res.ok === false`
would re-create the defect, because the transport arm was never the broken one.
Refs objectui#6593.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
This was referenced Aug 27, 2026
os-sales
marked this pull request as ready for review
August 27, 2026 13:14
This was referenced Aug 27, 2026
Uh oh!
There was an error while loading. Please reload this page.
os-sales
deleted the
claude/issue-6593-duplicatepackage-envelope-unwrap
branch
August 27, 2026 13:26
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Fixes#6593
duplicatePackage()readsuccessat the top level of the response body. That is theruntime dispatcher's envelope —
deps.success(result)answers{ success: true, data }—so on every HTTP 200 the flag it read was
trueby construction. The operation's ownverdict lives one level down in
data, and it is a real three-state.Two outcomes therefore answered HTTP 200 with
envelope.success: truewhile theduplicate had not succeeded, and both were reported to the Studio author as a complete
success, followed by a navigation into the new base:
failed[]carries a per-itemerrorstringthat is the only place the reason is ever stated, and none of it was read.
session that resolves no active organization (
copiedCount: 0,failedCount: 0).What changed
packages/app-shell/src/views/studio-design/packages-io.tsonly.envelope's
error.message, falling back to the status. That arm was never the broken one.revertCommitalready uses for the sibling commit-revert route inpreview/commitHistory.ts. This is one consumer converging on a pattern already in usenext door, not a new convention; the
?? payloadarm is carried over from that helperverbatim, and against today's single wrapping surface only the
dataarm is live.happened: the copied/failed counts, plus each
failed[].error(the first five named infull, then a
+N moretail). A genericHTTP nnnmessage is deliberately not sufficientfor the partial arm — it is the half an author needs to act on. The empty arm has no
failed[]to quote, so it says so in words rather than falling through to a status.The caller (
BuilderLanding.doDup) already renders a thrown message into the duplicatedialog's error slot and skips the success toast plus the navigation, so no caller edit was
needed. Turning the partial arm into a non-throwing "warn and continue" affordance would
require editing
BuilderLanding.tsx, which is outside this card's declared file surface; itis not attempted here.
The three upstream assumptions, re-measured
The card's readings were taken on objectstack
origin/main@d7b3963c2. Re-measuredtoday against objectstack
origin/main@15bf9e85— all three hold, none forked:packages/runtime/src/domains/packages.ts, theparts[1] === 'duplicate' && m === 'POST'branch, answers{ handled: true, response: deps.success(result) };success(data, meta)builds{ status: 200, body: { success: true, data, meta } }inhttp-dispatcher.ts.packages/rest/src/rest-server.tsmounts no twin — its fourduplicatehits are therecord-clone door and prose, none of them this route. Error responses come from
apiErrorResponse, which is{ success: false, error }at the top level with nodatato unwrap, which is why the non-2xx arm reads
error.messagewhere it always did.MetadataProtocol.duplicatePackagereturnssuccess: failed.length === 0 && copied.length > 0. Sosuccess === falsewith nofailures implies nothing was copied — the two arms this PR pins are exhaustive.
Corroborated in the protocol's own comments, which record an all-env-wide source under a
no-org session degrading "quietly:
{success: false, copiedCount: 0, failedCount: 0},nothing copied and nothing named as failed" — the empty arm, measured upstream rather
than imagined here.
route ledger row is still
{ route: 'POST /packages/:id/duplicate', disposition: 'sdk' }with no
responseSchema, so the contract absence recorded in objectstack#12038 is intactand stays there — nothing in this PR waits on it.
Tests
packages/app-shell/src/views/studio-design/packages-io.duplicateEnvelope.test.ts, 8 cases.Both reachable false states are pinned, plus the exact shape the defect read as success
(
success: trueoutside,success: falseinside); a suite covering onlyres.ok === falsewould re-create the defect. The partial case asserts the counts AND every per-item error
string, and asserts the message is not a generic
HTTP nnn.Reverse verification — direction predicted before running, and observed:
origin/mainand themutation was confirmed on disk before anything was read — on-disk
git hash-objectequal to the
origin/mainblob4f70bbf46, and the two markers flipped (DuplicateOutcome4 hits to 0, the old
payload?.success === falseline 0 hits to 1). The script aborts thereading rather than continuing if either check fails, and restores through a
trap ... EXIT INT TERMusing absolute paths.four HTTP-200 cases go red, with
promise resolved "undefined" instead of rejectingasthe defect stated verbatim; the two transport cases, the success case and the
request-shape case stay green, because that arm was never broken.
git checkout HEAD -- ABSOLUTE_PATH(never the bare form, which reads from theindex the mutation wrote), proven by observation rather than exit code — on-disk hash back
to the HEAD blob
ccf290825,git diff HEADempty,git statusclean, markers back to4 / 0.
./packages-io),so it resolves to source, and this package has no
diston disk in this worktree at all.Gates run locally, all on the final commit
586d725Union re-run after the last commit, with the sha echoed at the start and end of the same run:
pnpm exec vitest runover both packages-io suites —Test Files 2 passed (2) / Tests 17 passed (17)pnpm --filter @object-ui/app-shell type-check— green (tsc --noEmit && tsc -p tsconfig.test.json).--listFilesconfirms the new suite IS in the test project's program (1 hit), so "typecheckclean" actually covers the file this PR adds.
pnpm --filter @object-ui/app-shell lint—2760 problems (0 errors, 2760 warnings), exit 0.Warnings are the repo's declared pre-existing debt; the lint workflow sets no
--max-warningson purpose. The two changed files themselves report 0 errors and0 warnings under
--format json.node scripts/check-changeset-presence.mjs— green, naming the added changeset. It wasmeasured red before the changeset was written, so this is a live gate here, not a
decoration.
pnpm changeset:check,pnpm check:control-bytes,pnpm check:vi-mock-specifiers,pnpm check:i18n-keys— all green.pnpm check:readme-exports— NOT MEASURED, not red: it fails with 69 "type entry./dist/index.d.tsis not on disk -- runpnpm buildfirst" prerequisites in thisworktree, none of them in a file this PR touches (no README changed). CI builds first.
The repo-wide farm (
pnpm lint=turbo run lint, the fullpnpm test) is CI's run and isnot duplicated here.
Fence
Region fence with the sibling card in
packages/app-shell/src/views/metadata-admin/wasrespected: nothing under that path was read into the diff or edited. The neighbours the card
put off limits —
fetchCommits/revertCommit/fetchPendingDraftsand their defensivethree-arm envelope chains — are untouched; they remain contract-absence evidence for the
upstream survey. Card relationships are declared once here, not in the commit trailers,
because this branch squashes.
Generated by Claude Code
Generated by Claude Code