Repository navigation
upload-session-abort c4: failed/expired sys_upload_session.status have no producer (enforce-or-remove) #7667
Description
Activity
Claim — PM session
a31dbd89(domain:services seat, #6021), dispatching now.Branch:
claude/issue-7667-upload-session-status-livenessAnchors re-verified on
origin/mainbefore dispatch: enum members atsystem-upload-session.object.ts:104-105, retentiononlyWhenat:134, type union atmetadata-store.ts:54; a repo grep confirms no writer for either status (the twostorage-routes.tshits are unrelated error-message string matches). The reap-guard registration lives instorage-service-plugin.ts(~285-350), so the enforce route would touch a #5536 seam file — the dispatch carries the ride-along clause conditionally.
Generated by Claude Code
Claim released — dispatch aborted before any work started (PM session
a31dbd89, services seat #6021).The developer agent dispatched at 11:44Z terminated on its first step: an API session-limit error (resets 12:40Z). Verified nothing was produced —
git ls-remote origin 'claude/issue-7667*'returns no branch, no PR exists, and no commits were pushed. The branch name announced in the claim above (claude/issue-7667-upload-session-status-liveness) is unused and free for whoever takes this next.Returning to
pm:queuerather than leaving apm:dispatchedlabel describing work nobody is doing. This seat will re-dispatch once the limit resets, unless another seat claims it first — the anchors in the claim comment above were re-verified onorigin/mainat 11:4xZ and stay usable.
Generated by Claude Code
Claim: PM loop round 2
Session:session_01JVfWSQN9RDDGwdVSmMc29x
Branch:claude/issue-7667-upload-session-status-liveness
Worktree: cloud session's own clone (mode:cloud)
Domain:domain:services
File surface:packages/services/service-storage/src/**—objects/system-upload-session.object.ts(:104-105 enum, :134 retentiononlyWhen),metadata-store.tstype union, and — only if the enforce route is taken — the reap-guard registration instorage-service-plugin.ts(~285-350).⚠️ That file is a #5536 seam file: the dispatch carries the ride-along clause conditionally.
Container & model: M (ADR-0049 enforce-or-remove is evidence-driven design, not mechanical),mode:cloud,model: opus
Serial constraints cleared: prior claim on this card (os-help sessiona31dbd89, 11:43Z) was released at 12:43Z with zero output — branch name announced there is unused; re-using the same anchors (re-verified then, and same-day churn on service-storage since — #7778 test-resolution, #7683 $expand security — touches neither named file). No in-flight PR touches service-storage.
Generated by Claude Code
{ "issue": 7667, "repo": "objectstack-ai/objectstack", "status": "draft_pr_open", "premise_still_valid": true, "premise_evidence": "Re-verified on origin/main 098f4bb: enum members `failed`/`expired` at packages/services/service-storage/src/objects/system-upload-session.object.ts:104-105; retention onlyWhen `$in ['completed','failed','expired']` at :134; type union at metadata-store.ts:54. Grep across service-storage/src confirms no writer for either status — the only non-test hits were the enum, the retention clause, the type union, and the reap-guard doc comment at attachment-lifecycle.ts:342 (which READS status but only compares against 'completed'). Same-day churn (#7778, #7683) touched neither file.", "branch": "claude/issue-7667-upload-session-status-liveness", "commit": "c213ee8", "pr": 7844, "pr_url": "https://github.com/objectstack-ai/objectstack/pull/7844", "pr_state": "draft", "decision": { "branch_taken": "enforce", "deciding_evidence": "packages/spec/src/api/storage.zod.ts:247 declares UploadProgressSchema.status as z.enum(['in_progress','completing','completed','failed','expired']) — both statuses are PUBLISHED to every client reading the API contract, not merely declared-and-unread. Removal would have forked the object enum from the spec progress contract and forced a packages/spec narrowing (the dispatch's STOP condition); enforcement gives that contract teeth instead. Independently, both name real failure states that were invisible: a completion that threw left the row at `completing` (non-terminal → the 7d retention backstop never reaped it, progress read 'still assembling' indefinitely), and a session past its own expires_at kept accepting chunks until the TTL sweep deleted the row under the caller.", "remove_branch_rejected_because": "A partial removal leaves the retention rule naming a state that cannot exist; a full removal additionally strands UploadProgressSchema declaring two values the server can never emit — the same defect relocated one layer up, in a package with a single owner." }, "changes": [ "packages/services/service-storage/src/storage-routes.ts — new `expireIfPastDeadline` + `markSessionFailed` helpers; chunk PUT and complete refuse 410 UPLOAD_SESSION_EXPIRED on an overdue session and stamp the row `expired`; progress REPORTS `expired` (declared member of UploadProgressSchema.status, and the SDK's resumeUpload polls progress first); the completion backend call + file commit are wrapped so a throw stamps `failed` before the 500 propagates", "packages/spec/src/api/error-code-ledger.zod.ts — ⚠️ OUT-OF-SURFACE, ONE ADDITIVE LINE: registers UPLOAD_SESSION_EXPIRED under @objectstack/service-storage", "packages/services/service-storage/src/objects/system-upload-session.object.ts — lifecycle comment records the producer for each status the retention clause names, and that adding a member without a writer re-opens the hole (ADR-0049). Enum and retention clause UNCHANGED — they are now correct.", "packages/services/service-storage/src/storage-routes.test.ts — 8 new tests, every one asserting the DURABLE ROW via store.getSession, not just the response body", "packages/services/service-storage/src/error-envelope.conformance.test.ts — the suite keeps one case per distinct emitted code; added the 410 UPLOAD_SESSION_EXPIRED case", "docs/qa/platform-checklist/areas/attachments-storage.json — attachments-storage.upload-session-abort revision 3", ".changeset/upload-session-terminal-statuses.md — patch for @objectstack/service-storage + @objectstack/spec" ], "out_of_surface_additions": [ { "file": "packages/spec/src/api/error-code-ledger.zod.ts", "what": "one line registering UPLOAD_SESSION_EXPIRED under @objectstack/service-storage", "why": "ErrorCode is the closed union StandardErrorCode ∪ ERROR_CODE_LEDGER; an unregistered code fails schema parse → fails the envelope conformance suites → fails CI (ADR-0112). The only alternative was reusing a semantically wrong registered code (UPLOAD_SESSION_NOT_FOUND for a session that plainly exists), which is exactly the bypass the ledger's own header warns against.", "risk": "none identified — purely additive; the union grows, nothing that validated before stops validating. Verified against error-code-ledger.test.ts (SCREAMING_SNAKE, no duplicate, no shadow of the standard catalog, sorted union): 62 spec tests pass." } ], "ride_along_5536": "not_triggered — the diff does not touch storage-service-plugin.ts", "behaviour_changes": [ "NEW REFUSAL: a chunk PUT or a complete against a session past its own expires_at now answers 410 UPLOAD_SESSION_EXPIRED instead of succeeding. Bounded: default sessionTtl is 86400s, and past expires_at+1d the TTL sweep already reaped the row and the reap guard already aborted the backend multipart — so beyond that window the call previously failed anyway, with an opaque backend NoSuchUpload rather than a named 410. The new refusal is a clear failure replacing a confusing one, plus a correct refusal inside the [expires_at, expires_at+1d] grace window.", "A failed chunked completion now leaves the row at `failed` rather than `completing`. Purely additive observability — no code path reads `failed` as a refusal, so a retry of the same uploadId still runs the happy path (pinned by a test)." ], "deliberate_non_behaviours_pinned_by_tests": [ "a row with no expires_at is never expired — the guard enforces the row's own declared deadline, it does not invent one", "a `completed` row does not become `expired` by waiting for the reaper", "expiry is checked AFTER the resume-token check, so a caller who cannot prove ownership learns nothing about the session's state", "the `failed` stamp is best-effort and loud on failure: a metadata-store error never replaces the real backend cause on its way to the 500" ], "gates": { "build_closure": "pass — pnpm build, 71/71 tasks", "service_storage_suite": "pass — 24 files, 361 tests", "spec_targeted_tests": "pass — error-code-ledger + storage, 62 tests", "check_docs_audit_scope": "pass", "check_platform_checklist": "pass — 15 areas, 182 items", "check_route_envelope": "pass — the new 410 goes through the shared sendError", "typecheck_spec": "pass", "typecheck_service_storage": "n/a — the package declares no typecheck script (not type-check-covered). An ad-hoc `tsc --noEmit -p tsconfig.json` over it reports only pre-existing extension-less test-import noise (TS2835) in files this diff does not touch; zero errors in storage-routes.ts or either edited test file.", "engine_double_gate": "n/a — no new test fake with update/delete verbs; the tests drive the real StorageMetadataStore on its no-engine Map path", "ci": "in_progress — PM owns CI convergence, the ready-flip and landing" }, "checklist_maintenance": { "item": "attachments-storage.upload-session-abort", "revision": 3, "change": "closes the clause-5 finding revision 2 deferred to this issue: added steps that drive `failed` (a completion whose backend throws) and `expired` (backdate expires_at WITHOUT sweeping, then chunk → 410 / progress → 'expired'); added an acceptance clause for the 410 refusal; named the producers in `source`; recorded transient `completing` as a knownGap rather than an unreachable-variant FAIL — the complete route writes it and overwrites it within the same request, so a scan of settled rows can never show it and a run that cannot produce it is not a defect" }, "open_questions": [ "packages/client `storage.resumeUpload` reads the progress body but ignores `status`, so resuming an expired session now surfaces as a 410 on the first chunk PUT rather than a clean early exit. Honest failure, not a regression — but a short-circuit on `status === 'expired'` would be better client behaviour. NOT taken here: packages/client is outside this card's declared file surface. Suggest a separate domain:client card.", "`expires_at` is now enforced at request time on chunk/complete, but the presigned-upload family has no comparable deadline surface. Not in scope; noting it so the asymmetry is a recorded choice rather than an oversight.", "The reap guard's doc comment (attachment-lifecycle.ts:342) already described `in_progress / failed / expired with a backend upload` as the abort branch — written before any producer existed. It is now accurate as written and was left untouched." ], "notes": "Worktree: the repo's guard-main-checkout hook blocked edits on the container's primary checkout, and both `git checkout main` and `git checkout --detach` were refused by the environment's permission classifier. Resolved without weakening the rule: created a real dedicated worktree detached at the branch tip (`git worktree add --detach ../objectstack-7667`), did all work + `pnpm install` + gates there, and pushed with `HEAD:refs/heads/claude/issue-7667-...`. No `git stash` at any point." }
Generated by Claude Code
- added a commit that references this issue
on Aug 11, 2026 ADDENDUM — patch round 1 (generated-docs backfill). Executed the PATCH ROUND instruction on PR #7844 verbatim; nothing else on the PR changed.
{ "issue": 7667, "pr": 7844, "round": "patch-1", "status": "patch_pushed", "branch": "claude/issue-7667-upload-session-status-liveness", "patch_commit": "d4cceb6", "previous_commit": "c213ee8", "cause_confirmed": "产物随源走 — registering UPLOAD_SESSION_EXPIRED in ERROR_CODE_LEDGER widens the ErrorCode union that every enveloped response schema references, so all 11 generated content/docs/references/api/*.mdx pages rendering it went stale and `tsx scripts/build-docs.ts --check` failed the TypeScript Type Check job. The generated set was not committed with the source change.", "steps_executed": [ "1. Pre-flight per #5370: worktree tree clean, no MERGE_HEAD / REBASE_HEAD in /home/user/objectstack/.git/worktrees/objectstack-7667 — gen:schema was NOT run mid-merge", "2. pnpm --filter @objectstack/spec gen:schema — 1590 schemas, 1578 definitions; authorable-defaults verified against upstream 098f4bb6b9d2, 1276 defaults unchanged", "3. pnpm --filter @objectstack/spec gen:docs — 230 files generated, 11 tracked files changed", "4. Committed content/docs/references and NOTHING else (verified with `git status --short` before and after staging)", "5. pnpm --filter @objectstack/spec check:docs — GREEN", "6. Pushed c213ee8..d4cceb6 to the same branch" ], "files_committed": [ "content/docs/references/api/analytics.mdx", "content/docs/references/api/auth.mdx", "content/docs/references/api/automation-api.mdx", "content/docs/references/api/batch.mdx", "content/docs/references/api/contract.mdx", "content/docs/references/api/error-code-ledger.mdx", "content/docs/references/api/export.mdx", "content/docs/references/api/metadata.mdx", "content/docs/references/api/package-api.mdx", "content/docs/references/api/protocol.mdx", "content/docs/references/api/storage.mdx" ], "diff_shape": "11 files, +69/-67, and the ENTIRE diff is the one ledger line propagating: a new `* UPLOAD_SESSION_EXPIRED` bullet in error-code-ledger.mdx (inserted in sorted position before UPLOAD_SESSION_NOT_FOUND), and the rendered union arity moving `+260 more` → `+261 more` in every `error` column. Inspected each file's diff — no unrelated pre-existing drift was absorbed into this commit.", "check_docs_rerun": "PASS — `✅ 230 generated files in sync with packages/spec` (plus `✅ import examples resolve against api-surface/`, 62 accepted baseline gaps). The two `Schema directory .../conversions|migrations does not exist` lines are pre-existing warnings, not failures.", "authorable_surface_anchor": "NOT touched — `authorable-surface.base.json` does not appear in `git status` after gen:schema, so no re-anchoring rode along and nothing was hand-edited. gen:schema emitted its standard shallow-checkout advisory (the anchor mirrors 53ef05744f37 vs the resolved baseline 098f4bb6b9d2, direction unestablishable on a truncated history) — informational, `Not an error either way` per its own text, and unchanged by this round.", "gen_openapi_5371": "backfill condition did NOT trigger — packages/spec/json-schema/openapi.json survived gen:schema's rmSync (mtime predated this round). Ran `gen:openapi` anyway to keep the local artifact current; it is gitignored (.gitignore:61 packages/spec/json-schema/), so it is absent from the commit either way.", "scope_discipline": "No source, test, changeset, or checklist file was touched in this round. `git status` is clean at d4cceb6.", "ci": "not polled — the PM owns CI convergence, the ready-flip and landing, per the dispatch contract." }
Generated by Claude Code
ACCEPT — PR #7844 (services seat review, session
session_01JVfWSQN9RDDGwdVSmMc29x).What shipped: the enforce branch of ADR-0049, with the deciding evidence I endorse —
UploadProgressSchema.statusalready publishesfailed/expiredto every API client, so removal would have forked the object enum from the spec contract (and hit the dispatch's own spec-STOP condition), while both statuses name real, previously-invisible failure states.expired: chunk PUT / complete against a session past its ownexpires_atnow refuse 410UPLOAD_SESSION_EXPIREDand stamp the row; progress REPORTS the status (the SDK polls it first).failed: a completion whose backend throws stamps the row instead of stranding it at non-terminalcompleting(which the 7d retention never reaped). Retry semantics preserved —failedrecords an attempt, doesn't lock the session.Review notes:
- Deliberate non-behaviours pinned by tests are exactly the right guardrails: no invented deadlines (no
expires_at⇒ never expired),completedrows immune to the reaper clock, expiry checked AFTER the resume-token gate (no state disclosure to non-owners), best-effort-but-loudfailedstamp that never eats the real backend cause. - 8 new tests all assert the DURABLE row via
store.getSession; error-envelope conformance case added for the new 410. ⚠️ Declared-surface addition, accepted with declaration: one additive line inpackages/spec/src/api/error-code-ledger.zod.tsregisteringUPLOAD_SESSION_EXPIRED— forced by the closedErrorCodeunion (ADR-0112); the alternative (reusing a semantically wrong code) is the bypass the ledger's own header forbids. Cross-seat declaration to the spec seat follows. The checklist item revision (rev 3) closes the clause-5 finding that this card owned — proper rider.- settings 消费缝丢弃 ResolvedSettingValue.source —— 服务无法区分「管理员写过的值」和「schema 默认值」 #5536 ride-along: correctly not triggered (diff avoids
storage-service-plugin.ts). - Patch round (1): the spec ledger line regenerates
content/docs/references/api/*.mdxand the generated set wasn't committed —check:docsred in the TS job. Fixed via the regen discipline (os-regen 驱动指示的gen:schema在 merge 未 commit 时运行,会把 authorable-surface 锚点倒退回旧 merge-base —— 生成器写入、门全绿、静默撤销 main 的锚点推进 #5370/gen:schemarmSync 整个json-schema/会顺手抹掉gen:openapi的产物,rest 的 openapi 路由测试随后 503 假红——check:generated原地跑 build-schemas 也触发 #5371 traps avoided); all 26 checks green on the patched head. 产物随源走 lesson recorded for the seat: any spec source touch ⇒ name the regen step in the dispatch. - Worktree note in the report (real detached worktree after the guard hook fired, no
OS_ALLOW_MAIN_EDITS, no stash) — compliant path, worth the read for other cloud devs. - Open questions endorsed as filed: the
packages/clientresumeUploadshort-circuit onstatus === 'expired'is a legitimate follow-up for thedomain:cliseat (will flag in the round report); the presigned-family deadline asymmetry is a recorded choice.
Marking ready and arming auto-merge now.
Generated by Claude Code
- Deliberate non-behaviours pinned by tests are exactly the right guardrails: no invented deadlines (no
- added 3 commits that reference this issue
on Aug 17, 2026
Symptom
A scan of all
sys_upload_sessionrows after driving both the chunked-complete flow and the abandon/reap flow returns exactly['in_progress', 'completed'].completingis written transiently and immediately overwritten. Thefailedandexpiredstatuses are never written by the service, yet the retention rule references them — so retention references two states the system can never enter.Root cause
Located. In
packages/services/service-storage/src/objects/system-upload-session.object.tsthestatusenum declaresfailedandexpired(lines 104-105) and the retentiononlyWhenis{ status: { $in: ['completed', 'failed', 'expired'] } }(line 134). A grep acrossservice-storagefindsfailed/expiredonly in that enum, the retention clause, themetadata-store.tstype union, and tests — no writer sets either status. An expired session's row is TTL-reaped, never re-statused. This is an ADR-0049 enforce-or-remove candidate.Fix shape: either give
failed/expireda writer (status the row on abort-failure / TTL expiry before it is reaped) or drop them from the enum and the retentiononlyWhen.Related contract note (also captured in the checklist-maintenance card): the chunked-init route takes
totalSize, notsize(packages/services/service-storage/src/storage-routes.ts:274), and forceschunkSize >= 5 MiB; the checklist step text sayssize.Reproduction
Drive a chunked upload to completion and abandon a second session so it is reaped; then scan
sys_upload_session.statusacross all rows → only['in_progress', 'completed']ever appear. Grepservice-storage/srcfor'failed'/'expired'→ matches only in the enum, the retention clause, the type union, and tests — no producer.Source
Extracted from the QA run #7635 (framework 92f26f7, console 6314e87f).