Make assistant output review safe and lifecycle-complete

[alexeygrigorev]

Make assistant output review safe and lifecycle-complete

Status: blocked for coordinated release — final Tester PASS and PM ACCEPTED; exact commit 8839fcfb39a3553a673d395a7b3746e2c92218cd is clean on issue-158-stage-c-r1, held off main and unpushed; no shipped/deployed claim
Tags: bug, portal, assistant, frontend, backend, testing, data, design, P0
Related: historical frontend-convergence parent #148 is closed; #155, #156, and #157 are shipped
Depends on: #217 preview/deployment/post-deployment identity gate for coordinated publication; prior implementation dependencies are satisfied
Blocks: None
Next owner: Orchestrator after #217 release/identity PASS, for safe local integration and normal push; then On-Call
Resume condition: #217 completes its HUMAN preview, ordinary main/OIDC deployment, terminal On-Call result, and HUMAN identity verification; then resume coordinated publication without carrying the rejected old #146 candidate.

Product outcome

Make assistant-job review explicit, concurrent-safe, authenticated, and private. The canonical frontend/ flow creates and edits a persisted draft, exposes only actions valid for authoritative state, mediates every assistant-output access through a same-origin artifact-ID route, and recovers honestly from conflicts and failures.

Assistant jobs remain core internal capabilities, not plugins or model tools. Nothing in this issue publishes, sends, commits, dispatches, deploys, invokes an external provider write, or creates infrastructure.

The canonical work vocabulary is Template → Card → Task. Do not add workflow/bundle aliases, legacy fields, dual-read behavior, or defaulting for old rows.

Current defects confirmed in source

• Creating in the canonical UI immediately submits; Retry immediately submits again instead of leaving a visible retrying state.
• Assistant-job and artifact updates are unconditional. Job mutation, immutable event, artifact review, and Task/Card reference updates can partially succeed or race.
• AssistantJobRecord has no version; repository reads cast persisted values without validating the canonical shape.
• Browser routes can trust client requestedBy, actorId, or x-user-id fallbacks.
• Operator-facing routes expose runner-owned transition, event, and artifact-attachment actions.
• Assistant detail and shared artifact paths disclose provider URIs in records or nested references; the current privacy helper strips only s3://.
• Approval can copy a private provider URI into Task.link, Card.cardLinks, and Task/Card artifactRefs.
• Assistant job and event scans are not paginated.
• Prevent lost Task updates and history with optimistic concurrency #179 and Complete and archive cards automatically when all tasks are done #168 now define strict Task/Card version, history, transaction, lifecycle, conflict, and audit behavior. Make assistant output review safe and lifecycle-complete #158 must compose with those contracts rather than update Task and Card independently.

Canonical server contract

Persisted Assistant Job

Every persisted Assistant Job has an integer version >= 1. New creation writes version 1 with an absence condition. Every operator or runner mutation supplies an exact expected version, checks the exact allowed source state, increments version once, and writes one immutable lifecycle event in the same DynamoDB transaction.

There is no versionless compatibility. Reads and lists reject missing, non-integer, or otherwise noncanonical rows with a sanitized data_contract_error; they never coerce or default them. Before production release, the operator performs the sanitized shape-only preflight in Stage C. If retained production rows are noncanonical, stop and separately groom disposable table/resource replacement. Do not add a migration, backfill, import, dual-read, or fallback.

A stale mutation returns JSON 409 version_conflict with a sanitized current-job summary. It performs no state change, attempt increment, artifact review, Task/Card update, or success event. Repository list/event reads paginate to completion without fixed result-count assumptions.

State and action table

Current state	Operator actions	Result
draft	Edit/Save, Submit, local/test Run dry, Cancel	Save remains draft; Submit becomes queued; Cancel becomes canceled.
queued	local/test Run dry, Cancel	A trusted runner may move to running; Cancel becomes canceled.
running	Cancel	Trusted runner owns waiting_approval, succeeded, or failed.
waiting_approval	Mediated access, Approve, Reject with reason, Cancel	Approve becomes approved; Reject becomes rejected; Cancel becomes canceled.
failed or rejected	Retry when attempts remain	Confirmed Retry becomes retrying and increments the attempt exactly once.
retrying	Submit retry, local/test Run dry, Cancel	Confirmed Submit retry becomes queued; it is a separate request.
approved	Read only	Trusted runner may complete approved -> succeeded.
succeeded or canceled	Read only	Terminal.

State validation is shared by repository/service and route callers. Ordinary browser actors cannot invoke generic transition, append-event, or attach-artifact actions. Delete those public operator endpoints rather than retain compatibility aliases; trusted runner and local dry-run code call package-internal transaction services with explicit actor provenance.

Artifact privacy and access

The full ArtifactRecord.storageUri remains server-private. Browser DTOs, nested Assistant Job refs/log refs, Task/Card artifact refs, events, errors, logs, DOM, history, and screenshots contain no stable URI, signed URI, credentials, token query, or provider object key. Public refs contain safe artifact ID metadata only.

One server-owned policy and serializer are used by Assistant detail, Artifacts list/detail, nested refs, and access actions. Defense-in-depth response privacy removes every storageUri key recursively, regardless of scheme.

The browser receives only same-origin artifact-ID actions such as /api/artifacts/<id>/access?action=open|preview|download plus safe capability metadata:

Provider	Required behavior
DataOps-owned s3	Reauthorize each action. Only already-configured DataOps buckets are eligible; no IAM/infra expansion. The server may respond with a short-lived redirect. Unsafe inline types are attachment-only.
local-dev	Deterministic local/test fixture only. Production fails closed. The URI is never rendered.
github, google-drive, dropbox, external-url	Revalidate HTTPS, provider/host consistency, and absence of credential/token query at access time. Descriptor shows only provider and safe destination hostname before a same-origin Open action redirects.
Unknown, missing, malformed, disallowed bucket/host/content	No fabricated link. Return a sanitized retryable JSON unavailable error.

New-tab access uses noopener. An attacker-controlled filename or content type cannot execute active content in the portal origin. Tests use synthetic resolvers and make no AWS or provider call.

Canonical Task/Card integration

Creating from Task/Card context validates that referenced entities exist and, when both IDs are present, that the Task belongs to the Card.

Assistant Job and approved Artifact references are additive ID metadata only. Approval must never copy a provider URI into Task.link, Card.cardLinks, ArtifactRef, or AssistantJobLogRef; a private artifact does not silently satisfy a URL-proof requirement.

Any approval that touches a Job, its Artifacts, a Task, or a Card is one bounded DynamoDB transaction with exact source-state/version conditions and one audit outcome. It preserves #179 Task history/version and #168 Card counters/lifecycle/audit/owner behavior. Build the transaction from strong current reads; if any condition changes, return 409 with no blind retry and no partial review. Reject an oversized review batch before writing, based on the actual DynamoDB transaction capacity rather than truncating output IDs.

Acceptance criteria

• Create persists exactly one canonical draft, version 1, opens its canonical detail route, retains valid Task/Card/input context, and never submits or runs it.
• Draft fields are editable only in draft. Save, Submit, Run dry, Retry, Submit retry, Approve, Reject, and Cancel require expectedVersion and the exact source state.
• Every successful state mutation and its immutable event are atomic. Concurrent clients have one winner; losers get sanitized 409 version_conflict with no duplicate event, attempt, review, or Task/Card effect.
• Retry and Submit retry remain two explicit confirmed actions across success, failure, refresh, Back/Forward, and reload.
• Every successful or failed browser action re-fetches authoritative Job, Artifact, and event state before choosing the next controls. User-entered draft/rejection text remains recoverable after a conflict or network/server error.
• Active authenticated operator and admin users may use valid operator actions. Missing, expired, disabled, unsupported-role, header-spoofed, and body-spoofed identities receive JSON 401/403 with no disclosure or write.
• Assistant detail and Artifacts surfaces show safe title/status/provider/hostname/type/size/review metadata and only policy-allowed same-origin ID actions.
• Browser-facing JSON and HTML contain no provider URI or signed URL at any nesting depth. Approval mirrors only safe IDs and never auto-populates Task/Card URL proof from private storage.
• Invalid provider, disallowed bucket/host, unsafe content, not-found, access-denied, and server failures are distinct, sanitized, and retryable without false success.
• Ordinary operators cannot call runner-owned transition/event/attachment actions. Conversational/model output cannot call any lifecycle or artifact-access action.
• Changed controls and confirmations are keyboard/touch operable with visible focus, focus trap/restore, useful headings/live status, and no critical/serious WCAG A/AA findings in affected states.
• No implementation changes deploy workflow, SAM/IAM/CloudFormation, tables, provider accounts, import/export/restore code, or assistant engines/prompts/tools.
• Tests and screenshots use public-safe synthetic data only; no real provider execution or production identifiers.

Implementation stages

Stage A1 — backend domain, persistence, privacy; eligible locally now

Own Assistant Job/Artifact types, repositories, domain transaction services, public DTO/access policy, and focused backend tests only.

Implement strict Job validation/versioning, conditional create/mutations, atomic events/reviews, pagination, URI-free public refs/DTOs, provider eligibility, and same-origin access decisions. Keep actor resolution an explicit service input so this stage does not invent another auth path.

Do not edit frontend, Playwright/browser inventory, deploy workflow, SAM/infra, import/export/restore, or #180-owned paths. After Assistant Engineer, Architect, Tester, and PM acceptance, make an explicit-path Refs #158 checkpoint commit and keep it on an isolated branch, off main and unpushed.

Stage A2 — authenticated routes and Task/Card composition

Depends on Tester/PM-accepted #164 Stage A. Reuse its single verified active-User/supported-role actor resolver; do not duplicate it.

Wire public operator actions and artifact access, remove generic public runner actions, deny spoofed actor sources, and compose approval with exact #179/#168 Task/Card conditions. Focused API request/permission/concurrency tests own the matrix; Playwright does not duplicate it.

Accept and checkpoint with Refs #158, still off main.

Stage B — canonical UI and independent browser behavior

Depends on Tester/PM-accepted #180 source. Rebase or reconstruct from its accepted UI/browser baseline before editing Assistant, shared Artifact, Task-evidence, focus/navigation, or browser paths.

Implement explicit draft Save, two-step Retry/Submit retry, confirmations, authoritative refresh, safe artifact actions, responsive states, and conflict/failure recovery. Preserve #180’s accepted focus, navigation, server-isolation, accessibility, and race fixes.

Use real local API behavior with synthetic DynamoDB fixtures. Do not use request interception, mocked entity payloads, fixed sleeps, browser-history arithmetic, exact incidental copy, serialized byte pins, or screenshot-only proof. Add behavior titles to the independent non-deploy-blocking browser workflow and preserve its failure artifact/report upload; the full browser suite must not become a deploy prerequisite.

After Designer, Tester, and PM acceptance, checkpoint with Refs #158, still off main.

Stage C — steady-state integration and release

No #158 commit may enter the reviewed #166 Phase D artifact or transitional rollout chain. Wait for repaired #166 A/B/C, #182’s final Cards preflight while writers are closed, reviewed D, canonical first-write evidence, and accepted cleanup restoring the ordinary push OIDC workflow.

Then reconstruct/rebase the accepted #158 stages onto clean steady-state main, run the sanitized read-only Assistant Job/Artifact shape preflight, and fully reverify. If the preflight finds noncanonical retained rows, stop for a separate disposable-resource replacement plan; do not migrate, backfill, restore, import, or add compatibility.

Only then receive final Tester PASS and PM ACCEPTED, commit with Closes #158, merge/push normally, and obtain source/deployed on-call evidence.

Behavior scenarios

Concurrent lifecycle winner

Given two authenticated clients loaded at one Job version
When they concurrently save, submit, retry, approve/reject, or cancel
Then exactly one transaction wins; the loser receives 409; Job, event, attempts, Artifact review, and Task/Card references show no partial or duplicate effect

Retry remains visible

Given a failed or rejected Job with attempts remaining
When confirmed Retry succeeds and Submit retry later fails by a deterministic server fault
Then authoritative state remains retrying, no queued success is claimed, and Submit retry remains available after refresh/reload

Mediated provider access

Given synthetic allowed and denied records for every provider class
When an active operator reads detail and activates offered actions
Then the central policy allows only the declared behavior, unsafe cases fail closed, and stored/signed URIs never appear in JSON, DOM, logs, errors, history, or screenshots

Identity boundary

Given active admin/operator, disabled/deleted/unsupported users, expired/absent sessions, and spoofed identity headers/body fields
When each reads or mutates a Job or accesses an Artifact
Then only verified active supported actors receive state-valid behavior; all others receive JSON 401/403 and no side effect

Task/Card approval composition

Given a canonical Job linked to a canonical Task/Card and a concurrent Task/Card version change
When approval is attempted
Then relation and versions are revalidated, conflict returns 409, and no Job, Artifact, Task, Card, or audit item partially changes; a later fresh approval records only safe IDs

Responsive recovery

Given draft, confirmation, waiting approval, retrying failure, unavailable artifact, and conflict fixtures
When used by keyboard and pointer at desktop and mobile viewports
Then state, context, safe actions, error recovery, return navigation, and focus remain reachable without clipping or horizontal overflow

Verification contract

Stage owners run only checks proportional to their paths. The final combined candidate requires backend tests, typecheck, build, behavior-based browser tests, affected-state accessibility, source/SAM frontend parity, and git diff --check; commands/counts/exit codes and sanitized evidence are recorded by Tester.

Do not run or add import, migration, export, restore, or production-data suites. Do not call AWS or external providers. Exact screenshot names may be chosen at Stage B grooming, but assertions must be based on behavior, semantics, focus, and safe content—not pixels, file bytes, incidental prose, fixed timing, or exact history counts.

Lifecycle gates

• Stage A1 Software Engineer candidate in isolated worktree
• Stage A1 Assistant Engineer and Architect PASS
• Stage A1 Tester PASS and PM ACCEPTED; explicit-path Refs #158 checkpoint held off main
• Show signed-in identity and authorized teammates’ Task/Card work #164 Stage A Tester/PM accepted
• Stage A2 implementation and specialist review are complete; final-candidate Tester PASS and PM ACCEPTED recorded; checkpoint held pending
• Restore behavior-based browser coverage and fix canonical UI accessibility races #180 Tester/PM accepted
• Stage B implementation and Designer PASS
• Stage B Tester PASS and PM ACCEPTED; held checkpoint
• Replace the transitional Tasks table with the final cardId schema #166/Gate canonical Card lifecycle on the empty production Cards table #182 cutover, evidence, and cleanup complete; ordinary push deployment restored
• Stage C clean steady-state reconstruction/rebase and sanitized shape preflight
• Final combined Tester PASS and PM ACCEPTED
• Software Engineer explicit-path commit with Closes #158
• Orchestrator local merge and push
• On-Call source/deployed result

Out of scope

• Reopening Converge DataOps onto one canonical frontend #148/Port assistant job lifecycle to the canonical frontend #152/Verify the canonical frontend through normal DataOps deployment #155/Complete canonical frontend deep-link and operator parity #156/Secure and structure runtime-template administration #157, restoring the retired SPA, another router/frontend, broad information-architecture redesign, or compatibility aliases.
• New assistant engines, prompts, skills, plugin discovery, conversational/model mutation tools, or edits to source-system/private-knowledge repositories.
• Publishing, messaging, document commits, workflow dispatch, scheduling, provider writes, real assistant execution, credentials, AWS calls, manual deploy, or IAM/infrastructure expansion.
• Per-user artifact ACLs, new roles, upload/retention redesign, provider integrations, or active-content preview.
• Any migration framework, compatibility read, defaulted version, data backfill, import/export/restore change or test, production data mutation, or destructive cleanup.
• Raw/private operational knowledge, real people/contact data, private links, production identifiers, credentials-adjacent notes, or generated operational artifacts in this public repository.

[alexeygrigorev]

PM GROOMED — dependency blocked

Rewrote #158 as a self-contained P0 bug spec after reviewing the process/PM contract, the full issue, #148/#152/#154/#155/#156/#157 sequencing, the accepted conversational-agent/plugin architecture, and the assistant/artifact/API/canonical-frontend code at exact current main 56a1e2abd3f803172ea78bc9cd511bacc3733c31.

The bounded product decisions are now explicit:

• create persists a draft and never auto-submits;
• draft edit and every lifecycle action are state-gated, confirmed, versioned, and conditionally written;
• failed/rejected Retry and retrying Submit retry are two visible refresh-backed steps;
• every success/failure/reload rehydrates authoritative job, artifact, and event state;
• assistant-output access is provider-aware and same-origin/artifact-ID-mediated, with no stable/signed storageUri in browser-facing JSON, DOM, logs, errors, or evidence;
• enabled operators/admins are allowed, while unauthenticated, disabled, expired, and spoofed identities are denied without mutation or disclosure;
• real API/browser permission, provider, concurrency, partial-failure, reload, accessibility, and responsive scenarios plus exact screenshots are required;
• assistant jobs remain core internal infrastructure, not conversational plugins/model tools, and approval cannot publish, send, dispatch, deploy, or perform an external provider write.

#157 is already shipped on main. #158 must not start while #156's accepted frontend work remains outside main because both own frontend/src/app.js, styles, and canonical Playwright coverage. Resume only after #156 completes Tester/PM/commit/merge-push/source-On-Call integration onto the main containing #157, then create a fresh #158 worktree from that clean converged main. #158 also blocks final #155 deployed acceptance and #148 closure.

Required specialist order is Assistant Engineer, Architect, and Designer on the frozen uncommitted candidate, followed by Tester and PM. needs grooming was removed; the existing appropriate bug/portal/assistant/frontend/backend/testing/data/design/P0 labels were retained. No repository source, assistant execution, external message, deploy, workflow dispatch, or AWS action was performed.

Next owner: Software Engineer after the #156 integration resume condition is recorded.