diff --git a/application/single_app/config.py b/application/single_app/config.py index 53149a72b..49fea69cb 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -101,7 +101,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.261.342" +VERSION = "0.261.343" IS_DEVELOPMENT = is_development_env_enabled() # Opt-out for deployments where App Service Easy Auth is active but the platform diff --git a/application/single_app/functions_orchestration_execution.py b/application/single_app/functions_orchestration_execution.py index 3ee56aa59..bb9fc1323 100644 --- a/application/single_app/functions_orchestration_execution.py +++ b/application/single_app/functions_orchestration_execution.py @@ -1853,6 +1853,36 @@ def _finish(self, result, error): ) return list(self._frames) + def credential_continuation_status(self, item): + """Project the selected step from our acknowledged durable finalization.""" + record = self._final_record + continuation = item.get("continuation") or {} + if record is None or ( + record.get("id") != continuation.get("run_id") + or record.get("id") != self.record["id"] + or record.get("user_id") != item.get("user_id") + or record.get("conversation_id") != item.get("conversation_id") + ): + raise HarnessExecutionError("result_not_ready") + step = next( + (row for row in record.get("execution_steps") or [] + if row["step_id"] == continuation.get("step_id")), + None, + ) + if step is not None: + if step["status"] in {"completed", "partial"}: + return "completed" + if step["status"] in {"failed", "cancelled"}: + return "failed" + wait = step.get("wait") or {} + if step["status"] == "waiting" and wait.get("kind") == "action_credentials" and ( + wait.get("request_id") and wait["request_id"] != item["id"] + ): + return "superseded" + if record.get("status") in {"failed", "cancelled"}: + return "failed" + raise HarnessExecutionError("result_not_ready") + def _preparation_error(self, error): _log_failure( "Headless execution preparation failed.", self.record, error, diff --git a/application/single_app/functions_orchestration_executor.py b/application/single_app/functions_orchestration_executor.py index 1453c0bff..ce7ba9c32 100644 --- a/application/single_app/functions_orchestration_executor.py +++ b/application/single_app/functions_orchestration_executor.py @@ -1237,7 +1237,7 @@ def resume_waiting_dependency_step( if type(wait) is not dict: raise ResultContractError('result_wait_required') if wait.get("kind") == "action_credentials": - from functions_action_credentials import authorize_request, credential_continuation + from functions_action_credentials import authorize_request from functions_orchestration_adapters import _credential_step_continuation if ( @@ -1245,7 +1245,29 @@ def resume_waiting_dependency_step( or step.get("capability_id") not in {"action_invoke", "agent_invoke"} ): raise ResultContractError("result_wait_invalid") - request, _manifest = authorize_request(user_id, wait["request_id"]) + try: + identity = context.agent_execution_identity + if ( + identity is None or identity.user_id != user_id + or identity.conversation_id != context.conversation_id or not callable(identity.bridge) + ): + raise PermissionError("The credential execution identity is unavailable.") + # Catalog authorization uses legacy request helpers, just like action invocation. + with identity.bridge({}): + request, _manifest = authorize_request(user_id, wait["request_id"]) + except Exception as error: + log_event( + f"{_LOG_PREFIX} Credential resume authorization failed.", + level=logging.WARNING, + extra={ + **workflow_log_context( + run_id=context.run_id, conversation_id=context.conversation_id, + step_id=step["step_id"], + ), + "stage": "credential_resume_authorization", "error_type": type(error).__name__, + }, + ) + raise descriptor = _credential_step_continuation(step, context) if any( request["continuation"].get(key) != descriptor.get(key) for key in ("run_id", "step_id", "checkpoint_id") diff --git a/application/single_app/route_backend_action_credentials.py b/application/single_app/route_backend_action_credentials.py index ff8181c20..1a9157b94 100644 --- a/application/single_app/route_backend_action_credentials.py +++ b/application/single_app/route_backend_action_credentials.py @@ -3,6 +3,7 @@ from functools import wraps import hmac +import logging import secrets import uuid from datetime import datetime, timezone @@ -11,7 +12,7 @@ from action_credential_contracts import ActionCredentialError, ActionCredentialsRequired from functions_authentication import get_current_user_id, login_required, user_required -from functions_appinsights import log_event +from functions_appinsights import log_event, workflow_log_context from functions_action_credentials import ( authorize_conversation, authorize_request, credential_continuation, safe_request, submit_credentials, ) @@ -77,11 +78,19 @@ def __init__(self, execution, item, user_id): def execute(self, **kwargs): try: yield from self.execution.execute(**kwargs) - except Exception: - _finish_request(self.user_id, self.item["id"], self.item["continuation_claim"], "failed") - raise - else: - _finish_request(self.user_id, self.item["id"], self.item["continuation_claim"], "completed") + finally: + try: + status = self.execution.credential_continuation_status(self.item) + _finish_request(self.user_id, self.item["id"], self.item["continuation_claim"], status) + except Exception as error: + log_event("[PLUGINS] Credential continuation outcome could not be confirmed.", level=logging.WARNING, extra={ + **workflow_log_context( + run_id=self.item["continuation"]["run_id"], conversation_id=self.item["conversation_id"], + step_id=self.item["continuation"]["step_id"], + ), + "stage": "credential_continuation_finalization", "error_type": type(error).__name__, + }) + raise def close(self): return self.execution.close() diff --git a/application/v2_ui/src/components/chat/CredentialRequestCard.tsx b/application/v2_ui/src/components/chat/CredentialRequestCard.tsx index 360a38329..58903dbd3 100644 --- a/application/v2_ui/src/components/chat/CredentialRequestCard.tsx +++ b/application/v2_ui/src/components/chat/CredentialRequestCard.tsx @@ -7,7 +7,8 @@ import { useBootstrapStore } from '../../stores/bootstrapStore'; import { continueCredentialChat } from '../../stores/chatStore'; import { continueCredentialOrchestration } from '../../lib/orchestrationController'; import { ACTION_CREDENTIAL_EVENT, readCredentialRequest } from '../../lib/actionCredentialEvents'; -import { cancelActionCredentials, clearCredentialSession, fetchCredentialRequests, saveActionCredentials } from '../../lib/actionCredentials'; +import { cancelActionCredentials, clearCredentialSession, fetchActionCredentialRequest, fetchCredentialRequests, saveActionCredentials } from '../../lib/actionCredentials'; +import { fetchOrchestrationRun, isOrchestrationRunWaiting } from '../../lib/orchestration'; import type { CredentialRequest, CredentialSlot, CredentialSubmission } from '../../lib/actionCredentialTypes'; import { ApiError } from '../../lib/apiClient'; @@ -98,6 +99,9 @@ export function CredentialRequestCard({ credentialRequest, onUpdate, onRemove }: const [busy, setBusy] = useState(false); const [error, setError] = useState(''); const [saved, setSaved] = useState(null); + const [needsRefresh, setNeedsRefresh] = useState(false); + const [closed, setClosed] = useState(false); + const working = useRef(false); useEffect(() => () => { controller.current?.abort(); formRef.current?.reset(); @@ -107,13 +111,15 @@ export function CredentialRequestCard({ credentialRequest, onUpdate, onRemove }: const continueWork = async (request: CredentialRequest) => { if (request.run_id) await continueCredentialOrchestration(request); else await continueCredentialChat(request); - onRemove(); + if (!controller.current?.signal.aborted) onRemove(); }; const submit = async () => { - if (busy) return; + if (working.current || needsRefresh || closed) return; + working.current = true; setBusy(true); setError(''); controller.current = new AbortController(); + let confirmedSave = ready; try { if (ready) { await continueWork(ready); @@ -144,20 +150,70 @@ export function CredentialRequestCard({ credentialRequest, onUpdate, onRemove }: } finally { slots.length = 0; } + if (controller.current.signal.aborted) return; setSaved(updated); + confirmedSave = updated; await continueWork(updated); } catch { if (!controller.current?.signal.aborted) { - setError(ready || saved - ? 'Credentials are saved, but the action could not continue. Reload requests to check its status.' - : 'Saving or continuing failed. Reload requests to check whether credentials were saved before entering them again.'); + setNeedsRefresh(true); + setError(confirmedSave + ? 'Your credentials are saved, but continuation could not be confirmed. Refresh the saved request before trying again.' + : 'Saving could not be confirmed. Refresh the saved request before entering credentials again.'); } } finally { + working.current = false; if (!controller.current?.signal.aborted) setBusy(false); } }; + const refreshSavedRequest = async () => { + if (working.current) return; + working.current = true; + setBusy(true); + controller.current = new AbortController(); + try { + const current = await fetchActionCredentialRequest(saved || credentialRequest, controller.current.signal); + let waiting = true; + if (['pending', 'ready'].includes(current.status) && current.run_id) { + const run = await fetchOrchestrationRun(current.run_id, { + conversationId: current.conversation_id, signal: controller.current.signal, + }); + if (!run || run.run_id !== current.run_id || run.conversation_id !== current.conversation_id || + (current.turn_id && current.turn_id !== run.turn_id)) { + throw new Error('The saved work could not be confirmed.'); + } + waiting = isOrchestrationRunWaiting(run); + } + if (controller.current.signal.aborted) return; + if (current.status === 'pending' && waiting) { + setSaved(null); + setClosed(false); + setNeedsRefresh(false); + setError(''); + onUpdate(current); + } else { + setSaved(current.status === 'ready' || ready ? current : null); + const canContinue = current.status === 'ready' && waiting; + setClosed(!canContinue); + setNeedsRefresh(!canContinue); + setError(canContinue ? '' + : current.run_id + ? 'This work is no longer waiting for these credentials. Review run details for saved progress and recovery options.' + : 'This work is no longer waiting for these credentials. Review the conversation before starting new work.'); + } + } catch { + if (!controller.current.signal.aborted) { + setNeedsRefresh(true); + setError('The saved request could not be refreshed. Reload this conversation before trying again.'); + } + } finally { + working.current = false; + if (!controller.current.signal.aborted) setBusy(false); + } + }; const cancel = async () => { - if (busy) return; + if (working.current || needsRefresh || closed) return; + working.current = true; setBusy(true); setError(''); formRef.current?.reset(); @@ -168,6 +224,7 @@ export function CredentialRequestCard({ credentialRequest, onUpdate, onRemove }: } catch { if (!controller.current.signal.aborted) setError('The credential request could not be cancelled. Reload requests before trying again.'); } finally { + working.current = false; if (!controller.current.signal.aborted) setBusy(false); } }; @@ -186,7 +243,7 @@ export function CredentialRequestCard({ credentialRequest, onUpdate, onRemove }:
{ event.preventDefault(); void submit(); }} aria-busy={busy} autoComplete="off">
- {!ready ? credentialRequest.requirements.map((requirement) => { + {!ready && !needsRefresh ? credentialRequest.requirements.map((requirement) => { const prefix = `${id}-${requirement.slot}`; const candidates = credentialRequest.identities.filter((item) => item.auth_type === requirement.auth_type); const selectedIdentity = candidates.find((item) => item.id === identities[requirement.slot]); @@ -230,15 +287,21 @@ export function CredentialRequestCard({ credentialRequest, onUpdate, onRemove }: required autoComplete="new-password" spellCheck={false} /> : null}
; - }) :

Your personal identities are saved. Continue the blocked action when ready.

} + }) : ready ?

+ {needsRefresh || closed ? 'Your personal identities are saved.' + : 'Your personal identities are saved. Continue the blocked action when ready.'} +

: null} {error ?

{error}

: null}
- {busy ? 'Working...' : ready ? 'Continue' : 'Save and continue'} - void cancel()}>Cancel blocked work + {!closed ? <> + {busy ? 'Working...' : ready ? 'Continue' : 'Save and continue'} + void cancel()}>Cancel blocked work + : null} + {needsRefresh || saved ? void refreshSavedRequest()}> + Refresh saved request + : null}
- {saved && saved.etag !== credentialRequest.etag && !busy - ? onUpdate(saved)}>Refresh saved request : null} ; } diff --git a/application/v2_ui/src/lib/actionCredentials.ts b/application/v2_ui/src/lib/actionCredentials.ts index 58fd4e885..e2d92b7de 100644 --- a/application/v2_ui/src/lib/actionCredentials.ts +++ b/application/v2_ui/src/lib/actionCredentials.ts @@ -18,21 +18,29 @@ function captureCsrf(result: { csrf_token?: unknown }, epoch: number): void { async function credentialHeaders(credentialRequest: CredentialRequest, signal?: AbortSignal): Promise> { if (!csrfToken) { - const epoch = sessionEpoch; - const result = await request<{ credential_request: unknown; csrf_token?: unknown }>( - `/api/action-credentials/${encodeURIComponent(credentialRequest.id)}`, - { cache: 'no-store', signal }, - ); - const current = readCredentialRequest(result.credential_request); - if (current.id !== credentialRequest.id || current.conversation_id !== credentialRequest.conversation_id) { - throw new Error('The credential request no longer belongs to this conversation.'); - } - captureCsrf(result, epoch); + await fetchActionCredentialRequest(credentialRequest, signal); } if (!csrfToken) throw new Error('The credential request could not be authorized. Reload this conversation.'); return { 'X-CSRFToken': csrfToken }; } +export async function fetchActionCredentialRequest( + credentialRequest: CredentialRequest, signal?: AbortSignal, +): Promise { + const epoch = sessionEpoch; + const result = await request<{ credential_request: unknown; csrf_token?: unknown }>( + `/api/action-credentials/${encodeURIComponent(credentialRequest.id)}`, + { cache: 'no-store', signal }, + ); + const current = readCredentialRequest(result.credential_request); + if (current.id !== credentialRequest.id || current.conversation_id !== credentialRequest.conversation_id || + current.run_id !== credentialRequest.run_id || current.turn_id !== credentialRequest.turn_id) { + throw new Error('The credential request no longer belongs to this work.'); + } + captureCsrf(result, epoch); + return current; +} + export async function fetchCredentialRequests(conversationId: string, signal?: AbortSignal): Promise { const epoch = sessionEpoch; const result = await request<{ requests: unknown[]; csrf_token?: unknown }>( diff --git a/docs/explanation/fixes/YAMCS_CREDENTIAL_CONTINUATION_FIX.md b/docs/explanation/fixes/YAMCS_CREDENTIAL_CONTINUATION_FIX.md new file mode 100644 index 000000000..129a50d00 --- /dev/null +++ b/docs/explanation/fixes/YAMCS_CREDENTIAL_CONTINUATION_FIX.md @@ -0,0 +1,161 @@ +# YAMCS credential continuation fix + +Fixed/Implemented in version: **0.261.343**, recorded in +`application/single_app/config.py`. + +## Issue and root cause + +The React V2 personal-credential card could successfully save a YAMCS identity +and then report that saving or continuing failed. The detached orchestration +worker reauthorized a saved credential wait before entering the existing +execution-identity bridge. The real action catalog resolver reads Flask session +identity, which is unavailable on that worker without the bridge. + +There were two related state problems. The card's asynchronous submission +handler inspected React state captured before the save, misclassifying a +continuation failure as an unconfirmed save. The backend credential wrapper +marked an interaction completed whenever execution returned normally, although +the executor can persist a failed outcome and return failure frames normally. + +The production investigation confirmed a successful credential submission, +an accepted SSE continuation, and a subsequent worker RuntimeError with a saved +failed execution. The missing Flask request context was reproduced locally +through the real resolver; production telemetry did not retain the exact stack. +Successful credential storage does not establish that YAMCS accepted those +credentials. + +## Technical changes + +`functions_orchestration_executor.py` validates the captured user and conversation, +then enters `agent_execution_identity.bridge({})` only for saved-wait +reauthorization. Direct actions and agent-owned waits share this branch. Current +conversation ownership, action governance, credential fingerprint, continuation +descriptor, and execution guards remain enforced. The entire worker does not +inherit a mutable browser request. + +`functions_orchestration_execution.py` exposes the selected credential step's +outcome from its acknowledged durable finalization. The wrapper in +`route_backend_action_credentials.py` uses that outcome rather than SSE content, +HTTP status, or normal return: + +| Selected work | Credential interaction | +| --- | --- | +| Completed or partial, even with a sibling waiting or a later unrelated failure | Completed | +| Failed or cancelled before completion | Failed | +| Waiting on a new credential request | Superseded | +| No confirmed durable outcome | Remains continuing; not automatically retryable | + +Conditional writes still require the exact continuation claim and current ETag. +A stale worker cannot close a newer claim or overwrite a renewed pending request. + +`CredentialRequestCard.tsx` tracks confirmed saves within the active submission. +After an uncertain save or continuation failure it disables repeat submission +until **Refresh saved request** reads authoritative state. +`actionCredentials.ts` validates that refresh responses belong to the same +request, conversation, run, and turn. Orchestration requests also require a +waiting saved run before continuation is offered. Terminal, superseded, active, +and unconfirmed work does not gain an automatic retry. + +Secret inputs and submission buffers are cleared, duplicate submissions are +guarded synchronously, and navigation aborts stale card work. No credentials are +added to chat stores or browser storage. Diagnostics record static stages +`credential_resume_authorization` and `credential_continuation_finalization`, +exception types, and hashed correlation identifiers, never provider error text, +credential values, or request bodies. + +## Validation + +The regression coverage uses real application modules, detached workers, isolated +Flask identity bridges, catalog authorization, leases, checkpoints, and SDK-shaped +storage doubles. External storage, model, and provider I/O is local test data. + +- `functional_tests/test_action_credentials_orchestration_continuation.py` + covers submit-to-worker resume, concurrent identity isolation, missing and + mismatched identities, revoked or changed actions, selected-step outcomes, + renewed challenges, stale claims, unknown outcomes, private-error sentinels, + preserved deadlines, and no replay of completed siblings. +- `functional_tests/test_action_credentials_bootstrap.py`, + `test_action_credentials_sk_continuation.py`, and + `test_action_personal_credentials.py` cover initialization, retained SK tools, + ordinary chat continuation, and the real YAMCS SDK's local transport boundary. +- `functional_tests/test_v2_action_personal_credentials.mjs` covers safe + projection, authoritative refresh, CSRF, revision fences, and stream contracts. +- `ui_tests/test_v2_action_credentials.py` exercises actual React cards through + HTTP conflicts, failing SSE, transport loss, uncertain saves, failed refresh, + terminal or unconfirmed runs, latest-revision continuation, duplicate submits, + unmount, private-input cleanup, and desktop/mobile layout. + +All 79 tests across the four focused credential Python suites pass, as do the +22 credential browser tests, frontend transport checks, TypeScript/build, +route-policy checks, documentation coverage/quality checks, and 28 orchestration +Cosmos response-contract tests. Desktop light and mobile dark recovery states +were inspected using the production styles. + +The broader orchestration continuation run produced 120 passes and one failure: +`test_native_wait_reopens_original_handle_once_and_preserves_child_and_fingerprint` +in `functional_tests/test_orchestration_continuation.py`. That native-compute +fixture reports `result_producer_unavailable` before creating its expected +`compute` wait. The same test fails identically when both changed execution +modules are loaded from unmodified HEAD source in memory. It is a pre-existing +baseline failure, not repaired by this credential-boundary change. + +Validation used session-local Python dependency overrides and restored frontend +dependencies; global packages were not changed. These offline results are not +proof of live YAMCS credential acceptance or a full pinned-environment CI run. + +## PR #1774 Cosmos contract review + +The initial `cosmos-contracts` check failed during inventory validation, before +running its runtime contracts. The credential-continuation change left five +storage-owner reviews stale and introduced three unreviewed test-local +`read_item` overrides. This was a PR-caused review-inventory omission, not an SDK +response failure or transient CI infrastructure problem. + +All five storage function fingerprints still match the parent commit and its +reviewed inventory. Adding `credential_continuation_status` changed their shared +module fingerprint, which correctly requires renewed consumer review. +`functional_tests/cosmos_response_boundaries.json` was regenerated with +`scripts/check_cosmos_response_boundaries.py --refresh`, then reviewed against +the actual mapping consumers and SDK-backed runtime regressions. Saved delivery +metadata and regeneration-thread reviews also name their specific existing +runtime tests. + +The three new overrides are explicitly classified as `fault-injection`, with +their real runtime evidence attached. Two inject authorization read failures; +the third forces overlapping worker reads but returns the original SDK +`CosmosDict` unchanged. The shared containers still default to SDK responses, +and partition routing, ETags, conditional writes, authorization, and private-error +checks remain intact. No legacy exemption, CI change, weakened assertion, or +production storage change is involved. + +Review validation used the session-local Python environment and blocked external +network access before test collection: + +- `python scripts\check_cosmos_response_boundaries.py --run-contracts`: + **1,003 passed, 244 subtests passed**, with **1,059 owners, 407 doubles, and + zero review gaps**. +- The credential-continuation and orchestration Cosmos-response suites: + **60 passed**. +- The existing delivery-refresh, regeneration-thread, and conversation-source + runtime evidence: **27 passed**. +- `python functional_tests\test_docs_site_quality.py`: **6/6 checks passed**. + +The first complete local run passed 1,002 tests but failed the SDK-pin assertion +because that environment inherited Cosmos SDK 4.16.3. Restoring the application's +`azure-cosmos==4.9.0` pin in the session-local environment resolved that validation +setup failure; global dependencies and the assertion were not changed. + +This follow-up changes only the reviewed inventory and this documentation; +the application version remains **0.261.343**. + +## Impact and recovery limitations + +The fix applies to the shared credential-continuation boundary, not only YAMCS. +It does not redesign authentication, credential storage, or unrelated telemetry. +It adds no route, setting, browser dependency, or provider. + +Deploying the code does not repair a previously terminal attempt. Keep its +identity and saved run intact, refresh their state, and use only recovery +actions the server authorizes. Do not reset live credential or run records, +automatically resend the question, or replay an uncertain external operation. +No deployment, live retry, or production data repair is part of this change. diff --git a/docs/guides/personal-action-credentials.md b/docs/guides/personal-action-credentials.md index 6166faa8f..b7176275e 100644 --- a/docs/guides/personal-action-credentials.md +++ b/docs/guides/personal-action-credentials.md @@ -4,7 +4,7 @@ title: "Use your own credentials with a shared action" description: "Provide a private personal identity when a shared Yamcs action needs your sign-in." section: "Guides" audience: user -version: "0.261.337" +version: "0.261.343" --- ## What this does @@ -17,6 +17,7 @@ Yamcs. Fixed/Implemented in version: **0.261.333**, recorded in `application/single_app/config.py`. +Continuation failure feedback and status recovery were fixed in **0.261.343**. ## When a credential card appears @@ -59,6 +60,25 @@ empty identity. If continuation cannot be confirmed, reload requests and review the saved work before trying again. Do not resend the original question merely to complete the blocked action. +## If saving or continuing cannot be confirmed + +A successful save and a successful action are different outcomes. If the card +says **Your credentials are saved**, do not enter them again because continuation +failed. Select **Refresh saved request** to read its current status from the +server. SimpleChat also checks the saved run for orchestration work. + +Continue becomes available only if the request is still ready and its run is +still waiting. It uses the refreshed request revision, not a second credential +submission. If the work is running, finished, failed, or superseded, the card +does not offer another continuation. Review run details for saved progress and +the recovery actions the server permits. For ordinary agent chat, review the +conversation instead. + +If saving itself cannot be confirmed, refresh before entering credentials again: +the server may already have saved them even if the response was lost. If refresh +fails too, reload the conversation. Neither refresh nor reload automatically +replays the action or sends the original question again. + ## When credentials stop working A confirmed authentication rejection can ask for a replacement identity. diff --git a/functional_tests/cosmos_response_boundaries.json b/functional_tests/cosmos_response_boundaries.json index 25d130308..db8d1dafd 100644 --- a/functional_tests/cosmos_response_boundaries.json +++ b/functional_tests/cosmos_response_boundaries.json @@ -12405,7 +12405,7 @@ ], "queries": [], "batches": [], - "module_sha256": "13e3d0820f1601ae9a10a16aaa111abdf4571ad8705c8b70502859449a77465d" + "module_sha256": "0f62fc46056d332bd82c4a687f51096c9bc55cb537931091242f99b44077c625" }, "classification": "sdk-compatible", "contract": "Partitioned turn/source point mappings are reauthorized, checked for visibility and converted by normalize_history_message before strict saved-snapshot validation. Full source messages are resolved only after fingerprint checks; history disablement and revoked shared access cannot broaden reads.", @@ -12428,14 +12428,15 @@ ], "queries": [], "batches": [], - "module_sha256": "13e3d0820f1601ae9a10a16aaa111abdf4571ad8705c8b70502859449a77465d" + "module_sha256": "0f62fc46056d332bd82c4a687f51096c9bc55cb537931091242f99b44077c625" }, "classification": "sdk-compatible", "contract": "The previous assistant point mapping is checked for conversation/run ownership; model and reasoning fields are projected into plain dictionaries, without passing the SDK wrapper to an exact-dictionary contract.", "evidence_scope": "owner-runtime", "tests": [ "functional_tests/test_action_credentials_orchestration_continuation.py", - "functional_tests/test_orchestration_cosmos_response_contract.py" + "functional_tests/test_orchestration_cosmos_response_contract.py", + "functional_tests/test_orchestration_harness_execution.py::test_delivery_refresh_reuses_real_saved_results_without_any_execution" ] }, "application/single_app/functions_orchestration_execution.py::HarnessExecution._regeneration_thread": { @@ -12450,14 +12451,15 @@ ], "queries": [], "batches": [], - "module_sha256": "13e3d0820f1601ae9a10a16aaa111abdf4571ad8705c8b70502859449a77465d" + "module_sha256": "0f62fc46056d332bd82c4a687f51096c9bc55cb537931091242f99b44077c625" }, "classification": "sdk-compatible", "contract": "The question point mapping supplies role and nested plain thread metadata; only a deepcopy of the validated nested thread dictionary is returned.", "evidence_scope": "owner-runtime", "tests": [ "functional_tests/test_action_credentials_orchestration_continuation.py", - "functional_tests/test_orchestration_cosmos_response_contract.py" + "functional_tests/test_orchestration_cosmos_response_contract.py", + "functional_tests/test_orchestration_message_regeneration.py::test_regeneration_forces_review_despite_auto_and_disabled_approval_override" ] }, "application/single_app/functions_orchestration_execution.py::HarnessExecution._touch_conversation": { @@ -12472,7 +12474,7 @@ ], "queries": [], "batches": [], - "module_sha256": "13e3d0820f1601ae9a10a16aaa111abdf4571ad8705c8b70502859449a77465d" + "module_sha256": "0f62fc46056d332bd82c4a687f51096c9bc55cb537931091242f99b44077c625" }, "classification": "sdk-compatible", "contract": "A normalized owned conversation retains its _etag for IfNotModified replacement; the returned SDK mapping is consumed only by the Mapping-compatible cache invalidator. Lost-CAS retries retain current ownership.", @@ -12495,7 +12497,7 @@ ], "queries": [], "batches": [], - "module_sha256": "13e3d0820f1601ae9a10a16aaa111abdf4571ad8705c8b70502859449a77465d" + "module_sha256": "0f62fc46056d332bd82c4a687f51096c9bc55cb537931091242f99b44077c625" }, "classification": "sdk-compatible", "contract": "The published message point mapping is projected with items() into a metadata-free fingerprint and consumed by Mapping-compatible mirroring. Durable outcome telemetry reads only finalized scalar fields; strict task/result descriptors and saved-message confirmation remain unchanged.", @@ -32410,6 +32412,51 @@ "tests": [ "functional_tests/test_orchestration_conversation_sources.py::test_shared_membership_is_rechecked_before_exact_message_reads" ] + }, + "functional_tests/test_action_credentials_orchestration_continuation.py::test_owned_continue_consumes_only_selected_wait_and_never_replays_completed_steps.continue_wait::point-seam-1": { + "boundary": { + "sha256": "00949cc48e418c46407d3ceaa70f63b01d4e155796e299e750a4f73d9b87daeb", + "methods": [ + "read_item" + ], + "kind": "seam", + "imports_sha256": "082a12e0daa2974ba679862ce9899e4826c9bccd396b63e902310a8abe18acf0" + }, + "classification": "fault-injection", + "contract": "The authorization_failure case injects a private read error only on the detached worker; browser reads still delegate to the SDK-default AtomicMemoryContainer. Real continuation must fail the selected interaction durably, retain completed siblings and avoid both action invocation and private-error disclosure.", + "tests": [ + "functional_tests/test_action_credentials_orchestration_continuation.py::test_owned_continue_consumes_only_selected_wait_and_never_replays_completed_steps" + ] + }, + "functional_tests/test_action_credentials_orchestration_continuation.py::test_saved_wait_authorization_uses_real_isolated_identity::point-seam-1": { + "boundary": { + "sha256": "e76497bd85e13ddd72c040c71cfe2425c56c3bc03b0a91c014800ca76bc17e40", + "methods": [ + "read_item" + ], + "kind": "seam", + "imports_sha256": "082a12e0daa2974ba679862ce9899e4826c9bccd396b63e902310a8abe18acf0" + }, + "classification": "fault-injection", + "contract": "The storage_failure case raises a private read error instead of returning an easy document. Real action/agent saved-wait authorization propagates storage uncertainty, restores the Flask context and logs only the safe failure stage/type and hashed run identity; other cases retain SDK-default point responses.", + "tests": [ + "functional_tests/test_action_credentials_orchestration_continuation.py::test_saved_wait_authorization_uses_real_isolated_identity" + ] + }, + "functional_tests/test_action_credentials_orchestration_continuation.py::test_saved_wait_authorization_uses_real_isolated_identity::point-seam-2": { + "boundary": { + "sha256": "e9be874d64606980d74fbcc54ec3ba5e11ba1245251d44210c5c6c0dc5dddb51", + "methods": [ + "read_item" + ], + "kind": "seam", + "imports_sha256": "082a12e0daa2974ba679862ce9899e4826c9bccd396b63e902310a8abe18acf0" + }, + "classification": "fault-injection", + "contract": "The allowed case injects a two-worker barrier and records each bridged user/conversation, then returns the original AtomicMemoryContainer.read_item CosmosDict unchanged. Real action/agent authorization must keep overlapping Flask identities isolated and restore both worker contexts; partition routing and ETags remain those of the SDK-default backing container.", + "tests": [ + "functional_tests/test_action_credentials_orchestration_continuation.py::test_saved_wait_authorization_uses_real_isolated_identity" + ] } } } diff --git a/functional_tests/test_action_credentials_orchestration_continuation.py b/functional_tests/test_action_credentials_orchestration_continuation.py index 22421eed6..867165d7a 100644 --- a/functional_tests/test_action_credentials_orchestration_continuation.py +++ b/functional_tests/test_action_credentials_orchestration_continuation.py @@ -1,22 +1,55 @@ # test_action_credentials_orchestration_continuation.py """Real orchestration leases/checkpoints and independent credential waits. -Version: 0.261.337 +Version: 0.261.343 Implemented in: 0.261.333 +Worker authorization and durable outcome regressions added in: 0.261.343 Only external action/model/storage I/O is doubled. Retained completed steps, conditional continuation admission and immutable execution guards remain real. """ from copy import deepcopy +from concurrent.futures import ThreadPoolExecutor +from dataclasses import replace from datetime import datetime, timedelta, timezone import importlib -from flask import Blueprint, Flask +from threading import Barrier, current_thread, main_thread +from types import SimpleNamespace +from flask import Blueprint, Flask, g, has_request_context, session +import pytest from test_orchestration_continuation import harness, initialized_continuation from test_orchestration_action_chart_delivery import ACTION, _enable_action, _normalize, _store from test_action_personal_credentials import CredentialContainer, manifest from test_support.orchestration_harness_execution import compose_step, input_binding +from test_support.orchestration_revisions import AtomicMemoryContainer + + +def configure_real_credentials(harness, monkeypatch, raw): + """Keep runtime, catalog and conversation authorization real; double storage I/O.""" + credentials = importlib.import_module("functions_action_credentials") + continuation = importlib.import_module("functions_action_credential_continuation") + runtime = importlib.import_module("functions_action_credential_runtime") + chats = importlib.import_module("route_backend_chats") + monkeypatch.setattr(chats, "cosmos_conversations_container", harness.conversations) + actions = AtomicMemoryContainer("id") + actions.create_item(dict(raw)) + monkeypatch.setattr(harness.config, "cosmos_global_actions_container", actions) + monkeypatch.setattr(importlib.import_module("functions_settings"), "get_settings", + lambda: deepcopy(harness.settings)) + monkeypatch.setattr(credentials, "_services", credentials._services) + monkeypatch.setattr(continuation, "_services", continuation._services) + runtime.configure_action_credential_runtime() + identity = {"id": "fixture-identity", "_etag": "identity-etag", "name": "Mission", + "auth": {"auth_type": "api_key"}, "usage_contexts": ["action"], + "supported_source_types": ["action"], "provider": "generic"} + monkeypatch.setattr(credentials, "_services", replace( + credentials.credential_services(), + read_identity=lambda scope, user, identity_id: deepcopy(identity), + list_identities=lambda scope, user: [deepcopy(identity)], + )) + return credentials def test_human_wait_budget_is_bounded_and_original_deadline_is_immutable(): @@ -38,7 +71,10 @@ def test_human_wait_budget_is_bounded_and_original_deadline_is_immutable(): assert exhausted == datetime.fromisoformat(original) + timedelta(days=1) -def test_owned_continue_consumes_only_selected_wait_and_never_replays_completed_steps(harness, monkeypatch): +@pytest.mark.parametrize("outcome", [ + "success", "action_failure", "renewed_challenge", "downstream_failure", "authorization_failure", +]) +def test_owned_continue_consumes_only_selected_wait_and_never_replays_completed_steps(harness, monkeypatch, outcome): state = importlib.import_module("functions_action_credential_state") credentials = importlib.import_module("functions_action_credentials") adapters = importlib.import_module("functions_orchestration_adapters") @@ -50,20 +86,29 @@ def test_owned_continue_consumes_only_selected_wait_and_never_replays_completed_ original_resolver = executor._dependency_adapter raw = manifest() fingerprint = contracts.credential_fingerprint(raw) - monkeypatch.setattr(credentials, "authorized_manifest", lambda user_id, reference: deepcopy(raw)) - monkeypatch.setattr(credentials, "safe_identities", lambda user_id, requirements: []) - monkeypatch.setattr(credentials, "authorize_conversation", lambda user_id, conversation_id: - harness.bootstrap.read_owned_conversation(user_id, conversation_id)) + configure_real_credentials(harness, monkeypatch, raw) waits = {} def action(step, context, **kwargs): descriptor = adapters._credential_step_continuation(step, context) pending = state.request_for( - "owner", "conversation-1", ACTION["action_ref"], fingerprint, {"service": "api_key"}, + "owner", "conversation-1", credentials.manifest_reference(raw), fingerprint, {"service": "api_key"}, reason="setup_required", continuation=descriptor, ) if step["step_id"] not in waits else state.read("owner", waits[step["step_id"]]) waits[step["step_id"]] = pending["id"] if pending["status"] == "continuing": + if outcome == "action_failure": + return harness.schema.build_step_result( + status="failed", failure=harness.schema.build_failure("step_failed"), + ) + if outcome == "renewed_challenge": + pending = state.request_for( + "owner", "conversation-1", credentials.manifest_reference(raw), fingerprint, + {"service": "api_key"}, reason="credentials_rejected", continuation=descriptor, + ) + return adapters._credential_wait_result( + contracts.ActionCredentialsRequired(credentials.safe_request(pending, raw)), kwargs.get("emit"), + ) return original_action(step, context, **kwargs) return adapters._credential_wait_result( contracts.ActionCredentialsRequired(credentials.safe_request(pending, raw)), kwargs.get("emit"), @@ -117,39 +162,245 @@ def action(step, context, **kwargs): with client.session_transaction() as browser: browser["user"] = {"oid": "owner", "roles": ["User"]} - def continue_wait(name): + def continue_wait(name, expected="completed"): item = state.read("owner", waits[name]) - ready = state.replace("owner", item, {"status": "ready"}, item["_etag"]) - path = f"/api/action-credentials/{ready['id']}" + path = f"/api/action-credentials/{item['id']}" info = client.get(path) assert info.status_code == 200 + submitted = client.post( + path + "/submit", headers={"X-CSRFToken": info.json["csrf_token"]}, + json={"expected_etag": item["_etag"], "slots": [{"slot": "service", "identity_id": "fixture-identity"}]}, + ) + assert submitted.status_code == 200, submitted.json + ready = submitted.json["credential_request"] + if outcome == "authorization_failure": + actions = harness.config.cosmos_global_actions_container + original_read = actions.read_item + def unavailable(*args, **kwargs): + if current_thread() is not main_thread(): + raise RuntimeError("PRIVATE_AUTHORIZATION_SENTINEL") + return original_read(*args, **kwargs) + monkeypatch.setattr(actions, "read_item", unavailable) response = client.post( path + "/continue", headers={"X-CSRFToken": info.json["csrf_token"]}, - json={"expected_etag": ready["_etag"]}, + json={"expected_etag": ready["etag"]}, ) assert response.status_code == 200, response.get_data(as_text=True) frames = response.get_data(as_text=True) + assert "PRIVATE_AUTHORIZATION_SENTINEL" not in frames assert response.mimetype == "text/event-stream" completed = state.read("owner", ready["id"]) - assert completed["status"] == "completed" + assert completed["status"] == expected repeated = client.post( path + "/continue", headers={"X-CSRFToken": info.json["csrf_token"]}, - json={"expected_etag": ready["_etag"]}, + json={"expected_etag": ready["etag"]}, ) assert repeated.status_code == 409 return frames - continue_wait("one") + continue_wait("one", { + "action_failure": "failed", "authorization_failure": "failed", "renewed_challenge": "superseded", + }.get(outcome, "completed")) midway = harness.read() + if outcome == "authorization_failure": + assert midway["status"] == "failed" + assert completed_calls == [] + assert midway["task_results"]["before"] == original_before + return + if outcome in {"action_failure", "renewed_challenge"}: + assert midway["status"] == "waiting" + if outcome == "action_failure": + selected = next(row for row in midway["execution_steps"] if row["step_id"] == "one") + assert selected["status"] == "failed" + assert completed_calls == [] + assert midway["task_results"]["before"] == original_before + assert midway["pending_results"]["two"]["request_id"] == waits["two"] + if outcome == "renewed_challenge": + renewed = state.read("owner", midway["pending_results"]["one"]["request_id"]) + assert renewed["id"] != waits["one"] + assert renewed["status"] == "pending" + return assert midway["status"] == "waiting" assert completed_calls == ["one"] assert midway["task_results"]["before"] == original_before assert midway["pending_results"]["two"]["request_id"] == waits["two"] assert midway["execution_deadline_at"] == original_deadline + if outcome == "downstream_failure": + def fail_answer(*args, **kwargs): + return harness.schema.build_step_result(status="failed", failure=harness.schema.build_failure("step_failed")) + monkeypatch.setattr(executor, "_dependency_adapter", lambda capability_id: + action if capability_id == "action_invoke" else fail_answer) continue_wait("two") done = harness.read() - assert done["status"] == "completed" + assert done["status"] == ("failed" if outcome == "downstream_failure" else "completed") assert completed_calls == ["one", "two"] assert done["task_results"]["before"] == original_before assert done["execution_deadline_at"] == original_deadline - assert len(harness.model_calls) == 2 + assert len(harness.model_calls) == (1 if outcome == "downstream_failure" else 2) + + +@pytest.mark.parametrize("capability", ["action_invoke", "agent_invoke"]) +@pytest.mark.parametrize("access", [ + "allowed", "missing_identity", "wrong_user", "wrong_conversation", "revoked", "changed", "storage_failure", +]) +def test_saved_wait_authorization_uses_real_isolated_identity(harness, monkeypatch, capability, access): + state = importlib.import_module("functions_action_credential_state") + contracts = importlib.import_module("action_credential_contracts") + adapters = importlib.import_module("functions_orchestration_adapters") + executor = importlib.import_module("functions_orchestration_executor") + identity_owner = importlib.import_module("agent_execution_context") + monkeypatch.setattr(state, "_container", CredentialContainer("user_id")) + harness.settings["enable_semantic_kernel"] = True + raw = manifest() + credentials = configure_real_credentials(harness, monkeypatch, raw) + events = [] + monkeypatch.setattr(executor, "log_event", lambda message, **kwargs: events.append((message, kwargs))) + app = Flask("credential-worker-isolation") + app.secret_key = "local-test-only" + + def prepare(user, conversation): + context = SimpleNamespace( + user_id=user, conversation_id=conversation, run_id=f"run-{user}", turn_id="turn", + pending_results={}, task_results={"step": None}, + ) + step = {"step_id": "step", "capability_id": capability} + descriptor = adapters._credential_step_continuation(step, context) + item = state.request_for(user, conversation, credentials.manifest_reference(raw), + contracts.credential_fingerprint(raw), {"service": "api_key"}, + reason="setup_required", continuation=descriptor) + context.pending_results["step"] = {"kind": "action_credentials", "request_id": item["id"]} + with app.test_request_context(): + session["user"] = {"oid": user, "roles": ["User"]} + context.agent_execution_identity = identity_owner.capture_execution_identity(user, conversation) + return step, context + + step, context = prepare("owner", "conversation-1") + if access == "missing_identity": + context.agent_execution_identity = None + elif access in {"wrong_user", "wrong_conversation"}: + context.agent_execution_identity = replace( + context.agent_execution_identity, + **({"user_id": "other"} if access == "wrong_user" else {"conversation_id": "foreign"}), + ) + elif access in {"revoked", "changed"}: + actions = harness.config.cosmos_global_actions_container + stored = actions.read_item("mission", "mission") + stored.update({"is_enabled": False} if access == "revoked" else {"endpoint": "https://changed.invalid"}) + actions.upsert_item(stored) + elif access == "storage_failure": + def unavailable(*args, **kwargs): + raise RuntimeError("PRIVATE_STORAGE_SENTINEL") + monkeypatch.setattr(harness.config.cosmos_global_actions_container, "read_item", unavailable) + + def resume(selected, current): + assert not has_request_context() + try: + return executor.resume_waiting_dependency_step( + selected, current, settings=harness.settings, user_id=current.user_id, + input_fingerprint="fixture", + ) + finally: + assert not has_request_context() + + with ThreadPoolExecutor(max_workers=2) as workers: + if access == "allowed": + harness.conversations.create_item({"id": "conversation-2", "user_id": "other"}) + other_step, other = prepare("other", "conversation-2") + actions = harness.config.cosmos_global_actions_container + original_read = actions.read_item + barrier, observed = Barrier(2), [] + def read_with_overlapping_contexts(*args, **kwargs): + observed.append((session["user"]["oid"], g.conversation_id)) + barrier.wait(timeout=10) + return original_read(*args, **kwargs) + monkeypatch.setattr(actions, "read_item", read_with_overlapping_contexts) + futures = [workers.submit(resume, step, context), workers.submit(resume, other_step, other)] + results = [future.result() for future in futures] + assert all(result["status"] == "waiting" for result in results) + assert sorted(observed) == [("other", "conversation-2"), ("owner", "conversation-1")] + else: + failure = (contracts.ActionCredentialError if access == "changed" + else RuntimeError if access == "storage_failure" else PermissionError) + future = workers.submit(resume, step, context) + with pytest.raises(failure): + future.result() + assert events[-1][1]["extra"]["stage"] == "credential_resume_authorization" + assert events[-1][1]["extra"]["error_type"] == failure.__name__ + assert len(events[-1][1]["extra"]["run_id_hash"]) == 64 + assert "run-owner" not in repr(events) + assert "PRIVATE_STORAGE_SENTINEL" not in repr(events) + + +@pytest.mark.parametrize("status,step_status,wait_id,expected", [ + ("completed", "completed", None, "completed"), + ("failed", "completed", None, "completed"), + ("waiting", "partial", None, "completed"), + ("waiting", "failed", None, "failed"), + ("cancelled", "waiting", "request", "failed"), + ("failed", "waiting", "request", "failed"), + ("waiting", "waiting", "new-request", "superseded"), + ("waiting", "waiting", "request", None), + ("running", "running", None, None), +]) +def test_credential_outcome_uses_selected_durable_step(initialized_continuation, status, step_status, wait_id, expected): + execution = importlib.import_module("functions_orchestration_execution") + record = {"id": "run", "user_id": "owner", "conversation_id": "conversation"} + runner = execution.HarnessExecution(record, None) + item = {**record, "id": "request", "continuation": {"run_id": "run", "step_id": "selected"}} + row = {"step_id": "selected", "status": step_status} + if wait_id: + row["wait"] = {"kind": "action_credentials", "request_id": wait_id} + runner._final_record = {**record, "status": status, "execution_steps": [row]} + if expected is None: + with pytest.raises(execution.HarnessExecutionError): + runner.credential_continuation_status(item) + else: + result = runner.credential_continuation_status(item) + assert result == expected + runner._final_record = None + with pytest.raises(execution.HarnessExecutionError): + runner.credential_continuation_status(item) + + +@pytest.mark.parametrize("current_status,claim", [("continuing", "new-claim"), ("pending", "old-claim")]) +def test_credential_finalization_cannot_close_newer_interaction(harness, monkeypatch, current_status, claim): + routes = importlib.import_module("route_backend_action_credentials") + state = importlib.import_module("functions_action_credential_state") + monkeypatch.setattr(state, "_container", CredentialContainer("user_id")) + item = state.create("owner", { + "id": "request", "record_type": "credential_request", "conversation_id": "conversation-1", + "status": current_status, "continuation_claim": claim, + }) + routes._finish_request("owner", item["id"], "old-claim", "completed") + current = state.read("owner", item["id"]) + assert current["status"] == current_status + assert current["_etag"] == item["_etag"] + + +def test_unconfirmed_execution_keeps_claim_and_logs_no_private_text(harness, monkeypatch): + routes = importlib.import_module("route_backend_action_credentials") + state = importlib.import_module("functions_action_credential_state") + monkeypatch.setattr(state, "_container", CredentialContainer("user_id")) + item = state.create("owner", { + "id": "request", "record_type": "credential_request", "conversation_id": "conversation-1", + "status": "continuing", "continuation_claim": "claim", + "continuation": {"run_id": "run", "step_id": "selected"}, + }) + events = [] + monkeypatch.setattr(routes, "log_event", lambda message, **kwargs: events.append((message, kwargs))) + + class UnknownExecution: + def execute(self): + return [] + + def credential_continuation_status(self, _item): + raise RuntimeError("PRIVATE_PASSWORD_SENTINEL") + + wrapped = routes._CredentialExecution(UnknownExecution(), item, "owner") + with pytest.raises(RuntimeError): + list(wrapped.execute()) + current = state.read("owner", item["id"]) + assert current["status"] == "continuing" + assert current["_etag"] == item["_etag"] + assert events[0][1]["extra"]["stage"] == "credential_continuation_finalization" + assert "PRIVATE_PASSWORD_SENTINEL" not in repr(events) diff --git a/functional_tests/test_v2_action_personal_credentials.mjs b/functional_tests/test_v2_action_personal_credentials.mjs index d6e9c55f0..88cd4d96d 100644 --- a/functional_tests/test_v2_action_personal_credentials.mjs +++ b/functional_tests/test_v2_action_personal_credentials.mjs @@ -1,6 +1,7 @@ // test_v2_action_personal_credentials.mjs -// Version: 0.261.337 +// Version: 0.261.343 // Implemented in: 0.261.333 +// Continuation recovery regressions added in: 0.261.343 // Exercises real authoring, safe DTO projection, credential transport, and stream waiting. import assert from 'node:assert/strict'; @@ -15,7 +16,7 @@ const { buildEditorWrite, EDITOR_SECRET_MASK } = await import('../application/v2 const { buildActionConnectionPayload } = await import('../application/v2_ui/src/lib/workspaceActionServices.ts'); const { readCredentialRequest, ACTION_CREDENTIAL_EVENT } = await import('../application/v2_ui/src/lib/actionCredentialEvents.ts'); const { consumeCredentialChatResponse } = await import('../application/v2_ui/src/lib/sse.ts'); -const { saveActionCredentials, openCredentialContinuation } = await import('../application/v2_ui/src/lib/actionCredentials.ts'); +const { saveActionCredentials, openCredentialContinuation, fetchActionCredentialRequest } = await import('../application/v2_ui/src/lib/actionCredentials.ts'); const { runOrchestration } = await import('../application/v2_ui/src/lib/orchestration.ts'); const read = (name) => JSON.parse(fs.readFileSync(new URL( `../application/single_app/static/json/schemas/${name}`, import.meta.url, @@ -123,6 +124,16 @@ try { assert.deepEqual(JSON.parse(continued.options.body), { expected_etag: 'etag-2' }); assert.equal(continued.options.headers['X-CSRFToken'], 'fixture-csrf-token'); assert.equal(continued.options.body.includes('private'), false, 'continuation sends only its fence, never secrets or a new question'); + const refreshed = await fetchActionCredentialRequest(safe); + assert.equal(refreshed.etag, 'etag-2'); + assert.equal(calls.at(-1).options.cache, 'no-store'); + assert.equal(calls.at(-1).options.method, 'GET', 'refresh must read the server, not submit credentials'); + for (const change of [{ id: 'foreign' }, { conversation_id: 'foreign' }, { run_id: 'foreign' }, { turn_id: 'foreign' }]) { + globalThis.fetch = async () => new Response(JSON.stringify({ + credential_request: { ...safe, ...change }, csrf_token: 'wrong-work', + }), { headers: { 'Content-Type': 'application/json' } }); + await assert.rejects(fetchActionCredentialRequest(safe), /no longer belongs/); + } let waiting = 0; let runDone = 0; const runRequest = { ...safe, run_id: 'run-1', turn_id: 'turn-1', status: 'ready' }; diff --git a/ui_tests/test_v2_action_credentials.py b/ui_tests/test_v2_action_credentials.py index 540528dad..7f24be036 100644 --- a/ui_tests/test_v2_action_credentials.py +++ b/ui_tests/test_v2_action_credentials.py @@ -1,8 +1,9 @@ # test_v2_action_credentials.py """ Functional UI test for private credential cards in React V2. -Version: 0.261.337 +Version: 0.261.343 Implemented in: 0.261.333 +Continuation recovery regressions added in: 0.261.343 Exercises the real card and shared chat continuation, with closed local API fixtures. Secrets must be submitted only to the credential endpoint, disappear on submission and @@ -53,6 +54,10 @@ return new Response(JSON.stringify({credential_request: {...request, status: 'ready', etag: 'etag-2'}}), {headers: {'Content-Type': 'application/json'}}); } + if (path.endsWith('/api/action-credentials/' + request.id)) { + return new Response(JSON.stringify({credential_request: {...request, status: 'ready', etag: 'etag-3'}, + csrf_token: 'fixture-csrf-token'}), {headers: {'Content-Type': 'application/json'}}); + } if (path.endsWith('/cancel')) { return new Response(JSON.stringify({credential_request: {...request, status: 'cancelled'}}), {headers: {'Content-Type': 'application/json'}}); @@ -252,29 +257,51 @@ def test_ready_request_resumes_same_orchestration_without_new_run(): pytest.skip(str(exc)) -def test_failed_continuation_clears_secrets_and_cancels_with_saved_revision(): +@pytest.mark.parametrize("failure_kind", ["conflict", "error_sse", "transport"]) +def test_failed_continuation_clears_secrets_and_refreshes_saved_revision(failure_kind): errors = [] try: with hb.harness_page(collect_errors=errors) as page: page.evaluate(SETUP, REQUEST) - page.evaluate(r"""() => { + page.evaluate(r"""(failureKind) => { const originalFetch = window.fetch; window.fetch = async (url, options = {}) => { if (String(url).endsWith('/continue')) { window.__credentialCalls.push({path: String(url), body: JSON.parse(options.body)}); + if (failureKind === 'transport') throw new TypeError('Private provider failure sentinel'); + if (failureKind === 'error_sse') return new Response( + 'data: ' + JSON.stringify({error: 'The request could not be completed.'}) + '\n\n', + {headers: {'Content-Type': 'text/event-stream'}}); return new Response(JSON.stringify({error: 'request_changed'}), { status: 409, headers: {'Content-Type': 'application/json'} }); } return originalFetch(url, options); }; - }""") + }""", failure_kind) card = page.get_by_role("region", name="Personal action credentials") card.wait_for() card.get_by_label("Username", exact=True).fill("rejected-user-sentinel") card.get_by_label("Password", exact=True).fill("rejected-secret-sentinel") card.get_by_role("button", name="Save and continue", exact=True).click() card.get_by_role("alert").wait_for() + assert card.get_by_role("alert").inner_text() == ( + "Your credentials are saved, but continuation could not be confirmed. " + "Refresh the saved request before trying again." + ) + assert card.get_by_role("button", name="Continue", exact=True).is_disabled() + assert card.get_by_role("button", name="Cancel blocked work", exact=True).is_disabled() + screenshots = os.environ.get("SIMPLECHAT_CREDENTIAL_SCREENSHOT_DIR") + if screenshots and failure_kind == "conflict": + root = Path(__file__).resolve().parents[1] + css = next((root / "application" / "single_app" / "static" / "v2" / "assets").glob("*.css")) + page.add_style_tag(path=str(css)) + for width, theme in ((1440, "light"), (390, "dark")): + page.set_viewport_size({"width": width, "height": 900}) + page.evaluate("(dark) => document.documentElement.classList.toggle('dark', dark)", theme == "dark") + overflow = card.evaluate("(node) => node.scrollWidth > node.clientWidth") + assert not overflow + card.screenshot(path=str(Path(screenshots) / f"credential-recovery-{theme}-{width}.png")) assert card.locator("input").count() == 0 assert "rejected-secret-sentinel" not in card.inner_text() state = page.evaluate("""() => JSON.stringify({ @@ -284,16 +311,183 @@ def test_failed_continuation_clears_secrets_and_cancels_with_saved_revision(): })""") assert "rejected-user-sentinel" not in state assert "rejected-secret-sentinel" not in state + assert "Private provider failure sentinel" not in card.inner_text() + card.get_by_role("button", name="Refresh saved request", exact=True).click() + page.wait_for_function('() => !document.querySelector(\'section[aria-label="Personal action credentials"] [role="alert"]\')') card.get_by_role("button", name="Cancel blocked work", exact=True).click() page.wait_for_function("() => window.__credentialCalls.some(call => call.path.endsWith('/cancel'))") cancellation = page.evaluate("window.__credentialCalls.find(call => call.path.endsWith('/cancel')).body") - assert cancellation == {"expected_etag": "etag-2"} + assert cancellation == {"expected_etag": "etag-3"} page.wait_for_function('() => !document.querySelector(\'section[aria-label="Personal action credentials"]\')') assert not errors, errors except hb.HarnessUnavailable as exc: pytest.skip(str(exc)) +@pytest.mark.parametrize("request_status,run_status", [ + ("failed", "failed"), ("completed", "completed"), ("superseded", "waiting"), + ("continuing", "running"), ("pending", "failed"), ("ready", "failed"), ("ready", "waiting"), + ("ready", "unconfirmed"), +]) +def test_refresh_uses_authoritative_credential_and_run_state(request_status, run_status): + errors = [] + plan = make_plan("conversation-1", "turn-1") + plan["status"] = "waiting" + plan["approval"]["state"] = "approved" + ready = {**REQUEST, "status": "ready", "run_id": plan["run_id"], "turn_id": plan["turn_id"]} + try: + with hb.harness_page(collect_errors=errors) as page: + page.evaluate(SETUP, ready) + page.evaluate(r"""({plan, request, requestStatus, runStatus}) => { + const originalFetch = window.fetch; + window.__refreshed = false; + window.fetch = async (url, options = {}) => { + const path = String(url).split('?')[0]; + if (path.endsWith('/api/action-credentials/' + request.id)) { + window.__refreshed = true; + window.__credentialCalls.push({path, body: null}); + return new Response(JSON.stringify({credential_request: {...request, + status: requestStatus, etag: 'etag-4'}}), + {headers: {'Content-Type': 'application/json'}}); + } + if (path.endsWith('/runs/' + plan.run_id)) { + window.__credentialCalls.push({path: String(url), body: null}); + if (window.__refreshed && runStatus === 'unconfirmed') return new Response('{}', + {headers: {'Content-Type': 'application/json'}}); + return new Response(JSON.stringify({run: { + run_id: plan.run_id, turn_id: plan.turn_id, conversation_id: plan.conversation_id, + status: window.__refreshed ? runStatus : 'waiting', + outcome: window.__refreshed ? runStatus : 'waiting', plan + }}), {headers: {'Content-Type': 'application/json'}}); + } + if (path.endsWith('/continue')) { + window.__credentialCalls.push({path, body: JSON.parse(options.body)}); + return new Response('{}', {status: 409, headers: {'Content-Type': 'application/json'}}); + } + return originalFetch(url, options); + }; + }""", {"plan": plan, "request": ready, "requestStatus": request_status, "runStatus": run_status}) + card = page.get_by_role("region", name="Personal action credentials") + card.wait_for() + card.get_by_role("button", name="Continue", exact=True).click() + card.get_by_role("alert").wait_for() + card.get_by_role("button", name="Refresh saved request", exact=True).click() + page.wait_for_function("() => window.__refreshed && document.querySelector('form')?.getAttribute('aria-busy') === 'false'") + if request_status == "ready" and run_status == "waiting": + assert card.get_by_role("button", name="Continue", exact=True).is_enabled() + assert card.get_by_role("alert").count() == 0 + card.get_by_role("button", name="Continue", exact=True).click() + card.get_by_role("alert").wait_for() + continued = page.evaluate("window.__credentialCalls.filter(call => call.path.endsWith('/continue'))") + assert [call["body"]["expected_etag"] for call in continued] == ["etag-1", "etag-4"] + elif run_status == "unconfirmed": + assert card.get_by_role("button", name="Continue", exact=True).is_disabled() + assert "could not be refreshed" in card.get_by_role("alert").inner_text() + else: + assert card.get_by_role("button", name="Continue", exact=True).count() == 0 + assert card.get_by_role("button", name="Cancel blocked work", exact=True).count() == 0 + assert "Review run details" in card.get_by_role("alert").inner_text() + assert card.locator('input[type="password"]').count() == 0 + calls = page.evaluate("window.__credentialCalls") + assert not any(call["path"].endswith("/submit") for call in calls) + if request_status == "ready": + assert any("conversation_id=conversation-1" in call["path"] for call in calls) + assert not errors, errors + except hb.HarnessUnavailable as exc: + pytest.skip(str(exc)) + + +@pytest.mark.parametrize("refresh_result", ["ready", "unavailable", "cancelled"]) +def test_unconfirmed_save_requires_real_refresh_without_resubmission(refresh_result): + errors = [] + try: + with hb.harness_page(collect_errors=errors) as page: + page.evaluate(SETUP, REQUEST) + page.evaluate(r"""({request, refreshResult}) => { + const originalFetch = window.fetch; + window.fetch = async (url, options = {}) => { + const path = String(url); + if (path.endsWith('/submit')) { + window.__credentialCalls.push({path, body: JSON.parse(options.body)}); + throw new TypeError('Connection lost after saving'); + } + if (refreshResult !== 'ready' && path.endsWith('/api/action-credentials/credential-1')) { + window.__credentialCalls.push({path, body: null}); + if (refreshResult === 'unavailable') throw new TypeError('PRIVATE_SENTINEL'); + return new Response(JSON.stringify({credential_request: {...request, + status: refreshResult, etag: 'etag-3'}}), {headers: {'Content-Type': 'application/json'}}); + } + return originalFetch(url, options); + }; + }""", {"request": REQUEST, "refreshResult": refresh_result}) + card = page.get_by_role("region", name="Personal action credentials") + card.wait_for() + card.get_by_label("Personal identity", exact=True).select_option("identity-1") + card.get_by_role("button", name="Save and continue", exact=True).click() + card.get_by_role("alert").wait_for() + assert card.get_by_role("alert").inner_text() == ( + "Saving could not be confirmed. Refresh the saved request before entering credentials again." + ) + assert card.get_by_role("button", name="Save and continue", exact=True).is_disabled() + assert card.locator("input").count() == 0 + card.get_by_role("button", name="Refresh saved request", exact=True).click() + if refresh_result == "unavailable": + card.get_by_text("The saved request could not be refreshed. Reload this conversation before trying again.").wait_for() + assert card.get_by_role("button", name="Save and continue", exact=True).is_disabled() + assert "PRIVATE_SENTINEL" not in card.inner_text() + elif refresh_result == "cancelled": + card.get_by_text("This work is no longer waiting for these credentials. Review the conversation before starting new work.").wait_for() + assert card.get_by_role("button", name="Save and continue", exact=True).count() == 0 + assert card.get_by_role("status").count() == 0 + else: + card.get_by_role("button", name="Continue", exact=True).click() + page.wait_for_function("() => window.__credentialCalls.some(call => call.path.endsWith('/continue'))") + continued = page.evaluate("window.__credentialCalls.find(call => call.path.endsWith('/continue')).body") + assert continued == {"expected_etag": "etag-3"} + calls = page.evaluate("window.__credentialCalls") + assert len([call for call in calls if call["path"].endswith("/submit")]) == 1 + assert not errors, errors + except hb.HarnessUnavailable as exc: + pytest.skip(str(exc)) + + +def test_duplicate_submit_and_unmount_do_not_start_another_continuation(): + errors = [] + try: + with hb.harness_page(collect_errors=errors) as page: + page.evaluate(SETUP, REQUEST) + page.evaluate(r"""(request) => { + const originalFetch = window.fetch; + window.fetch = async (url, options = {}) => { + if (String(url).endsWith('/submit')) { + window.__credentialCalls.push({path: String(url), body: JSON.parse(options.body)}); + return new Promise(resolve => {window.__finishSave = () => resolve(new Response( + JSON.stringify({credential_request: {...request, status: 'ready', etag: 'etag-2'}}), + {headers: {'Content-Type': 'application/json'}}));}); + } + return originalFetch(url, options); + }; + }""", REQUEST) + card = page.get_by_role("region", name="Personal action credentials") + card.wait_for() + card.get_by_label("Personal identity", exact=True).select_option("identity-1") + card.locator("form").evaluate("""form => { + form.dispatchEvent(new Event('submit', {bubbles: true, cancelable: true})); + form.dispatchEvent(new Event('submit', {bubbles: true, cancelable: true})); + }""") + page.wait_for_function("() => typeof window.__finishSave === 'function'") + page.evaluate("""() => window.OrchHarness.mount('mount-a', 'CredentialRequestCards', + {conversationId: 'conversation-2'})""") + page.wait_for_function('() => !document.querySelector(\'section[aria-label="Personal action credentials"]\')') + page.evaluate("async () => {window.__finishSave(); await new Promise(resolve => setTimeout(resolve, 0));}") + calls = page.evaluate("window.__credentialCalls") + assert len([call for call in calls if call["path"].endswith("/submit")]) == 1 + assert not any(call["path"].endswith("/continue") for call in calls) + assert not errors, errors + except hb.HarnessUnavailable as exc: + pytest.skip(str(exc)) + + def test_replace_existing_identity_uses_its_revision_fence(): errors = [] try: