UN-4168 [DEV] Bring UN-4108-zid up to main - #2345

…'s creator (#2274)

* UN-3853 [FIX] Attribute platform-key-created resources to the key's creator

A platform API key authenticates as a service account, and every resource
create path granted the OWNER membership row to that machine identity. Service
accounts are filtered out of every owner surface (HasMembersMixin), so such a
resource ended up with no human owner: invisible to its creator in list views,
manageable only through the org-admin fallback, and rendered in "Owned By" as a
synthetic @platform.internal address dressed up as a colleague.

Record the key's creator as owner instead — the same successor
delete_api_user_for_key already hands ownership to when a key is deleted, now
applied at creation rather than only at deletion. The service account loses
nothing: permission classes and for_user() short-circuit on is_service_account.

Where no human can be named (the key's creator has since been deleted), the
resource stays deliberately ownerless and the table labels it "Platform key"
rather than naming a machine.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JHnDZZWGhsevUdwgMyR2ai

* UN-3853 [FIX] Name the real owner on the deployment and pipeline cards too

The ticket asks for Owned By to be correct on every resource type. The API
deployment and ETL pipeline card views were still wrong: OwnerFieldRow read
created_by_email only, and their serializers never exposed owner_emails — so
those cards named the audit creator, which on a platform-key create is the
service account. The backend fix alone could not reach them.

Expose owner_emails on both serializers (their querysets already prefetch
memberships__user, so it costs no extra query), and move the owner-label rule
into one resolveOwnerDisplay helper shared by the table and the cards. The two
had already drifted on both the source field and the "Me" rule — the card said
"Me" to any owner, which is the co-owner bug the table's comment warns about.
Cards now match the table: "Me" tracks the displayed owner.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JHnDZZWGhsevUdwgMyR2ai

* UN-3853 [FIX] Cover platform-key resource ownership with tests

`owner_user_for` had no coverage. Adds the resolver's own branches (normal
user early-returns with no query, service account resolves to the key's
creator, a deleted creator or a missing key leaves the resource ownerless)
and one case per OSS resource that grants an OWNER row on create: workflow,
prompt studio, ETL pipeline, API deployment, connector, adapter.

The resource cases drive the real URLconf and middleware chain with a key
minted in the test, so the service-account swap that caused the bug is
exercised rather than simulated. Verified by mutation: reverting each call
site to the raw request user fails the matching case.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BAfubrF8kR2rKewMSjXagg

* UN-3853 [FIX] Do not grant ownership to a departed organization member

F1: owner_user_for granted an OWNER membership row to the key's creator
without checking they are still a member of the key's organization.
_is_resource_owner grants on any surviving OWNER row with no live-membership
check, which is why cleanup_user_org_access purges those rows on departure;
minting a new one afterwards reopened that rejoin backdoor for the key's
lifetime. AddOwnerSerializer already refuses both non-members and service
accounts, so these sites were the only OWNER writes without the gate.

The check reads _base_manager: the default manager is org-scoped by
UserContext, which is unset outside a request, so the plain manager would
return empty and silently strip every resource of its owner.

F6: the fallback arms now log, naming the key id (not the key itself).
F7, F8: the docstring claimed service accounts are filtered from "every"
owner surface and that the residual resource "stays deliberately ownerless".
Neither held -- owner_memberships() and is_owner() apply no such filter, and
a real OWNER row is written that the last-owner guard counts as live.

Tests: the fixtures created a key creator with no OrganizationMember row, a
user who could never have created a key through IsOrganizationAdmin. They
now build the member production guarantees, and two cases pin the new guard
(creator left the org; creator belongs to a different org). Both fail when
the guard is neutered.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tm83ikxv1euWWQQGLMBcjC

* UN-3853 [FIX] Show the owner on a freshly created deployment, and cover the gaps

F5: the create response is a seven-field summary carrying none of the owner
fields the list renders, and the modal spliced it straight into the table, so
a just-created API deployment showed no owner until the next fetch. It now
refetches; the response is still handed to the code modal, which is the only
thing that needs the API key. The edit path already merged over the existing
row, so it kept its owner fields and is unchanged.

F3: reverting the seventh grant site (create_tool_from_import_data, reached by
project-transfer/ and sync-prompts/ with create_copy) left the whole backend
suite green -- six cases covered seven sites. A case now pins it, and fails
when that site is reverted.

F9: the comment called the "Platform key" branch reachable only via a deleted
creator. With no backfill it is also every resource created through a key
before this change; the accurate statement already sits on the fallback above.

F11: resolveOwnerDisplay decides the Owned By cell on every list and card and
had no test -- the only suite rendering it asserts nothing about that cell.
Six cases now cover its four branches; they fail when the platform-key
suppression or the "Me" rule is broken.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tm83ikxv1euWWQQGLMBcjC

* UN-3853 [FIX] Backfill existing platform-key resources and document the field

F10: resources created through a platform key before this change hold an OWNER
row naming the service account, which every owner surface filters out, so they
show no owner and only an org admin can manage them. Nothing moved them -- the
only re-pointer runs from delete_api_user_for_key. A data migration now hands
each to the same successor owner_user_for picks, under the same membership
rule: a creator who has left the org is skipped rather than handed a fresh
OWNER row, which would reopen the rejoin backdoor. Where the creator already
holds a row on that resource the stronger role is kept and the service
account's is dropped, so the unique triple is never violated.

Verified on a disposable database seeded with all three cases -- live creator,
departed creator, and a creator already holding a VIEWER row: forward applies,
reverse unapplies, and a re-apply is a no-op.

F4: APIDeploymentSummary subclasses the list serializer and declares no fields
of its own, so owner_emails reached the published platform-key contract with no
description. Stripping it there would only desynchronise the spec from the
response, and the UI reads the same endpoint, so the field stays and the
docstring now documents what a caller receives. Spec regenerated.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tm83ikxv1euWWQQGLMBcjC

* UN-3853 [FIX] Move the owner grant onto HasMembersMixin

F12: resolving the acting user was a rule every create site had to remember,
applied at seven sites here and four in cloud. That is how the agentic
sample-project site came to be missed. HasMembersMixin is already on every one
of these models, so grant_owner() lives there and each site calls it -- the
rule now has one enforcement point, and all seven ownership cases fail when it
is broken, instead of one case per site.

Also drops seven imports of platform_api from the business apps, for an
operation permissions/ owns; the resolver import is now lazy and in one place.

Out of scope, checked: sharing_helpers grants VIEWER rather than create-time
OWNER, and platform_admin's two onboarding grants take the human from signup
rather than request.user, so no bearer session reaches them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tm83ikxv1euWWQQGLMBcjC

* UN-3853 [FIX] Apply the membership rule on the key-deletion and backfill paths

Found by adversarial verification of the previous commits.

F21: closing the create path left delete_api_user_for_key still handing every
OWNER row to the key's creator with no membership check, so deleting a key
re-granted a departed creator the rows the create path had just refused --
the same rejoin backdoor, one step later. The membership question is now
live_key_creator(), asked by both paths instead of stated twice, and
transfer_ownership already short-circuits on None. Two cases pin it.

F22: migration 0006 declared no edge to UN-2202's *_absorb_shared_users
backfills, which write an OWNER row from created_by -- the service account on
a key-created resource. Six of the eight sorted before 0006 by alphabetical
accident and workflow_v2.0022 sorted after it, so workflows would have been
backfilled ownerless and left that way. The edges are now declared;
workflow_v2.0022 moves from plan index 289 to 183, immediately before 0006.

F23: the create-modal refresh called the fetch with its own defaults, so it
dropped any active search and reset to page 1 -- and a never-run deployment
sorts into the tail, so it was usually not on the page it landed on. It now
uses handleListRefresh, which the hook documents for exactly this and which
the sibling delete and co-owner callbacks already use.

Also corrects two comments this branch introduced: the owner-display pointer
named a fallback that states something else, and the lazy-import note named a
cycle that does not exist (the real constraint is import-time model loading).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tm83ikxv1euWWQQGLMBcjC

* UN-3853 [FIX] Name the backfill's model bindings like the rest of the tree

Sonar flagged all three `apps.get_model` bindings in the backfill (python:S117).
`0005_add_reconciliation_task` already uses the `*_model` form, so this matches
the migration the repo most recently reviewed rather than the older PascalCase
one in `0004_pg_periodic_tasks`. The quoted model names are untouched -- those
are lookup keys, not identifiers.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BAfubrF8kR2rKewMSjXagg

* UN-3853 [FIX] Cut the comment prose down to what stays true

The comments had grown into mechanism retellings and change history -- why a
site was missed, what the two views used to disagree on, which surface filters
what. That rots as soon as the code moves and costs every later reader.

Keeps the contract (what a function returns, why a lazy import, why
_base_manager) and drops the narration. No code changed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BAfubrF8kR2rKewMSjXagg

* UN-3853 [FIX] Keep the service account authorized and the audit trail intact

Review findings from #2274.

Moving the OWNER row off the service account broke two gates that authorize by
OWNER membership and have no service-account bypass, so a workflow provisioned
through a platform key failed its own execution: `validate_adapter_access` and
`IsWorkflowOwnerOrShared.has_permission`. Both now short-circuit on a service
account, matching every other access surface.

On key deletion the audit half and the ownership half are now separate.
`created_by`/`modified_by` follow the key's creator even if they have left the
org -- deleting the account is SET_NULL on those FKs, and a null `created_by`
breaks `CustomTool.delete()`. The OWNER row still goes only to a live member.

Also: eager-load `by_prompt_studio_tool` so `owner_emails` does not fan out,
write `modified_at` in the backfill (historical models skip BaseModel.save),
and return to page 1 after creating a deployment, since never-run rows sort
last and the new one could land off-page.

Three regression tests, each verified by reverting its fix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BAfubrF8kR2rKewMSjXagg

* UN-3853 [FIX] Drop the now-unused list refresh handler

Rewiring the create modal to the page-1 refresh left `handleListRefresh`
destructured but unread here. The hook still exports it and Workflows.jsx
still uses it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BAfubrF8kR2rKewMSjXagg

* UN-3853 [FIX] Share the platform-key ownership repair for cloud-only apps

tenant_account_v2's backfill migration can't depend on cloud-only apps'
absorb_shared_users migrations, so their service-account OWNER rows could
land after the repair and never get revisited. Extract the repair into
_membership_backfill so a cloud migration can import and re-run it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* UN-3853 [FIX] Cover the third ownership gate, and stop jumping to page 1

Re-review findings from #2274.

`IsFrictionLessAdapterDelete` is a third gate that authorizes by OWNER
membership with no service-account bypass, so a full_access key could no longer
delete an adapter it had just created. Every sibling in the file has the
bypass, including `IsFrictionLessAdapter` directly above it.

Swept the rest rather than wait for a fourth: all 13 `_is_resource_owner` call
sites across both repos now either carry the bypass or are unreachable by a
service account -- `prompt_studio_helper._adapter_accessible_by` is guarded by
an `is_service_account` early return at its only caller.

The page-1 refresh is reverted. The ordering is `last_run_time DESC NULLS
LAST`, so a never-run deployment sorts to the END -- page 1 is the wrong
target, and jumping there also threw the user off the page the row was
actually on. `handleListRefresh` at least keeps them there. Making the new row
visible needs an ordering change, which is a product decision, not this PR.

Regression test for the adapter delete gate, verified by reverting the bypass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BAfubrF8kR2rKewMSjXagg

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

… shared with a user (#2273)

* UN-2868 [FIX] Restrict workflow connector and tool changes to owners and org admins

Workflow sub-resources were never gated when the sharing model landed.
tool_instance_v2 was fixed; endpoint_v2 was missed, so any user a workflow
was shared with could change its destination folder, database table or
connector.

Adds a reusable WorkflowOwnerMutationMixin next to is_workflow_mutator and
applies it to WorkflowEndpointViewSet. Sharing -- direct, group or org-wide
-- now grants read only; owners, co-owners, org admins and service accounts
may still write.

The UI now says so up front instead of failing on save: a shared user sees a
read-only notice and greyed controls in the connector modal, tool settings,
and the Prompt Studio project selector, with no Save button to press.

The connector modal's Save now also flushes the HITL plugin's rules. It
previously lit up for rule changes it could not save, then closed as if it
had saved them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016eB6bLmieWVwUnmYH6H5WZ

* UN-2868 [FIX] Show the Prompt Studio project name to shared users instead of its ID

The workflow builder resolved the project name from exportedTools, which
holds only the viewer's own exported projects. On a shared workflow the
lookup missed and fell through to the raw function name, so shared users saw
an ID where the owner saw a name.

The tool instance already carries the display name (ToolInstanceSerializer
sets it from the tool's properties), so fall back to that before falling back
to the ID. The ID remains the last resort for a tool the registry can no
longer resolve.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016eB6bLmieWVwUnmYH6H5WZ

* UN-2868 [FIX] Do not close the connector modal on a failed or partial save

Addresses review feedback plus two defects found alongside it.

- perform_create skipped authorization entirely when the payload carried no
  workflow, then saved anyway. It now fails closed: every viewset using this
  mixin has a required workflow field, so a payload without one cannot be
  authorised.
- handleValidateAndSubmit swallowed its own error, so a failed endpoint write
  still fell through to the HITL write and "Save and Close" dismissed the
  modal. Both writes now report success and the modal stays open on failure.
- The HITL write only runs when its form is actually dirty; otherwise every
  connector save would have written a rule row too.
- A view-only user can no longer be asked whether to save unsaved changes on
  close. They cannot have made any.
- ReadOnlyNotice used defaultProps, which React 19 ignores on function
  components, so its default message never applied. Uses a default parameter.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016eB6bLmieWVwUnmYH6H5WZ

* UN-2868 [FIX] Refuse form submits from view-only users, not just clicks

The read-only styling suppresses pointer events but leaves fields focusable,
so a keyboard user could tab into one and press Enter. That fired a request
the backend rejects, producing exactly the confusing error this ticket is
about.

Both RJSF forms now refuse to submit when the workflow is not editable, so
the request is never made.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016eB6bLmieWVwUnmYH6H5WZ

* UN-2868 [FIX] Hide edit and delete actions on resources shared with the user

Sharing grants read only, but every resource list still offered Edit, Share,
Delete and the enable/disable toggle to the people it was shared with. The
backend refused them; the UI did not say so.

All eight shareable resources render through two shared widgets, so the row
actions are gated in one place each -- ResourceTable covers workflows, Prompt
Studio, connectors, adapters, agentic projects and lookups; CardActionBox
covers pipelines and API deployments.

The two card kebabs mix read and write actions, so those are filtered per
page: Manage Keys, Notifications and Clear File History go, while View Logs,
File History, Sync Now, Code Snippets and Download Postman stay. Running and
watching a shared pipeline is still allowed -- that is what sharing is for.

The rule itself now lives in one helper, canEditResource, which
useWorkflowCanEdit also delegates to.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016eB6bLmieWVwUnmYH6H5WZ

* UN-2868 [FIX] Expose is_owner on the Prompt Studio editor payload

The Prompt Studio editor is served by CustomToolSerializer, which never
carried is_owner -- only the list serializer did. Without it the editor cannot
tell a shared user from an owner, so its edit controls cannot be gated.

canEditResource now treats a payload that has not arrived yet as editable.
The backend refuses the write either way, and the alternative flashes a
read-only view at the resource's own owner while the request is in flight.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016eB6bLmieWVwUnmYH6H5WZ

* UN-2868 [FIX] Keep Share available to shared users; gate only edit and delete

Sharing onward is allowed for someone a resource was shared with: they may
pass access to a group they belong to, or to a user in the same organisation.
ShareAuthorizationService enforces both rules per axis, which is why the
share endpoint sits at IsOwnerOrSharedUserOrSharedToOrg rather than IsOwner.

The previous commit hid the Share button along with Edit and Delete. Only the
latter two should go.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016eB6bLmieWVwUnmYH6H5WZ

* UN-2868 [FIX] Make Prompt Studio settings read-only for shared users

Settings hold the project's LLM profiles and adapter selections -- the
credential-bearing part. A shared user can read them but not change them: the
panel gets the read-only notice and its controls are inert.

Prompts are deliberately untouched. Editing, running and deleting prompts is
what a project is shared for; only the settings panel is restricted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016eB6bLmieWVwUnmYH6H5WZ

* UN-2868 [FIX] Restrict the gate to edit and delete only

The scope is settings and deletion, nothing else. The pipeline and API
deployment cards had also lost their enable/disable toggle, Manage Keys,
Notifications and Clear File History for shared users, which goes further
than intended.

Both card configs are reverted. Only the Edit and Delete controls in the two
shared list widgets stay gated; Share, the toggle and every kebab action are
available again.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016eB6bLmieWVwUnmYH6H5WZ

* UN-2868 [FIX] Show Edit and Delete disabled rather than hiding them

Hiding the two controls left a shared user with no idea they existed or why
they were missing. They now stay on screen, greyed out, with a tooltip
reading "Only the owner can change this". Same treatment in both list widgets
so every resource looks the same.

The rename pencil beside a project title follows the same rule: ToolNavBar
takes an editTitleDisabled prop, and Prompt Studio passes it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016eB6bLmieWVwUnmYH6H5WZ

* UN-2868 [FIX] Report one outcome when the connector save also writes rules

The connector write showed "Configuration saved successfully" before the
rule write ran. A rule failure after it left a green toast on screen next
to a red one, reading as though everything had landed.

The connector success message is now suppressed when a rule write follows,
and the rule write reports the outcome for both.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TaTtTMaDriVZgw6BR8HZ4G

* UN-2868 [FIX] Prefill the workflow rename form and limit shared-user locks to owner-only actions

Four defects found while testing the shared-user gates.

Workflow rename modal opened blank for the owner. `Form.Item` controls its
child and injects a value from the form store, so the `defaultValue` on the
Input never reached the DOM; the OK button also started disabled and only
flipped on change, so the owner had to retype both fields to save. Seed the
form via `initialValues` and derive the button's initial state from the props.

Workflow builder header offered its rename pencil to shared users. `ToolNavBar`
already takes `editTitleDisabled` and Prompt Studio and Agentic already pass
it; `MenuLayout` was the one screen that did not.

Prompt Studio settings were locked wholesale for shared users, which also
blocked adding an LLM profile of their own. The backend only refuses editing
and deleting an existing profile (`IsParentToolOwner`) -- create, copy, set
default and every other tab are open to a shared user. Move the lock onto the
two buttons that are actually refused and drop the modal-wide notice. The rows
are built in an effect, so `canEdit` joins its deps: it starts true while
`details` loads and would otherwise leave the buttons stale-enabled.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UwnxaoVTsxbzufUixPGFQ9

* UN-2868 [FIX] Make Prompt Studio prompts and settings read-only for shared users

Shared access is meant to grant read and run only, but Prompt Studio still
saved prompt and settings writes from a shared user, and three toggles
offered actions the backend already refuses.

Prompts. `PromptAcesssToUser` admitted viewers and group-shared users on
every method, so their edits and deletes went through; shared access is now
limited to SAFE_METHODS. `reorder_prompts` is routed without a pk, so DRF ran
no object check at all and any org member could reorder any project's
prompts -- it now resolves the prompt and checks it.

Settings. Every tab in the modal writes to the project row, so it is
owner-only again, LLM profiles included. `update`, `partial_update` and
`make_profile_default` move to `IsOwner`, which also closes project rename.
`IsParentToolOwner` gains a `has_permission`: `create` was already listed as
owner-only, but DRF never calls the object check on a create, which is why a
shared user could add an LLM profile and then not edit it.

Toggles. The ETL pipeline and API deployment enable/disable switches carried
no ownership gate even though both endpoints reject the PATCH.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UwnxaoVTsxbzufUixPGFQ9

* UN-2868 [FIX] Share Prompt Studio for collaboration, and gate the API key controls

Product call on the ticket thread: sharing is not one uniform rule. A Prompt
Studio project is shared for collaboration -- a locked one is not useful to a
team -- while adapters, connectors, workflows and deployments are shared for
use. Across all of them the owner keeps three things: renaming, deleting, and
deciding who else gets access.

Prompt Studio therefore reopens. Prompts, every Settings tab and LLM profiles
(create, edit, delete, and the default) are back to collaborative, in Agentic
Prompt Studio too. The credential concern behind locking profiles does not
hold: a profile can only point at adapters the requester already has access
to, and it never renders a key.

Renaming a project moved to a per-field check in the serializer. Settings and
the name share one PATCH endpoint, so gating the action would have closed both.

Two checks that were missing stay in place rather than reverting with the
rest. `reorder_prompts` is routed without a pk, so DRF ran no object check and
any org member could reorder any project's prompts. And `ProfileManagerView`
now uses `ParentToolAccess`: create is collection-level, so without a
`has_permission` it was open to the whole org rather than to collaborators.

Manage Keys hid nothing behind ownership, so a shared user could press New Key
and the active toggle and get a 4xx -- the toggle surfacing as "Api deployment
not found". Both are disabled now; listing and copying keys are unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UwnxaoVTsxbzufUixPGFQ9

* UN-2868 [FIX] Restrict file history deletion to the workflow owner

Shared access to a workflow granted read only everywhere except its file
history, where a shared user could delete single entries, bulk-clear by
filter, or wipe every file marker. All three paths are reachable from both
the ETL card and the workflow builder.

Gate the two FileHistoryViewSet write actions and clear_file_marker on the
workflow's owner, co-owner, org admin or service account. Listing and
retrieving history stay open to shared users. No UI change: the existing
exception handler surfaces the 403.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015aPCgGhE6Ma2NEhB8LQP1c

* UN-2868 [FIX] Scope the endpoint list, correct the rename status, unlock frictionless deletes

F1 (Critical): workflow_endpoint_list took the workflow id straight from the
URL, bypassing get_queryset() and object permissions -- and the response
serializes connector_metadata decrypted for S3/GCS/DB. Any org member could
read any workflow's connector credentials. Resolve the workflow through
Workflow.objects.for_user() first.

F5: the Prompt Studio rename refusal raised ValidationError, rendering as
400 {"tool_name": [...]} and painted under the name input by
AddCustomToolFormModal, while the sibling delete refusal returns 403. Raise
PermissionDenied so an authorization refusal reads as one.

F7: frictionless adapters are deletable by any org member server-side
(IsFrictionLessAdapterDelete), but the table gated delete on ownership and
AdapterListSerializer never exposed is_friction_less, so the only route to
remove a platform-provisioned adapter disappeared for non-admins. Expose the
field and exclude those rows from the delete lock. Edit stays locked.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015aPCgGhE6Ma2NEhB8LQP1c

* UN-2868 [FIX] Correct three docstrings in prompt_studio/permission.py

Each stated a rule the code does not enforce, on the sharing surface:
sharing onward is open to shared users, not owner-only; the rename is gated
in CustomToolSerializer.validate_tool_name, not on the viewset; and the
deferred import's stated reason was false -- no model imports this module,
and hoisting it creates no cycle (verified both directions). The comment is
removed rather than given a new reason I could not confirm.

Also records what the gate actually admits: direct viewers, groups and
admins, but not shared_to_org -- which IsOwnerOrSharedUserOrSharedToOrg does
admit. That divergence is finding F9 and is not fixed here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015aPCgGhE6Ma2NEhB8LQP1c

* UN-2868 [FIX] Admit org-shared users to Prompt Studio; make disabled-control tooltips render

F9: _can_access_tool admitted owner, direct viewer, group member and admin but
not shared_to_org, while IsOwnerOrSharedUserOrSharedToOrg -- used for the same
project's settings PATCH and profile reads -- does admit it. A user whose only
access was an org-wide share could change project settings and read profiles
but got 403 on every prompt edit and profile write: half the collaborative
surface working, half refusing, for one user on one project. The frontend
cannot express that state, since usePromptStudioCanEdit collapses to a single
boolean. Admit shared_to_org so both sides agree. Verified both directions.

F3: every "hover for the reason" tooltip added by this PR sat on a `disabled`
control, which swallows mouseenter, so the trigger never fired and the reason
never rendered -- 6 of the 8 new lock affordances communicated nothing. Fixed
once in the Tooltip shim rather than at each call site: a disabled child is
wrapped so the pointer events land on the wrapper. Not pinned by a test; the
existing cascade-and-affordances suite asserts the pointer-events-none
behaviour this works around, and 63 shim tests still pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015aPCgGhE6Ma2NEhB8LQP1c

* UN-2868 [FIX] Close the connector-credential path the endpoint-list fix left open

Findings raised by adversarial verification of the previous commits.

NEW-1 (Critical): WorkflowEndpointSerializer.connector_instance_id is a
writable PrimaryKeyRelatedField whose queryset was ConnectorInstance.objects
.all() -- org-scoped but not user-scoped, unlike every sibling. Any org member
could create their own workflow (becoming its owner, so IsParentWorkflowOwner
passes), PATCH their endpoint to a colleague's connector, and read that
connector's decrypted credentials out of the PATCH response, which
re-serialises through the nested ConnectorInstanceSerializer. This is the
surviving half of the exposure the endpoint-list fix closed. Scope the
queryset with for_user; no request in context now yields none() rather than
everything, and the only instantiation without context is read-only.

NEW-3 (Critical): WorkflowEndpointAPIView logged dict(request.headers) at INFO
-- including the internal service bearer key that authenticates it -- on the
view that returns decrypted connector metadata for any workflow in any
organization. No redaction filter exists. Drop the headers entry and lower the
line to debug. Pre-existing and outside this PR's surface; fixed here because
a logged credential that unlocks that view should not wait.

NEW-2 (High): the same stored-parent-gate-plus-writable-parent-FK shape is
open at ToolInstanceSerializer.workflow and
ProfileManagerSerializer.prompt_studio_tool. Refuse a parent change on update
at both. NOTE: with the cloud sites this is now five occurrences of one rule
patched site by site. The rule -- a gate that authorises against a stored
parent requires that parent to be non-writable -- is stated nowhere and
enforced nowhere central, so the next sub-resource added under
WorkflowOwnerMutationMixin will repeat it. Recommend hoisting the check into
the mixin; not done here, to avoid a broad refactor late in the run.

F6 (High): APIDeploymentSerializer -- the class returned for retrieve, create
and update -- did not carry is_owner, while canEditResource treats a missing
is_owner as editable. Any consumer seeding state from a non-list response
unlocked every Edit/Delete/toggle on it. Carry the field on both serializers.

NEW-6/NEW-7 (Low), both regressions in the previous endpoint-list fix: restore
the specific WorkflowDoesNotExistError 404 that get_object_or_404 had
flattened to "Not found.", and stop re-resolving the workflow row a second
time inside get_endpoints_for_workflow.

NEW-5 (Low): canEditResource's docstring claimed delete follows ownership,
which the frictionless-adapter carve-out makes false.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015aPCgGhE6Ma2NEhB8LQP1c

* UN-2868 [FIX] Close two more reparenting sites; simplify the endpoint-list fix

Two further sites of the stored-parent-gate class, both verified writable
before fixing:
- APIKeySerializer.api / .pipeline. IsParentDeploymentOwner gates every detail
  action against the stored parent, and create spends ~60 lines refusing a
  path-vs-body disagreement -- while update had nothing, so a live key could be
  retargeted onto a deployment the caller does not own.
- ToolStudioPromptSerializer.tool_id, gated by PromptAcesssToUser on obj.tool_id.

That makes seven occurrences of one rule now patched site by site: a gate that
authorises against a stored parent requires that parent to be non-writable.
Recommend hoisting it into a shared mixin beside WorkflowOwnerMutationMixin --
deliberately NOT done here, since it touches seven viewsets across two repos.

Cleanup from the review pass:
- workflow_endpoint_list reuses get_queryset() and get_serializer() instead of
  rebuilding the scoping by hand. That restores select_related and injects
  request context, which the serializer's new for_user scoping reads.
- get_queryset also joins connector_instance; the nested serializer needs it.
- Removed WorkflowEndpointUtils.get_endpoints_for_workflow, which inlining had
  left with zero callers -- a public helper carrying exactly the unscoped
  lookup this change removed.
- Dropped a comment claiming this raises "the same 404 the sibling actions do".
  It does not: the siblings raise DRF NotFound via get_object().
- Trimmed comments that retold mechanism or narrated an exploit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015aPCgGhE6Ma2NEhB8LQP1c

* UN-2868 [FIX] Make the disabled-control tooltip work beyond Button, and keep orphan profiles attachable

The span wrapper alone does not restore hover. A browser suppresses mouse
events on a disabled control and does not bubble them, so the span only
becomes the hit target once the child stops hit-testing. ui/button.tsx sets
disabled:pointer-events-none and worked; switch, checkbox, input, label,
radio-group, select and textarea set only cursor-not-allowed and did not --
which includes the live case this PR is about, the owner-only Switch in
ManageKeys. Neutralise the child from the wrapper instead of relying on the
child's own class, and give the wrapper tabIndex so the tooltip is reachable
by keyboard, which a disabled child never is.

Separately: ProfileManager.prompt_studio_tool is nullable and ParentToolAccess
has an explicit orphan branch, so refusing every value on an orphan left such
a row permanently unattachable. Only refuse a change away from an existing
parent.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015aPCgGhE6Ma2NEhB8LQP1c

* UN-2868 [FIX] Guard the workflow_id alias that bypassed the tool reparenting check

Two independent reviewers found the same hole in the guard added two commits
ago, and it is the sharpest argument yet for fixing this class structurally.

ToolInstanceSerializer declares workflow_id = UUIDField(write_only=True), and
workflow_id is also the attname of ToolInstance.workflow. validate_workflow
fires only when the payload key is "workflow"; a body of {"workflow_id": ...}
skips it entirely, and DRF's ModelSerializer.update does
setattr(instance, "workflow_id", uuid), writing the FK column directly.
IsParentWorkflowOwner has already passed, because it authorises against the
STORED parent. So the guarded key is the one no caller uses and the required
one was unguarded. Verified: the model's attname is workflow_id and the
serializer field is writable.

This is the eighth site of one rule, and the miss was an ALIAS of a field
already guarded in the same file -- enumerating the class by field name cannot
find it, only enumerating by gate can. The recommendation to hoist this into
WorkflowOwnerMutationMixin stands and is now stronger.

Also: reverted the orphan-profile relaxation from the previous commit. Letting
a NULL stored parent be attached looked like a usability fix, but
ParentToolAccess falls back to created_by for orphans, so it would let that
creator attach the profile to any project in the org. Refusing is the safer
half of the disagreement between the two reviewers.

Narrowed two comments that over-claimed. Scoping the write field and the list
action does NOT redact connector_metadata; the nested read serializer still
returns it decrypted to anyone the workflow is shared with. That residual is
escalated, not fixed here -- redacting it would change what the read-only
connector view shows shared users, which is a product decision.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015aPCgGhE6Ma2NEhB8LQP1c

* UN-2868 [FIX] Drop the focusable tooltip wrapper SonarCloud flagged

Sonar raised "tabIndex should only be declared on interactive elements"
(MAJOR) on the span added for disabled-control tooltips, and it is right: a
focusable span with no role and no accessible name is a stop in the tab order
that announces nothing, which is worse for a screen reader than no stop.

The substantive half stays -- neutralising the child's hit-testing is what
makes the tooltip open at all on a disabled Switch, Checkbox or Input, which
is the reported defect. Keyboard access to a disabled control's tooltip needs
a role and a name to be worth anything and is a larger change than this.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015aPCgGhE6Ma2NEhB8LQP1c

* UN-2868 [FIX] Close the ungated tool-instance create, and cover the gates with tests

Four findings from the standardized review, plus the test coverage the whole
batch was missing.

Adding a tool to a workflow was ungated. `ToolInstanceViewSet.get_permissions`
covered update/partial_update/destroy but not `create`, and the serializer
resolved the workflow unscoped and flipped `is_active` on it -- so any org
member could attach a tool to a workflow they do not own, while this branch
disabled only the button. Gated with the `is_workflow_mutator` idiom already
used by `reorder` in the same file. `WorkflowOwnerMutationMixin` does not fit
here: its `perform_create` reads `validated_data["workflow"]`, and this
serializer carries `workflow_id`, so adopting it would fail closed for owners.

`clear_file_marker` moves from GET to POST. It clears execution markers, so a
GET made it reachable by prefetch or a pasted URL with no CSRF in the way. No
frontend caller exists.

The workflow builder's connector-type dropdown was the one control left
ungated on that card -- a shared user picked a type and got a bare failure
toast from the endpoint this branch gates.

`IsParentToolOwner` is deleted. It had zero production callers after the swap
to `ParentToolAccess`, while its test asserted the inverse of the shipped
behaviour and passed. That test now targets the live gate, with the viewer
case flipped to allowed.

Tests: shared viewer refused on endpoint update/destroy and on tool-instance
create, allowed to read; a no-access user gets 404 rather than 403; rename
refused per-field while other settings save. Each gate was broken in turn to
confirm the matching test fails.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015aPCgGhE6Ma2NEhB8LQP1c

* UN-2868 [FIX] Close the credential read path and the ungated prompt creates

Review findings D1-D4, D6, SF2 and CR4.

- Shared viewers were served the destination connector's decrypted
  credentials on the endpoint read path. Redacted for anyone who may only
  read; the previous test asserted a bare 200 and could not see it.
- create_prompt and create_profile_manager never reached the object gate,
  so any org member could write into any project. The parent now comes
  from the URL, which both runs the gate and stops the payload naming
  another project.
- clear-file-marker moved to POST but its only caller still sent GET, so
  clearing file history 405'd for everyone, owners included.
- LookupDefinition was missing from the shareable-resource registry, so
  its group shares were never purged and never shown to an admin deleting
  the group.
- Two gates filtered a UUID column on raw request data ahead of any
  serializer, turning a malformed id into a 500.
- Manage Keys gated the switch and New Key but left per-key Edit and
  Delete live, so a shared user could fill in a form that only 403s.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NBTWcZd4CFXXRZSG94TsnB

* UN-2868 [DOCS] Correct five comments this change made untrue

Review findings D6, O1 and L16-2/4/6.

- PromptAcesssToUser enumerated four ways to qualify and the code has
  five; point at _can_access_tool instead of restating it.
- The share endpoint's docstring counted 7 shareable resources; there are
  eight, so name the rule rather than the count.
- The frontend helper and the Prompt Studio hook both said sharing grants
  read only, which is not true of Prompt Studio or Agentic PS.
- A test cited IsParentToolOwner, which no longer exists and whose
  successor admits collaborators.
- The read-only CSS named the HITL query builder, which no longer uses it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NBTWcZd4CFXXRZSG94TsnB

* UN-2868 [DOCS] Stop the shared list widgets claiming a rule they cannot know

Cross-document sweep for the read-only rule corrected in 025c12c.

Both widgets render Prompt Studio alongside every other resource, and
Prompt Studio is shared for collaboration -- so "sharing grants read only"
is wrong for one of their own consumers. What they actually gate is edit
and delete, which is owner-only everywhere; say that instead.

The card grid also carried a second owner rule, falling back to
``created_by`` when ``is_owner`` was absent. That fallback cannot see
co-owners, and every list serializer sends the field.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NBTWcZd4CFXXRZSG94TsnB

* UN-2868 [FIX] Answer the verifier: three of the last round's fixes were wrong

An adversarial pass re-found three findings and raised five more.

- The credential redaction asked who owned the workflow once per row, which
  is a membership query each time. Prefetch the rows on the endpoint
  queryset and let the owner check read them: measured 44 queries for 7
  endpoints both with and without the redaction, so it now costs nothing.
  A test pins it.
- The share docstring traded a stale count ("all 7 resources") for a
  universal that is false: lookups is the one host whose share entry is
  org-member-gated. Name the mechanism instead of the class.
- The read-only CSS comment dropped the HITL query builder as a stale
  consumer. It is not stale -- the connector modal still wraps the builder
  in that class. Stop naming a list that goes stale.
- Both new comments called a detail action "collection-level". The
  conclusion held but the reason did not: no custom @action gets
  get_object() for free, pk in the route or not.
- OwnerFieldRow kept a sessionDetails prop nothing read after the owner
  rule was collapsed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NBTWcZd4CFXXRZSG94TsnB

* UN-2868 [DOCS] Name the two child-create gates separately

The docstring called both halves custom ``@action``s. Only the prompt half
is: profiles go through a plain DRF ``create`` whose parent is read from
the payload in ``ParentToolAccess.has_permission``. Conflating them points
the next reader at a decorator that is not there.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NBTWcZd4CFXXRZSG94TsnB

* UN-2868 [DOCS] Drop the child-create restatement instead of rewording it again

Three attempts, three wrong descriptions -- the last one named a route
that does not exist: ProfileManagerView registers no POST, and the live
add-profile endpoint is PromptStudioCoreView.create_profile_manager, a
detail @action like its sibling.

The rule is already stated where it is enforced, once per half, in
prompt_studio_core_v2/views.py. A third copy on the test class is a
surface that has now gone stale every time it was touched, so remove it
rather than correct it a fourth time.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NBTWcZd4CFXXRZSG94TsnB

* UN-2868 [FIX] Reject a malformed prompt id at validation, and un-vacuum its test

The security pass caught this in the previous round's own work. The view
gate was guarded with parse_uuid, but the controller re-reads prompt_id
from validated_data and feeds it to objects.get, where a malformed value
is a 500. prompt_id was a CharField, so a complete request still reached
it -- the guard only covered the gate.

The test hid that: it omitted a required field, so it 400'd before the id
was ever parsed and would have passed against no fix at all. It now sends
a complete payload and asserts the 400, and fails if the field goes back
to CharField.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NBTWcZd4CFXXRZSG94TsnB

* UN-2868 [FIX] Admit group-shared users to a workflow's file history

Opening a workflow reached through a group worked; its File History tab
returned 403. `Workflow.objects.for_user` has always included group shares,
so the workflow is listed and opens, but `IsWorkflowOwnerOrShared` stopped at
OWNER membership, direct VIEWER membership, `shared_to_org` and org admin.
The queryset and the gate disagreed, and the gate was the narrower one.

Pre-existing rather than introduced here: this branch only added the
`IsWorkflowOwnerForFileHistoryWrite` subclass for delete and clear, and left
the read path as it is on main. `FileHistoryViewSet` is the only consumer, so
that tab was the whole blast radius.

Swept both repos for the same shape afterwards. Every other call site of the
owner/viewer predicates without a group branch is deliberately owner-only --
`IsOwner`, `is_workflow_mutator`, `IsRegistryToolOwner`,
`IsFrictionLessAdapterDelete`, the co-owner serializers -- since a group share
grants read and run, never mutation. Every `for_user` either includes groups
or delegates to one that does; `AppDeployment` predates the membership model
entirely.

Tests: a group member can open file history, the owner still can, and a
non-shared org member still cannot -- widening to groups must not widen to
everyone. Verified by removing the branch again and watching the first fail.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015aPCgGhE6Ma2NEhB8LQP1c

* UN-2868 [FIX] Scope the deployable workflow, and pin last round's gates with tests

Two findings from the standardized review.

**Deploying someone else's workflow.** `workflow` is writable on both the API
deployment and pipeline serializers and was bound through a merely org-scoped
manager, so any organisation member could POST a deployment naming a
colleague's private workflow -- then execute it, with its connectors and
adapters, at the owner's cost, and mint API keys against it. Reachable on
create and on update. Pre-existing.

Both serializers now scope the field in `get_fields`, the pattern
`WorkflowEndpointSerializer` already uses, and refuse reparenting the way the
other guards in this branch do.

The scope is `mutable_workflows_for` -- owners, co-owners, org admins -- not
the `for_user` floor. A deployment is a persistent execution surface its
creator owns and can share onward, so standing one up is an owner act rather
than a use of a shared workflow. Co-ownership is the route for anyone else.
Switching to the looser floor is a one-line change in that helper if the
product call goes the other way.

`resources_visible_via_memberships` grows an optional `role`, so the helper
reuses its varchar/UUID cast: joining the membership table directly fails
with `operator does not exist: uuid = character varying`.

**Tests.** Last round's fixes could each be reverted with the suite staying
green. Now pinned: endpoint-list scoping and its credential redaction, both
tool-instance reparent guards including the `workflow_id` alias, file-history
delete and clear, `clear_file_marker` as POST-only, and the new deployment
scoping. Every gate was reverted in turn to confirm the matching test fails.

Two of those runs found tests that passed for the wrong reason -- the
deployment cases were being refused by the endpoint-configuration check, not
by the gate -- so they now assert the `does_not_exist` code specifically.

`CreateEndpointGrantsCreatorOwnershipTests` built its parent workflow with
`created_by` but no OWNER row, which the scoping correctly refuses; its own
docstring says `created_by` is audit-only, so the fixture now grants the row
the real create path grants.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UwnxaoVTsxbzufUixPGFQ9

* UN-2868 [FIX] Clear the four SonarCloud smells on this branch

- `WorkflowEndpointSerializer.to_representation` had two `return rep`
  statements. Sonar reads that as always returning the same value; it does
  not, since `rep` is mutated between them, but a single exit says so
  plainly and the rule was flagged BLOCKER.
- The ETL and API deployment status tooltips were nested ternaries. Both
  lift to a named `toggleTitle`, which also removes a second
  `canEditResource` call per row.
- `canEditResource` uses an optional chain for the in-flight-payload guard.

Behaviour is unchanged throughout; 72 permission tests and the frontend
build pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UwnxaoVTsxbzufUixPGFQ9

* UN-2868 [FIX] Give the deployment-provision fixture the OWNER row it needs

`test_deploy_mints_key_and_endpoint` built its workflow with neither
``created_by`` nor an OWNER membership, so the deployable-workflow scoping
added in ba4f559 correctly refused it: ``created_by`` is audit-only, and
ownership comes from the membership row the real create path writes.

The fixture now grants that row, matching ``WorkflowViewSet.perform_create``.
Same shape as the `CreateEndpointGrantsCreatorOwnershipTests` fixture
corrected in the same commit; CI reported this as the only other one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UwnxaoVTsxbzufUixPGFQ9

* UN-2868 [FIX] Unbreak co-owners, and let a shared user start a pipeline

Findings from a code review of the last four commits.

**Co-owners locked out (2 sites).** Scoping the writable `workflow` field to
workflows the requester owns broke every edit of a deployment or pipeline
whose workflow belongs to someone else: both forms PUT the whole record
back, `workflow` included, so a co-owner of the resource 400'd on a field
they were not changing. The bound workflow is now always selectable;
`validate_workflow` still refuses an actual move, so reparenting stays shut.

**The enable toggle.** The rule is that a shared user may start and stop what
is shared with them, and the code said owner-only on both halves. Toggling is
now an activation-only PATCH -- one field, nothing riding along -- admitted
for anyone the resource is shared with. Settings stay with the owner. The API
deployment toggle sends that narrow PATCH instead of a full PUT, as the
pipeline one already did.

**A test that could not fail.** `test_a_get_is_rejected` drove
`as_view({"post": ...})` and sent a GET, so the 405 came from its own method
map; it passed with the decorator back on `methods=["get"]`. It now asserts
the action's mapping and every route that binds it.

**The deploy picker** offered workflows the serializer then refused, with a
raw `Invalid pk` naming an id the user never typed. It lists only deployable
ones now, and the refusal says why.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NBTWcZd4CFXXRZSG94TsnB

* UN-2868 [FIX] Make Enter save the HITL rules too, not just the endpoint

Greptile P1. The schema form's own submit was wired to the endpoint-only
path, so pressing Enter with both a connector edit and a rule edit pending
wrote the endpoint, announced "Configuration saved successfully", and
dropped the rules. The Save button took the coordinated path; the keyboard
did not.

Both now go through `handleSave`. It takes RJSF's validated data when the
form supplies it and falls back to state for the button, so neither path
loses the fresher value.

The Save button's `onClick` is wrapped rather than passed by reference: a
bare reference would hand the click event to that new parameter, and the
event would then be compared against the saved configuration and written
as it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NBTWcZd4CFXXRZSG94TsnB

* UN-2868 [MISC] Empty commit to re-run CI

No code change. The last red run predates the fixture and SonarCloud fixes on
this branch; this re-triggers the checks against the current head.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UwnxaoVTsxbzufUixPGFQ9

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

…rded (#2286)

* UN-2868 [FIX] Stop the connector modal reporting a HITL save it discarded

Editing the HITL rules, switching to the connector's Settings tab and saving
reported success and wrote nothing. The tabs are the antd shim over Radix,
which unmounts the pane behind an inactive tab, so leaving Manual Review
destroyed RuleEngine along with the edits and the ref the modal saves
through. `writesRules` then read false and, being false, also told the
endpoint write to post its own success toast.

Only the owner sees this: `canEdit` keeps a shared user from making the edit
in the first place.

Unmounting is not new -- it predates the sharing work and the shim is
untouched by it. What is new is that HITL lost its own Save button and now
writes through the modal's, so an unmount stopped losing a visible unsaved
edit and started making the Save button lie.

The shim gains antd's behaviour of holding a pane once it has been opened,
opt-in per pane so nothing else changes. Held from first open rather than
from render, because RuleEngine loads on mount and alerts when that fails --
mounting it up front would raise an error for a tab nobody opened. Radix
treats a force-mounted pane as present and so leaves off `hidden`; the pane
is hidden on its state instead.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UwnxaoVTsxbzufUixPGFQ9

* UN-2868 [FIX] Pin the ref across a tab switch, and trim the comments

The tests measured mounts; the failure is a ref. A dropped pane nulls the
handle the modal saves through, so the new test asserts a ref into a kept
pane is still callable after switching away -- it fails on its own when the
hold is removed.

Comments cut back to what is not in the code.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UwnxaoVTsxbzufUixPGFQ9

* UN-2868 [FIX] Decide the hold once per pane

`forceMount` required the pane to have been opened; the hiding class went on
`keepMounted` alone. Only ever a cosmetic mismatch -- an unopened pane is not
rendered, so the class had nothing to hide -- but the two reading differently
invites a fix to the wrong one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UwnxaoVTsxbzufUixPGFQ9

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

…, backend and SDK (#2284)

* UN-4078 [MISC] Delete the SDK Celery ExecutionDispatcher and retarget its tests

First slice of the Celery transport removal. `ExecutionDispatcher` published
`execute_extraction` to RabbitMQ and blocked on an `AsyncResult`; nothing has
selected it since UN-4046 made `get_executor_dispatcher()` return the PG
request-reply dispatcher unconditionally. It had zero non-test importers.

Deleted: `unstract/sdk1/.../execution/dispatcher.py` (316 lines), its export
from the package `__init__`, and the `TestExecutionDispatcher` suite.

The 11 workers test files that imported it are retargeted rather than dropped —
they encode live product behaviour (which queue each operation lands on, what
the payload carries), and that behaviour survives on `PgExecutionDispatcher`:

- Queue naming moves to `unstract.workflow_execution.executor_rpc.QUEUE_PREFIX`,
  the surviving definition. The `celery_executor_*` names are NOT dead Celery
  surface — `worker-pg-executor` subscribes to exactly those strings — so the
  assertions pin the literal wire names.
- New `tests/executor_dispatch_fakes.py` holds the shared fake transport, an
  eager variant that runs the task in-process (replacing the `send_task`
  monkey-patching four round-trip tests did), and a correctly-shaped callback
  signature builder.
- The header-forwarding suite is replaced, not ported: PG dispatch takes no
  `headers=` by design, so the new tests assert org routing rides the payload
  AND that passing `headers=` still raises — the exact mistake that broke three
  call sites during UN-4046.
- The raw-`send_task` canary is kept and its message updated; it matters more
  now, since a raw send_task publishes to a broker nothing drains.

Two docstrings that claimed executor calls "go through Celery" were corrected —
they became false with this change, not merely dated.

Verification: workers 1517 passed / 1 skipped. sdk1 unchanged at 20 pre-existing
failures (`test_llm_compat`, `test_litellm_cohere_timeout`), reproduced on a
stashed tree to confirm they predate this work. Pre-commit clean on all 17 files.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4078 [MISC] Delete the Celery execution transport

Removes the Celery fan-out substrate and the per-execution transport field.
Nothing has selected either since UN-4046 made `select_backend()` return PG
unconditionally, but both were still compiled, imported and shipped — and
importable Celery surface has already cost two incidents (UN-3779's hardcoded
dispatcher, and the `headers=` breakage at three call sites during UN-4046).

Deleted outright:
- `queue_backend/redis_barrier.py` (761 lines) — the DECR-counter substrate
- `CeleryChordBarrier` + `BarrierBackend` + `get_barrier()` + the
  `WORKER_BARRIER_BACKEND` env selector
- `WorkflowTransport`, `DEFAULT_WORKFLOW_TRANSPORT`, `normalize_transport`,
  `is_pg_transport` (88 lines of `unstract/core/data_models.py`)
- `barrier_pg_decr_and_check` — a thin @worker_task wrapper reachable only as a
  Celery `.link`; the decrement runs in-body via `_barrier_pg_decrement`

Collapsed: the 10 `is_pg_transport` branch sites, the `transport` parameter
threaded through the general/api/scheduler orchestrators, the `transport` field
on `WorkflowContextData` and `FileProcessingContext`, the `is_pg` flag on the
file-batch path, and the `transport` key the backend wrote into the dispatch
payload. `_barrier_for_transport` becomes `PgBarrier()`.

Two guards become unconditional rather than PG-gated: the terminal-execution
skip and the duplicate-destination-write check. Both were only ever no-ops on
Celery.

This also starts retiring a documented live hazard. `DEFAULT_WORKFLOW_TRANSPORT` was
`"celery"`, so a payload that lost the field during a rolling deploy selected a
Celery fan-out with no consumers. Worth correcting the in-code note while
deleting it: it claimed `CeleryChordBarrier` was selected and no PG rows were
written, so the reaper could not see the strand. Neither held — every PG worker
sets `WORKER_BARRIER_BACKEND=pg`, so `PgBarrier` was selected, and its
`_reset_barrier` UPSERT is unconditional, so the barrier row was written and the
reaper did see it. Real behaviour was a ~2.5h delayed ERROR.

The reads are removed here, but the writes are NOT: a pre-UN-4078 worker still
defaults an absent field to Celery, so during a rolling deploy it would publish to
a RabbitMQ with no consumers. The next commit keeps writing the field for one
release; the hazard is gone only once that shim is removed.

`EXECUTION_EXCLUDED_PARAMS` deliberately keeps its `"transport"` entry: a
rolling deploy can still have an un-upgraded producer emitting the field, and
without the exclusion it reaches the legacy `execute_workflow` signature and
raises.

Tests: four whole suites deleted with their substrate (redis barrier, barrier
backend selection, barrier differential, workflow-context transport). The rest
are retargeted rather than dropped — the chord canary now asserts *zero* chord
call sites (with a non-vacuity lock, since a zero-expectation assertion is
exactly what a path bug passes silently), and the Celery-gated guard tests are
replaced by the unconditional behaviour they now have.

Verification: workers 1438 passed / 1 skipped / 0 failed. Pre-commit clean.
The backend suite cannot start in this environment (`AppRegistryNotReady`,
reproduced on a stashed tree — pre-existing), so the four backend edits are
CI-verified only.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4078 [MISC] Keep writing the transport field for one release as a rolling-deploy shim

The previous commit removed every read of the `transport` payload field and
every write. Removing the writes in the same release is unsafe: a pre-UN-4078
worker still treats an absent field as "celery" (DEFAULT_WORKFLOW_TRANSPORT)
and, during a Kubernetes RollingUpdate, can receive a payload from an upgraded
producer. It then publishes to RabbitMQ, which has no Celery consumers, so:

- general workflows are marked ERROR by the reaper after ~2.5h, even with every
  file processed;
- API deployments take no orchestration claim, so nothing sweeps them and they
  stay EXECUTING.

The window is real: old file-processing pods keep draining claimed batches for
their full grace period (up to 9120s).

The four writes are restored as `transport: "pg_queue"`, all via one constant,
LEGACY_TRANSPORT_KEY / LEGACY_TRANSPORT_VALUE in unstract.core.data_models, so
the follow-up removal is a single grep:

- backend WorkflowHelper orchestrator dispatch payload
- backend create_workflow_execution response
- workers scheduler async_execute_bin kwargs
- workers PgBarrier callback descriptor (CallbackDescriptor gains
  `transport: NotRequired[str]`)

Nothing on the new side reads the field. Tests pin each write so the shim cannot
be dropped early; the scheduler cases also assert a stale "celery" in the
backend response is never forwarded.

Remove in the release after UN-4078: the writes, the constants, the
`transport` entry in EXECUTION_EXCLUDED_PARAMS, and test_legacy_transport_shim.py.

Verification: workers 1478 passed / 1 skipped / 0 failed; backend dispatch
tests 5 passed (with backend/sample.env loaded); pre-commit clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4078 [MISC] Address standardized review: shim gating tests, dead branches, stale comments

Review findings on #2284, highest first.

#1 (High) — half the rolling-deploy shim had no gating test that runs in the
default lane. The create_workflow_execution response write had none at all, and
the PgBarrier descriptor was pinned only by a test behind the barrier_db fixture,
which skips wherever Postgres is absent. Descriptor construction moves to
build_callback_descriptor(), pinned by a DB-free suite
(workers/tests/test_legacy_transport_shim.py); the response write is pinned by a
new backend case. All four writes now fail a required build if dropped early.

#2 (High) — _process_file_batch_core's Args entries still described
barrier_context=None as the supported Celery chord path while the body treats it
as a malformed payload. Both entries rewritten; the log line now carries
execution/workflow ids and the batch size.

#3 — process_file_batch_django_compat delegates with two arguments, so after this
PR every call landed in that malformed-payload branch: a false ERROR, and a full
batch run with no claim (no pg_batch_dedup marker, so a redelivery re-runs it —
LLM spend twice and a duplicate destination write with use_file_history=False).
Its only producer was the Celery chord this PR removes, so it now refuses, the
same fail-fast choice step_execution makes. Its MRQ helpers are left in place per
the no-removal rule.

#5 — PG_TRANSPORT_CALLBACK_KWARG became unconditional but its consumers still
branched on it, leaving an else branch that swallows a failed finalization and
strands the execution. The marker is now popped for wire compatibility only;
the duplicate guard and the re-raise are unconditional. is_pg on
_update_execution_status_unified becomes raise_on_failure (default True); the one
caller inside an except block opts out so a raise cannot mask the original error.

#6 — the removal checklist now lists all five artefact groups, and
EXECUTION_EXCLUDED_PARAMS names the key through LEGACY_TRANSPORT_KEY instead of a
hardcoded literal grep would miss.

#7 — CallbackDescriptor.transport is Literal["pg_queue"] and required, not
NotRequired[str]: it is mandatory this release, "celery" must never be written,
and the follow-up removal becomes a type error at the write site.

#8 — the Barrier protocol's "two call sites still program against it" was untrue;
both construct PgBarrier concretely. Corrected, with what actually keeps it.

#10 — executor_dispatch_fakes reimplemented the queue-naming rule and accepted
any enqueue signature. queue_for now delegates to a new production helper
queue_for_executor() (also used by the three dispatch sites), and the fake mirrors
the QueueTransport protocol's keyword-only key set.

#11, #14 — stale "PG only" / "No-op on Celery" comments at the sites this PR made
unconditional, a citation of the deleted Celery max_retries=0 wrapper, and a
cross-reference to a chart file that lives in the cloud repo.

Five tests that asserted the removed Celery branches are restated against the
collapsed behaviour rather than deleted.

Verification: workers 1481 passed / 1 skipped; backend shim, dispatch-orchestrator
and async-wait tests pass; pre-commit clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

… so nested cell values expand again (#2288)

* UN-4124 [FIX] Implement antd's expandable on the shared DataTable

The Ant Design removal (#1683 / #2212) replaced `<Table>` with the shared
DataTable, which never implemented antd's `expandable` API. Undeclared, the
whole object fell into `...props` and was spread onto the wrapper <div>, where
React ignores it — so `expandedRowRender` was never called and the table
rendered as if the prop had not been passed, with no console error.

HITL's review editor is the visible casualty: an array or object inside a table
cell shows a truncated JSON blob with an expand button beside it, and clicking
that button did nothing at all, leaving nested values unreadable in Table view.

Support `expandedRowRender`, controlled `expandedRowKeys` and uncontrolled
`defaultExpandedRowKeys`, `rowExpandable`, `showExpandColumn`, `expandIcon`,
`expandedRowClassName`, `onExpand` and `onExpandedRowsChange`. The panel is a
sibling <tr> spanning every column, since a row may only contain cells.

Keys are compared as strings because that is what TanStack's `getRowId` (and so
`row.id`) produces: a call-site numbering its rows `key: index` passes numbers,
and `[0].includes("0")` is false — a mismatch that would have hidden every
expansion on its own.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4124 [FIX] Report expansion keys in the caller's own type

Review feedback on #2288.

`onExpandedRowsChange` built its argument from the internal `expandedKeys`
set, which is normalized with `.map(String)` so it can match TanStack's
`row.id`. That normalization leaked out: a controlled caller that passed
`[1]` was handed back `["1"]`, and a parent testing `includes(1)` against it
would never match.

Normalization is now strictly internal. The reported array is built from the
caller's own keys — `controlledExpandedKeys ?? ownExpandedKeys` — appending
`originalRowKey()`, which reverses `getRowId`'s stringification by reading
`rowKey` off the record directly. Collapse compares with `String(k) !== key`
so it can filter a normalized id out of an un-normalized list without
rewriting the survivors' types.

Expansion itself is unaffected: `expandedKeys` is still a string Set, so
matching behaves exactly as before. `defaultExpandedRowKeys` no longer
stringifies on the way into state, since the memo normalizes anyway.

Both directions are covered and mutation-checked: stringifying the appended
key fails the expand test, and filtering the normalized set fails the
collapse test.

Also addresses SonarCloud S9020 — `waitFor` + `getByText` becomes
`findByText` in the nested-panel test.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

* [MISC] Mark use_file_history as internal in the published spec

The field is accepted by the server but not documented, and the CLI builds
its flags from this spec, so it exposed the option. Dropping the field would
break the generated clients that forward it by name, so it stays in the
serializer and in the spec with an `x-internal` marker a reader can honour.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ

* [MISC] Withdraw use_file_history from the published spec instead of marking it

The option is internal-only, so nothing generated from the spec should offer
it: the serializer keeps accepting it, the spec no longer declares it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ

* [MISC] Drop comments the decorator and test name already cover

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

* UN-4128 [FIX] Stop the Pipeline list page 500ing on the workflow-scoping guard

PipelineSerializer.get_fields() (UN-2868, #2273) scopes the workflow field
to the requester's own workflows, guarding the single-instance case with
`self.instance is not None`. On a list request DRF hands the child
serializer of a many=True ListSerializer the whole queryset as
self.instance, not one row -- `is not None` let that through and
`.workflow_id` crashed on a list.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018KZLGSa3oWxgVJdFqvRUQX

* UN-4128 Add regression test for pipeline list serializer crash

Covers the AttributeError DRF's paginated list GET triggers when
self.instance is a list instead of a Pipeline or None.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* UN-4128 Cover the public many=True constructor in the list-scoping test

Greptile asked for a test that exercises PipelineSerializer(queryset,
many=True) directly instead of hand-setting self.instance. DRF's
many_init passes the same instance to the child, so both paths already
caught the regression, but this removes any doubt.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

…cutor RPC caller for an hour (#2289)

* UN-4126 [FIX] Stop a NUL in an extracted value from stranding the executor RPC caller for an hour

A finished executor task could fail to hand its result back, leaving the
blocking caller to wait out the full EXECUTOR_RESULT_TIMEOUT (3600s) on work
that had already succeeded, then error. Seen in prod us-central and reproduced
in a dev namespace.

`store_result` encodes the reply with `json.dumps` and inserts it as
`%s::jsonb`. That cast makes Postgres *parse* the JSON, and `jsonb` stores
strings as `text`, which cannot hold a NUL:

    psycopg2.errors.UntranslatableCharacter: unsupported Unicode escape sequence
    DETAIL:  \u0000 cannot be converted to text.
    CONTEXT: JSON data, line 1: ..."output": {"invoice_number": "POZFBBOK\u0000...

LLMWhisperer's `native_text` mode returns a PDF's embedded text layer verbatim,
NUL bytes included; the byte travels through the prompt into the extracted
output. The consumer then logs-and-acks (deliberately, to avoid an expensive
re-run), so a write that never lands is a reply that never comes.

This is PG-only and did not exist before. Celery's result backend stored the
reply object in a `PickleType` column (bytea), where a NUL is just a byte; the
payload shape never changed, only the storage medium. The code has been
reachable since #2108 (Jul 23) but became reachable for *all* traffic when
UN-4046 made `get_executor_dispatcher()` return the PG dispatcher
unconditionally.

Adds `unstract.core.jsonb` as the single place that knows what `jsonb` refuses
(NUL, lone surrogates, NaN/Infinity) and points all three PG-queue jsonb writers
at it. They previously carried three different partial defences — the barrier
guarded NUL reactively, the producer guarded NaN, the result backend guarded
neither — which is why an existing regression test for this exact hazard
(`test_nul_byte_result_tears_down_barrier`) could not catch it.

Strings are repaired rather than rejected: the work is done and paid for, and a
stray control byte is not a reason to discard an extraction. Numbers are not —
`NaN` has no correct jsonb spelling and coercing it would corrupt a value, so
`allow_nan=False` is enforced and the caller decides.

`store_result` now always writes a row. If the payload is still unstorable it
degrades to a `failed` row carrying a diagnostic message, so the caller fails in
seconds instead of hanging. That is the property that made this invisible for
two weeks, and it is now the one under test.

Behaviour change worth review: `allow_nan=False` moves NaN/Infinity in a barrier
header result from the DataError path to the encode path. That class used to
reach the barrier teardown, so the teardown is extended to the encode-failure
branch — without it a NaN would newly hang the barrier to its ~6h expiry.

No migration: `pg_task_result.result` stays `jsonb`.

Tests: new `unstract/core/tests/test_jsonb.py` (15); three DB-backed tests in
`test_pg_result_backend.py` including a literal reproduction of the production
payload; `test_pg_barrier.py`'s NUL test inverted to assert repair, plus a new
teardown test for the unencodable case. Full workers suite 1485 passed against a
real Postgres.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4126 [FIX] Close the same jsonb gap on the enqueue path, which loses a callback instead of hanging

An audit of every raw-SQL jsonb writer in the PG queue found three more sites on
the plain `json.dumps` path. One of them carries the SAME payload as the bug this
branch already fixes.

`consumer._handle` self-chains a continuation with `prepend=eager.result` — the
executor result dict, byte-for-byte the object `store_result` stores. It reaches
`PgQueueClient._insert_message`, which encoded with a plain `json.dumps` and
inserted as `%s::jsonb`. A NUL in an extracted value therefore fails the INSERT
on this branch too.

The symptom differs, which is why it was not visible alongside the original
report. `_chain_continuation` is documented "never raises" (a failure must not
wedge the executor message's ack), so the enqueue failure is swallowed and the
run falls back to `on_error`. That fallback prepends only a `task_id` string, so
it succeeds — meaning the user is shown a FAILURE for work that completed, with
the LLM spend already made. No hang, no error in the caller's logs, just a wrong
terminal state.

Sites changed:

- `pg_queue/client.py::_insert_message` — the workers' enqueue. Kept without a
  `default=` so the TypeError-on-UUID contract that `consumer._json_safe`
  compensates for is unchanged.
- `pg_queue/consumer.py::_json_safe` — the prepend coercion; the same pass now
  strips what jsonb refuses instead of only coercing UUID/datetime.
- `pg_queue/pg_scheduler.py` (x2) — scheduler-built payloads. Lower risk (they
  carry schedule rows, not document content) but the same shared rule, so the
  remaining `json.dumps` in the tree is gone.

Audited and deliberately NOT changed: `pg_barrier.py`'s `'[]'::jsonb` literals
(no user data) and `liveness.py`'s `json.dumps(...).encode()` (an HTTP body, not
SQL).

Test: `test_send_repairs_a_nul_instead_of_failing_the_enqueue` in
`test_pg_queue_client.py` — real Postgres, sends the executor-result shape with a
NUL and asserts the message lands and reads back repaired. Full workers suite
1486 passed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4126 [FIX] Keep valid surrogate pairs — strip only the halves jsonb rejects

Greptile P1 on #2289, and it is correct: `JSONB_UNSAFE_RE` was the character
class `[NUL + U+D800-U+DFFF]`, which matches BOTH halves of a well-formed
surrogate pair. An emoji in an extracted result reaches Python as a high+low
pair, so the sanitiser deleted it outright — silent data loss, in the module
written to prevent silent data loss.

Verified against a live Postgres rather than reasoned:

    valid pair (U+1F600)  -> ACCEPTED  {'a': 'GRINNING FACE'}
    lone high  (U+D800)   -> REJECTED  invalid input syntax for type json
    lone low   (U+DC00)   -> REJECTED  invalid input syntax for type json
    NUL        (U+0000)   -> REJECTED  unsupported Unicode escape sequence

So a pair is storable and must be preserved; only unpaired halves are unsafe.
The pattern now matches NUL, a high surrogate with no low after it, and a low
surrogate with no high before it. `re.sub` evaluates against the original
string, so a deletion cannot fuse two neighbours into a spurious "pair".

Tests: four new cases in test_jsonb.py (pair preserved, lone low stripped, only
the unpaired half stripped, pair intact in the encoded output) and a DB-backed
round-trip in test_pg_result_backend.py storing an emoji through the real
result store. Full workers suite 1487 passed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4126 [FIX] Close the three High findings from the standardized review

All three reproduced independently before fixing; none was taken on trust.

1. `default=` hook output bypassed sanitisation (jsonb.py)

   `sanitize_for_jsonb` runs before `json.dumps`, so a string the hook
   manufactures at encode time was never inspected — and `str` on an exception
   carrying document text is exactly how a NUL gets there. The module's stated
   guarantee was false at every call site passing a hook: a NUL-bearing object
   coerced via `default=str` re-emitted the escape jsonb refuses. The hook is
   now wrapped rather than passed through. No call site changes.

2. `RecursionError` escaped both degradation seams (jsonb.py)

   The Python pre-walk sits in front of a C encoder that has its own cycle
   check, so a circular reference raised `RecursionError` where `json.dumps`
   raised `ValueError`, and nesting past ~1000 newly failed. `RecursionError`
   subclasses `RuntimeError`, so `except (TypeError, ValueError)` missed it at
   both seams — re-creating the exact strand this PR removes, on a new input
   class. Now re-raised as `ValueError` so the documented contract and every
   downstream seam keep working.

3. "A row is always written" was false (result_backend.py)

   `ProgramLimitExceeded` (SQLSTATE 54, "string too long") subclasses
   `OperationalError`, so it was absent from the `DataError` net and present in
   `CONN_DEAD_ERRORS`: an oversized result was misread as a dead connection,
   pointlessly retried, then escaped — no row, caller stranded for the full
   3600s. Signalling was also skippable, and two inserts were unguarded.

   Departed from the review's suggested fix here. It proposed `except
   Exception`; that breaks three existing tests encoding a deliberate contract
   (infrastructure errors propagate) and, worse, would record a database outage
   as "payload unstorable" — hiding an outage behind a content error. Content
   rejections are a knowable, closed family, so they are enumerated in
   `_PAYLOAD_REJECTED_ERRORS` instead. Signalling moved into a `finally`, and
   the degraded insert is now itself best-effort.

Also from the review (Medium): `status="failed"` now has a third meaning —
completed-but-undeliverable. Recovery logic that retries every `failed` reply
key would re-run an executor task that already finished, a second full LLM
spend. Documented on both the backend module and `PgTaskResult`.

Testing, addressing the review's strongest finding — every real-Postgres
assertion for this fix sat in `integration-workers`, which is `optional: true`
and cannot turn CI red. The strand-prevention contract now has DB-free tests in
the gating `unit-workers` lane (`TestStoreResultNeverStrands`), using a fake
cursor: oversized result still records a failed row, unencodable result still
records a failed row, and the waiter is signalled even when every write fails.
Plus three encoder tests in the gating `unit-core` lane.

Full workers suite 1490 passed, 1 skipped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4126 [FIX] Close the four open High findings, and the barrier's sibling of the bug this branch just fixed

Two of these are regressions introduced by the previous commit.

1. The `finally` test asserted nothing (test_pg_result_backend.py)

   `test_waiter_is_signalled_even_when_every_write_fails` injected
   `ProgramLimitExceeded` — a member of the rejected family — so
   `_write_outcome` caught it, `_insert_degraded` swallowed the second failure,
   and the method returned NORMALLY. The `finally` was never the reason the
   signal fired, so deleting it left the suite green: a future refactor moving
   the call back to a trailing statement would have silently restored the
   strand.

   Split in two. The raising path now injects an error OUTSIDE the rejected
   family so it propagates and only the `finally` can signal; the swallowing
   path keeps the original scenario. Mutation-checked: removing the `finally`
   now fails the first and passes the second, which is the point.

2. `Raises:` named ValueError for a TypeError case (jsonb.py)

   The previous commit rewrote this block and kept the false clause, describing
   one condition with two classes. `json.dumps` raises `TypeError` for an
   unserialisable type with or without a `default`; `ValueError` is only
   NaN/Infinity and the recursion conversion.

3. The shared status enum was not extended (data_models.py)

   `PgTaskStatus` exists so the workers writer and backend reader agree on this
   vocabulary — its docstring says exactly that. Both consumers' docstrings were
   updated last commit; the canonical definition still read "task raised", so a
   maintainer reading the source of truth got the contract this branch
   invalidated and would build the retry that double-spends. Documented there,
   including why there is no third status value (it would break every existing
   reader) and that the discriminator is therefore the error text.

4. Two of four jsonb sinks had no gating coverage

   The previous commit closed this for `result_backend` and `jsonb`; `client.py`
   and `pg_barrier.py` were still verified only by `integration-workers`, which
   is `optional: true`. Both now have DB-free tests in gating lanes.

5. The barrier carried the same DataError-only net (pg_barrier.py)

   This branch's whole argument is that `ProgramLimitExceeded` subclasses
   `OperationalError` and slips a `DataError`-only catch. `result_backend` was
   widened; its sibling was not — an oversized header result was misclassified
   as a dead connection, the same oversized UPDATE re-sent, then it escaped and
   the barrier hung to `expires_at` (~6h) with no actionable log.

   The classification tuple moves to `pg_queue/connection.py` beside
   `CONN_DEAD_ERRORS`, for the reason that module already gives for hoisting the
   other one: both sites classify on it and must not drift. The comment records
   the trap explicitly — `ProgramLimitExceeded` is absent from `DataError` AND
   present in `CONN_DEAD_ERRORS`, so payload rejection must be tested first.

Full workers suite 1493 passed, 1 skipped; core 22 passed.

Deliberately still open, and recorded as deferred rather than waived: the
sibling message asymmetry (consumer.py:660), the stale `json.dumps` docstring
(consumer.py:751), the producer's per-field sanitisation, key-collision and
mutation-logging in the sanitiser, and the degraded row's ON CONFLICT semantics.
The last two are design decisions that want an owner's call, not a default.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4126 [FIX] Stop the barrier decrement releasing the reaper's recovery handle

Regression introduced by this branch, found by the standardized review and
confirmed independently. It is worse than the bug it was trying to avoid.

The first commit extended the barrier teardown to the encode-failure branch,
reasoning that `allow_nan=False` moved NaN/Infinity onto that path so the
teardown should follow it. That reasoning checked only which exception reached
the branch — never whether tearing down there was correct at all. It is not.

`run_batch_with_barrier` is the only caller, and it deliberately implements the
opposite ordering:

    if _mark_execution_error_on_abort(barrier_context, reason=reason):
        _abort_barrier_in_body(...)          # confirmed terminal -> release
    else:
        # Mark unconfirmed (backend down / no org): leave the barrier row so
        # the reaper marks it ERROR and reclaims it at expiry. Do NOT erase the
        # handle — that's the strand this ticket fixes.

`_barrier_pg_decrement` called `_delete_barrier` and then re-raised, so by the
time that `else` runs the row is already gone. The execution stays EXECUTING
with no `pg_barrier_state` row for the reaper's sweep to find — permanently
stranded, not hung-to-expiry — and stale EXECUTING rows also make the workflow
look busy to later runs (`SourceConnector._get_active_workflow_executions`).

Both `_delete_barrier` calls on the raising paths are removed; the failures now
propagate and the caller's mark-then-release ordering governs. The two calls
that remain in the function are on non-raising paths (expiry/replay at
`remaining < 0`, and cleanup after the callback fires at `remaining == 0`), so
they cannot pre-empt the caller's handler — verified by AST walk.

Scope note: this removes the pre-existing teardown on the DB-rejection branch as
well, not only the one this branch added. The two were the same defect; leaving
one would keep the strand reachable for oversized payloads.

Tests: both tests that pinned the old behaviour now assert the new contract —
`test_unencodable_result_raises_and_leaves_the_row_for_the_caller` (real PG,
asserts the row survives) and `test_oversized_result_propagates_without_
releasing_the_handle` plus a new encode-side twin (both gating, assert
`_delete_barrier` is never called). Full workers suite 1494 passed, 1 skipped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4126 [FIX] Gate the result-backend encoder, and stop the error-only path stranding

Four findings from the second standardized review, all in the result backend.

1. The encoder swap had ZERO gating coverage. Every test that proved
   `dumps_for_jsonb` was in use ran against real Postgres, and `conftest.py`
   marks those `integration`, which routes them to `integration-workers` —
   `optional: true`, so it cannot fail CI. Reverting the call to plain
   `json.dumps` left the gating lane green at `15 passed, 17 deselected`.
   Three mock-level tests now assert on the SQL parameter itself:
   `test_result_payload_is_sanitised_before_the_jsonb_cast`,
   `test_error_text_is_sanitised_before_the_insert`,
   `test_rejected_error_text_also_degrades_to_a_row`. With the sanitisers
   reverted the lane now reports `2 failed, 16 passed` — mutation-verified,
   not assumed.

2. The `result is None` branch of `_write_outcome` could still strand. The
   completed branch catches `_PAYLOAD_REJECTED_ERRORS` and writes a degraded
   row; the failure branch did not, so an error string the database refused
   (a NUL from a traceback quoting extracted text, or one over the field
   limit) propagated with no row written at all — the exact strand this
   ticket exists to close, reachable from the error path instead of the
   success path. It now degrades the same way.

3. The surrogate round-trip test could not fail. `chr(0x1F600)` is a single
   code point on a wide build, not a surrogate pair, so the naive
   `[\x00\ud800-\udfff]` class the previous commit removed would have left it
   untouched and the test would have passed against the bug it was written to
   catch. It now builds the pair explicitly — `chr(0xD83D) + chr(0xDE00)` —
   and asserts Postgres recombines it to `chr(0x1F600)`.

4. `PgResultBackend.PAYLOAD_UNSTORABLE_ERROR` in the `PgTaskStatus.FAILED`
   docstring was a dotted path that does not resolve; the constant is a module
   attribute, not a class one. Corrected to
   `queue_backend.pg_queue.result_backend.PAYLOAD_UNSTORABLE_ERROR`.

Tests: `test_pg_result_backend.py` 35 passed. Full workers suite 1497 passed,
1 skipped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4126 [FIX] Record the encoder's measured cost, and why the fast path was rejected

Regression sweep before merge. The sanitising walk rebuilds every container and
string on the way to the database, so it is the one place on this branch that
could cost something on the hot path. Measured rather than assumed: ~11x a plain
`json.dumps` on a 165 KiB result (0.38 ms -> 4.2 ms) and ~16x on 2 MB
(3.1 ms -> 49 ms).

Accepted. One encode per stored result, against a task that took seconds, at a
measured platform ceiling of ~4-6 executions/s spread across the executor fleet.

The obvious optimisation was built and thrown away, which is the part worth
recording: encode first, scan the OUTPUT for the escapes jsonb refuses, and walk
only on a hit. It is correct and it is fast, but the walk is recursive and the C
encoder is not, so it made a deeply nested payload succeed or fail depending on
whether it also contained a NUL -- and it broke
`test_excessive_nesting_raises_valueerror`, which pins exactly that contract.
Trading a consistent contract for milliseconds nobody is waiting on is the wrong
way round, so the note explains the cost instead of the code chasing it.

No production code changes in this commit.

Tests: unstract/core 22 passed. Full workers suite 1497 passed, 1 skipped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4126 [FIX] Stop an oversized payload being re-sent as if the connection died

Greptile's remaining P2 on this PR, and it is right. The previous commit
documented the trap and left it in place.

`ProgramLimitExceeded` ("string too long", SQLSTATE 54) subclasses
`OperationalError`, so it is a member of BOTH `PAYLOAD_REJECTED_ERRORS` and
`CONN_DEAD_ERRORS`. Every retry site classified with a bare
`isinstance(exc, CONN_DEAD_ERRORS)`, which answers "dead connection" for a
payload the server merely refused: the handle is condemned and closed, a
reconnect happens, and the write is re-sent once before the caller's
payload-rejection handler finally sees it. It can never land, and the payload is
by the nature of this error very large — so the cost is a pointless reconnect
plus a second large write, on the path that is already degrading.

The order is no longer left to each call site. `is_connection_dead` in
connection.py is now the single place that decides, testing the payload family
first, and the four retry sites ask it instead of testing membership themselves:
`PgResultBackend._cursor` / `_store_with_reconnect`, `PgQueueClient._cursor` /
`send` / `delete`, and the barrier's `_recover_after_error` / `_run_with_reconnect`.
A rejected payload leaves the connection perfectly usable, so it also stops
being closed.

Tests: `TestPayloadRejectionIsNotAConnectionDeath` covers the predicate directly
(including an assertion that ProgramLimitExceeded really is an OperationalError,
so the test fails if psycopg2 ever changes the hierarchy this reasoning rests on)
and the behaviour end to end — one rejected INSERT then the degraded row, no
reconnect, connection not closed. Mutation-verified: restoring the bare
`isinstance` turns the gating lane red (2 failed, 20 passed).

Full workers suite 1501 passed, 1 skipped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4126 [FIX] Stop a genuinely failed task being recorded as completed-but-unstorable

Praveen's review finding on #2289, and it is correct.

`_write_outcome`'s `result is None` branch runs only when the task RAISED. When
the database refused its error text, that branch called `_insert_degraded`,
which wrote `PAYLOAD_UNSTORABLE_ERROR` -- and this PR's own contract makes that
exact string the signal for "the task completed; retrying this reply key is a
second full LLM spend". So the one case where a retry is not merely safe but
correct was being labelled as the one case where it must not happen, and the
task's real error message was dropped on top.

`ERROR_TEXT_UNSTORABLE` is the twin for the failure channel, saying plainly that
the task did NOT complete and the reply key is safe to retry. `_insert_degraded`
now takes the text as a keyword argument, defaulting to the completed-branch
constant since that is the common path. Two constants rather than one
parametrised string, so the difference is visible at both call sites.

The module docstring and the `PgTaskResult.status` comment previously described
`failed`'s third meaning as a single case; both now describe two, and say
explicitly that seeing `failed` is not enough -- recovery logic must match the
error text, because the two point in opposite directions.

Why the existing test missed it: `test_rejected_error_text_also_degrades_to_a_row`
asserted only that a second row was written with status `failed`, never what it
said. It now asserts the text, and a twin test pins the completed branch from the
other side; both assert the two constants are not each other, so swapping them
fails rather than silently passing. Mutation-verified -- restoring the original
call turns the gating lane red.

Full workers suite 1502 passed, 1 skipped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

…ns complete instead of timing out (#2294)

* UN-4137 [FIX] Stream LLM completions under the hood so long generations complete instead of timing out and being replayed

A non-streaming completion keeps the socket silent until the last token.
On Anthropic, generations the console finishes in ~16 minutes never
arrived: litellm.Timeout after 900 s (staging) and after 1800 s
(production), then replayed up to 4x by the retry helper because Timeout
is retryable, until the Celery time limit killed the task.

- LLM.complete() streams (stream=True), collects the chunks and rebuilds
  the full response with litellm.stream_chunk_builder; callers unchanged.
- collect_with_retry: retry only before the first content chunk; a drop
  after content started is raised immediately so a long generation is
  never replayed, and chunks from a failed attempt are discarded.
- "Enable Streaming" checkbox on all LLM adapter forms, default on;
  adapters without a stored value stream too. Opt-out for endpoints that
  cannot stream. Read from raw adapter metadata, never sent to litellm.
- Anthropic Timeout description now reflects per-chunk semantics.

Verified locally on the reporting project: 12-minute Sonnet 4.6
generations (64k / 67k completion tokens) complete with no retry under
the same 900 s / 3-retry settings that failed in staging.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* UN-4137 [FIX] Retry request errors raised while creating the stream

collect_with_retry created the stream outside its protected block, so an
error raised by fn() itself escaped on the first attempt. litellm's
streaming completion() sends the HTTP request when called, which is
exactly where a 429/5xx/connection error surfaces, so those requests
would have failed where the non-streaming path retried them.

Create the stream inside the try block; a failure there is a failed
request and retries like before. Tests cover fn() raising before
returning an iterable, both retryable and not, and the same through
LLM.complete() with a RateLimitError from completion().

Addresses Greptile finding on PR #2294.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* UN-4137 [FIX] Keep mocked completions on the non-streaming path

The e2e rig mocks the LLM through litellm's mock_response and counts
calls with litellm's fixed mock usage (10/20/30), which litellm reports
only on the non-streaming path; the streaming mock token-counts the real
prompt instead. A mocked completion never touches the network, so
streaming buys nothing there. Bypass streaming when a mock response is
injected, restoring the rig's contract.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

…nd a configurable metrics database (#2287)

* UN-4123 [FEAT] Support TLS to Redis, and keep pooled/idle connections healthy

Makes an encrypted connection to Redis possible so the platform can run against a
managed endpoint (Memorystore / ElastiCache / Azure Cache, which disables its
non-TLS port by default). Chart-side support for pointing at an external Redis is
UN-4122; password-only auth already worked, so what was missing was TLS.

Everything here is additive and inert by default: with nothing configured, the
local/in-cluster path builds exactly the client it built before.

**The scheme is the switch.** `{prefix}URL` (falling back to REDIS_URL) is handed
to redis.Redis.from_url, and `rediss://` selects TLS on its own — no separate
"use TLS" flag to forget, and `redis://` behaves as today. Discrete host/port vars
remain the primary path: they need no percent-encoding, and they are what the Helm
chart, every sample.env and the non-Python services (api-hub, llm-whisperer) read.

Two redis-py behaviours that bite silently, both handled and pinned by tests:

  * The URL path beats a `db=` kwarg. sdk1 metrics asks for db=1 explicitly, so a
    URL ending in /5 would have moved its keys into another service's keyspace
    with nothing to show for it. The path is stripped when an override is given.
  * `ssl=True` into a ConnectionPool does NOT fail at construction — the pool
    defers kwargs to the connection class, so platform-service (max_connections=10)
    started healthy, kept the PLAIN Connection class, and raised
    `TypeError: AbstractConnection.__init__() got an unexpected keyword argument
    'ssl'` on its first command. Pooled TLS now selects SSLConnection instead.

Also fixed, because they are the same class of silent failure:

  * `{prefix}SSL` falls back to REDIS_SSL. Enabling TLS platform-wide previously
    meant remembering CACHE_REDIS_SSL and MANUAL_REVIEW_REDIS_SSL too, and a
    forgotten one is a plaintext client dialling a TLS port.
  * django-redis 5.4.0 ignores both DB and USERNAME from OPTIONS (verified against
    the installed 5.4.0: make_connection_params reads only PASSWORD and timeouts).
    The db now travels in the LOCATION URL, so the backend cache stops sitting on
    db 0 while every other service honours REDIS_DB — with REDIS_DB=N the workers
    RPUSH log_history_queue to db N and the backend LPOPs an empty db 0. USERNAME
    is deliberately NOT restored: auth stays password-only as the built-in
    `default` user, which is what a managed AUTH string is.
  * kombu reads TLS off the scheme but defaults ssl_cert_reqs to CERT_NONE —
    encrypted while accepting any certificate. Socket.IO's manager URL carries an
    explicit ssl_cert_reqs, since KombuManager takes a URL, not kwargs.
  * The sidecar and tool-container environments are hand-picked allowlists (the
    trap that made the LOG_TRANSPORT fix necessary). TLS settings and REDIS_DB now
    reach both, and only when actually set — an empty string reads as "configured"
    to os.getenv and would suppress the fallback.

health_check_interval now defaults to 30s, configurable via
{prefix}HEALTH_CHECK_INTERVAL. Only the two worker caches set it before, so a
connection killed while parked — managed failover, or Azure Cache's 10-minute idle
reaper — was discovered by a real command failing. This is the one intentional
behaviour change on the existing path, and it applies with or without TLS.

Retries are deliberately NOT enabled globally: retry_on_timeout would re-issue
blocking BLPOP/BLMOVE calls whose reply was lost, which risks consuming a second
message rather than recovering the first.

Tests: 21 new in unstract/core/tests/test_redis_client_config.py, 5 added to the
runner sidecar suite. Core 136, runner 10, Redis-related worker tests 51 — green.
Django settings verified by rendering both modes: plaintext yields
redis://host:6379/0 with no pool kwargs; TLS yields rediss://…/3 plus
ssl_cert_reqs/ssl_ca_certs, and the Socket.IO URL gains ?ssl_cert_reqs=required.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FEAT] Add a local TLS Redis for dev testing, and fix the CA gap it found

A managed Redis differs from the dev one in two ways that matter to this change —
it requires AUTH and speaks TLS — and neither was reachable from a laptop, so the
TLS path had no way to be exercised before merge. `docker-compose-redis-tls.yaml`
runs a Redis that does both on port 6380, alongside the normal `unstract-redis`,
so a developer can flip between the two and confirm BOTH still work.

Its plaintext listener is disabled (`--port 0`), matching Azure Cache's default:
a component that fails to pick up the TLS settings cannot then quietly succeed
over plaintext and hide the bug.

**Running it immediately found one.** URL mode carries TLS in the scheme and never
sets `{prefix}SSL`, but the CA was read inside that flag's branch — so a
`rediss://` URL verified against the system trust store alone and could not talk
to any server with a privately-signed certificate. That is exactly the case the CA
option exists for (Memorystore's CA is Google-managed and not publicly trusted).
The read moved out of the gate; consumers decide whether it applies, so a
`redis://` URL still ignores it. Two regression tests cover both directions.

Verified live against the container, with the new client code:

  url rediss + CA                -> OK   [SSLConnection] roundtrip
  url rediss, no CA              -> FAIL CERTIFICATE_VERIFY_FAILED (expected)
  discrete REDIS_SSL=true + CA   -> OK   [SSLConnection] roundtrip
  plaintext against TLS port     -> FAIL connection closed  (expected)
  TLS, wrong password            -> FAIL AuthenticationError (expected)
  pooled TLS (platform-service)  -> OK   [SSLConnection] ping
  plain local redis (mode A)     -> OK   [Connection] ping

The negatives matter as much as the positives: they show a misconfigured client
fails loudly rather than silently degrading to plaintext.

redis-tls/README.md carries the switch-over runbook, including the one deliberate
asymmetry — the runner uses `ssl_cert_reqs=none`, because it forwards its settings
to tool sidecars and those get no CA mount (their environment is an allowlist and
only the shared log dir is mounted). Encrypted without verification still exercises
the forwarding fix and the handshake; a real managed endpoint does not hit this,
since ElastiCache and Azure chain to public CAs.

Certificates are gitignored — generate-certs.sh writes them locally, and the keys
are unencrypted dev material.

Core tests now 138.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FEAT] Ship generate-certs.sh, which .gitignore silently dropped

The repo ignores *.sh, so `git add -A` skipped the script and the committed
README referenced a file that did not exist in the branch — the dev harness was
unusable for anyone cloning it. Force-added, as every other tracked .sh in this
repo is.

Found by running the harness from a fresh checkout rather than the worktree it
was written in.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FIX] Honour REDIS_URL in the Django cache and Socket.IO manager

Found on a live run against the TLS Redis, not by reading the code: an API
deployment came back `execution_status: COMPLETED` with `result: null`.

`create_redis_client` honoured REDIS_URL from the start, but settings/base.py
still built its own URL from REDIS_HOST/REDIS_PORT — so with a URL configured the
WORKERS moved to the managed endpoint while the BACKEND stayed on the in-cluster
one. Nothing errors in that state; the two simply stop sharing a keyspace. The
execution really did run, its result really was cached — into the URL's Redis —
and the backend looked for it in the other server and found nothing. Verified by
key: `api_results:b2dd28d9…:d4355e01…` existed only in the TLS instance.

REDIS_URL now drives both the cache LOCATION and SOCKET_IO_MANAGER_URL. The CA is
appended as a query parameter rather than passed separately, because BOTH
consumers parse it out of the URL — confirmed against the pinned redis-py 5.2.1
(-> SSLConnection with ssl_ca_certs) and kombu 5.5.4 (-> CERT_REQUIRED +
ssl_ca_certs). An explicit ssl_ca_certs already in the URL is left alone, and a
plaintext redis:// URL never gets one.

In URL mode the db and credentials are dropped from OPTIONS: they travel in the
URL, and passing both invites one silently winning over the other — the same
class of bug as the db kwarg losing to the URL path in redis-py.

Tests: backend/backend/tests/test_redis_settings_derivation.py, 9 cases. They
execute the real source range from settings/base.py rather than a copy of the
logic, since the block runs at import and cannot be called. Covers the
plaintext default unchanged, the db-in-LOCATION fix, TLS pool kwargs, kombu's
CERT_NONE default, and URL mode in both directions.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* UN-4123 [FIX] Let a prefix's own DB survive an inherited REDIS_URL

Spotted while documenting the Helm keys, not at runtime — which is the only
reason it was caught before someone hit it in a cluster.

The chart configures ONE endpoint for the platform and sets CACHE_REDIS_DB=1 for
the worker cache. With the endpoint given as REDIS_URL, the CACHE_REDIS_ prefix
inherits that URL (no CACHE_REDIS_URL of its own) and took the db from its path —
so the worker cache would quietly move from db 1 to db 0 and sit beside every
other key. Nothing errors; the keys just relocate.

A prefix's own {prefix}DB now applies to an INHERITED url. An explicit
{prefix}URL is untouched, since it names its db deliberately, and an explicit
db= argument still beats both (sdk1 metrics depends on that).

Three tests pin the precedence chain: inherited URL + prefix db, explicit prefix
URL, and the db= argument. Suite now 26.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* UN-4123 [FIX] One Socket.IO URL builder, so the worker publisher gets TLS too

Review caught that enabling REDIS_SSL moved the BACKEND's Socket.IO manager to
rediss:// while workers/log_consumer/tasks.py kept a hardcoded redis://. Against
a TLS-only endpoint that publisher simply cannot connect, and execution-log
events stop reaching the UI — the backend meanwhile looking perfectly healthy.

The cause is that both files assembled this URL by hand, so they could drift; the
fix is one builder in unstract.core that both import, not two corrected copies.

Two more holes it closes, both specific to kombu taking a URL and nothing else:

  * ssl_cert_reqs was absent in URL mode. Kombu defaults a rediss:// URL to
    CERT_NONE, so TLS was encrypting traffic to a server it never authenticated —
    the connection an operator believes is verified is exactly the one that isn't.
  * ssl_ca_certs never reached Socket.IO at all. The Django cache gets the CA
    through pool kwargs, so against a privately-signed server (Memorystore) the
    cache would work while WebSocket delivery silently died.

An explicit ssl_cert_reqs already in the URL is left alone, and a plaintext
redis:// URL gains no TLS query.

Verified live, not just in unit tests: kombu connects to the local TLS Redis with
CERT_REQUIRED against the generated CA. 9 new cases in TestSocketIoUrl; core +
backend suites 180 green, runner 10, worker log-stream 17.

The backend's settings test moved its Socket.IO assertions to the core suite,
which is where that logic now lives.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* UN-4123 [FEAT] Make the sdk1 metrics database configurable, for single-database endpoints

sdk1's MetricsMixin hardcoded create_redis_client(db=1) — the last place in the
codebase that chose a Redis database in code rather than in configuration, and
the only code blocker to running against a SINGLE-DATABASE endpoint.

That matters because a whole class of managed Redis exposes db 0 only: Azure
Managed Redis, Redis Enterprise / Redis Cloud, and every cluster-mode service
(ElastiCache and Memorystore included). Against those, this client's SELECT 1
fails on first command — caught by the existing try/except, so the run survives
but the time-taken metric is silently lost and the logs fill with errors.

METRICS_REDIS_DB defaults to 1, the value that was hardcoded, so an unset var
leaves every existing deployment byte-identical. A single-database deployment
sets it to 0 alongside CACHE_REDIS_DB and FILE_ACTIVE_CACHE_REDIS_DB — those
three are the whole of Unstract's on-prem database map. Multi-database stays the
default and needs no configuration at all.

Read per instance rather than once at import, so a malformed value raises inside
__init__'s existing try/except (costing the metric) instead of killing the
process at import time.

The tool-container allowlist gains the key too. Tool containers build their own
Redis client from a hand-picked environment, not an inherited one, so without
this entry the setting would apply everywhere EXCEPT the containers doing the
work — the same silent, partial failure as the LOG_TRANSPORT allowlist miss in
UN-3755. The SIDECAR allowlist deliberately does not: it publishes logs and
never imports sdk1.

Note for operators, documented in workers/sample.env: CACHE_REDIS_DB and
FILE_ACTIVE_CACHE_REDIS_DB must always match. Workers write file_active:* to one
and the backend reads from the other, so splitting them stops active-file dedup
finding anything — no error, just files reprocessed as new.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FIX] Import pytest as a module in the metrics-db test

SonarCloud python:S9084. `from pytest import MonkeyPatch` was only there to give
the fixture a type annotation for ruff ANN001; `pytest.MonkeyPatch` does the same
job without the from-import.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FIX] Verify the server's hostname, and resolve ssl_cert_reqs once

Two review findings, both reproduced against the pinned redis-py 5.2.1.

1. TLS was never authenticating the server. redis-py defaults ssl_check_hostname
   to False and then OVERRIDES ssl.create_default_context()'s safe default with
   it, so every path this work added — discrete kwargs, Redis.from_url on
   rediss://, and kombu — validated the chain but not the identity. For the
   ElastiCache/Azure case this targets (public CA, no pinned ssl_ca_certs) that
   means any publicly trusted certificate for any domain is accepted, and an
   on-path attacker can terminate the connection and read or write execution
   state, cached results and log traffic. Encryption without server
   authentication is not what enabling TLS is understood to buy — the same
   argument this module already makes about kombu's CERT_NONE default.

   ssl_check_hostname now defaults to True, overridable per prefix via
   {prefix}SSL_CHECK_HOSTNAME falling back to REDIS_SSL_CHECK_HOSTNAME, and is
   forced off when ssl_cert_reqs is "none" because Python's ssl module raises on
   that combination. Applied in all four places: the discrete kwargs, the URL
   path, the Socket.IO/kombu URL, and the Django cache pool. The dev harness is
   unaffected: docker/redis-tls issues SANs for unstract-redis-managed,
   localhost and 127.0.0.1.

2. ssl_cert_reqs was resolved three different ways in one module, with two live
   consequences, both executed:

   * URL mode never read it. `REDIS_URL=rediss://host:6380/0` with
     REDIS_SSL_CERT_REQS=none gave the data-plane clients redis-py's default
     while build_socketio_redis_url emitted ?ssl_cert_reqs=none — one process,
     two verification policies, and nothing to say which was intended.
   * Prefixed clients had no generic fallback, unlike {prefix}SSL and
     {prefix}SSL_CA_CERTS which this work already gave one. REDIS_ honoured
     "none" while CACHE_REDIS_ and MANUAL_REVIEW_REDIS_ stayed "required", so the
     worker cache alone failed verification — and cache_backends.py catches that,
     logs a warning and sets available=False, degrading silently to no-cache.

   Now resolved once, outside the ssl gate and with the generic fallback, exactly
   as ssl_ca_certs already was; applied in both the discrete and URL paths, with
   a setting already present in the URL's query string still winning.

One existing expectation updated deliberately, not to reach green:
test_ssl_switches_scheme_and_pool_kwargs asserted the exact pool-kwargs dict and
now includes ssl_check_hostname, because the rendered behaviour changed on
purpose. Eight new tests cover prefix inheritance of cert_reqs, URL mode, the
URL-query precedence, the hostname default, the forced-off case, the opt-out and
the Socket.IO URL. 53 green across the core and backend Redis suites.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FIX] Split the URL composer out of build_socketio_redis_url

SonarCloud python:S3776 — the hostname-verification branch added in 3bc8aaa
pushed the function's cognitive complexity from under the limit to 17.

The assembly of a URL from the discrete vars was the part that did not belong:
that function exists to get kombu's TLS settings into a query string, and it was
also a URL builder. Moved to _compose_redis_url, leaving one expression at the
call site. No behaviour change — 53 tests green across the core and backend Redis
suites.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FIX] Test the settings that ship, not a copy of them, and pin the credentials

Two testing findings, both mutation-verified by the reviewer and re-verified here.

1. The harness re-implemented the very lines this work adds. Its docstring says it
   executes the shipping source "instead of re-implementing it, which would test a
   copy rather than the thing that ships" — but the exec'd slice started at
   REDIS_SENTINEL_MODE (base.py:531), while REDIS_SSL, REDIS_SSL_CERT_REQS,
   REDIS_SSL_CA_CERTS and REDIS_URL are defined at :112-121. All four ran from a
   hand-written prelude instead, so the shipping definitions were untested and the
   copy could not diverge visibly.

   The prelude now keeps only the three imports and splices in the real
   definitions range. Both of the reviewer's mutations, which previously left the
   suite green, now fail: a typo'd REDIS_URI (which re-introduces the exact
   REDIS_URL-ignored regression this file exists to pin) fails 9 tests, and a
   flipped REDIS_SSL default fails test_plaintext_is_unchanged.

2. Discrete mode's cache credentials were never asserted. Discrete mode is the
   default for every existing on-prem and Helm deployment and the only path where
   the password reaches Redis through OPTIONS — in URL mode it rides inside the
   URL. Removing the PASSWORD assignment left the suite green while the cache
   authenticated as nobody: every read and write failing at runtime, nothing
   failing at import. That block is now built conditionally in this PR, so the
   credentials are newly reachable-or-not depending on a branch.

   Three cases added: the password reaches OPTIONS (mutation-verified red), DB and
   USERNAME are passed through, and URL mode does NOT duplicate them. The USERNAME
   assertion pins the stated invariant — django-redis 5.4.0 discards it, so
   password-only auth currently holds by accident of the pinned version; the test
   makes a bump that starts honouring it a deliberate decision rather than a
   surprise.

Also documented in docker/redis-tls/README.md why the recipe now works: the
reviewer was right that REDIS_SSL_CERT_REQS was ignored in URL mode, and the fix
in 3bc8aaa is what makes the documented dev setup correct as written. The URL
query form is noted as an equivalent that wins if both are set.

61 green across the backend, core and sdk1 Redis suites.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FIX] Blank values, a lever that never arrived, and a silent TLS downgrade

Three review findings, all reproduced.

1. A declared-but-BLANK METRICS_REDIS_DB disabled every sdk1 metric. An empty
   variable is this repo's own convention for "leave the default"
   (CACHE_REDIS_PASSWORD=, REDIS_SSL_CA_CERTS=), and every other variable in this
   work treats it that way — this one raised int(""). The exception was caught, so
   the claim that it costs the metric rather than the process held, but the
   consequence was every LLM and x2text timing silently missing platform-wide,
   once per instrumented call, behind a log line naming Redis rather than the
   variable. Blank now means unset; an unparseable value falls back to db 1 and
   says which variable it came from.

   The old test codified "" as malformed, so it was rewritten rather than
   adjusted: losing every metric is a bad trade for a typo.

2. REDIS_HEALTH_CHECK_INTERVAL reached neither container allowlist, though
   _resolve_health_check_interval reads it in the sidecar and in every tool
   container, three sample.env files document it, and the PR body names
   REDIS_HEALTH_CHECK_INTERVAL=0 as THE way to restore the previous behaviour for
   this work's one intentional default change. That lever stopped at the pod
   boundary. Added to both allowlists and both forwarding tuples. Benign in
   itself — an extra PING on connections idle past 30s — but it is exactly the
   trap the comments beside it were written to warn about.

3. REDIS_SSL=true beside an older plaintext REDIS_URL downgraded silently and
   then crashed the Django cache. The URL wins for the connection, so traffic went
   in clear while the operator believed TLS was on; meanwhile the cache's pool
   kwargs were gated on the flag alone, handing ssl_cert_reqs to a plain
   redis.Connection — verified: TypeError on first use, in a request rather than
   at startup. The pool kwargs now follow the EFFECTIVE scheme, and
   _resolve_redis_env logs an error when a URL's scheme disagrees with
   {prefix}SSL, since that combination is never deliberate.

Tests: blank/unparseable/metrics-still-enabled for the sdk1 db, and
TestSchemeFlagMismatch for the cache — the latter mutation-verified red against
the flag-only gate. 63 green across the backend, core and sdk1 Redis suites.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FIX] Name the TLS scheme instead of repeating the literal

SonarCloud python:S1192 — the scheme-mismatch check added in 178f1c7 made
"rediss://" a third occurrence.

Extracted as _TLS_SCHEME with a note on why it carries weight here: the scheme IS
the TLS switch for redis-py, kombu and django-redis alike, which is the reason
URL mode has no separate flag. No behaviour change; 65 green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FIX] TLS for Sentinel's discovery connections, and warn when the cache moves db

1. With REDIS_SSL=true, Sentinel deployments encrypted the master connection and
   left DISCOVERY in plaintext. _build_connection_kwargs returns from its
   auth-only branch before the ssl block, and redis-py builds the Sentinel clients
   from those kwargs alone — so against a Sentinel still accepting plaintext the
   Sentinel password went out in clear while the operator believed one switch had
   turned TLS on everywhere; against a TLS-only Sentinel, discovery failed through
   the full ten-attempt backoff first.

   The asymmetry predates this work, but this work is what makes REDIS_SSL the
   documented one-switch way to enable TLS, which is what brings operators to it.
   The discovery connections now follow {prefix}SSL rather than getting a switch
   of their own: a Sentinel node IS a redis-server and takes the same tls-port
   configuration, so the two never差 in practice. Verified — discovery is now
   SSLConnection and the master stays SentinelManagedSSLConnection.

   The review also checked and rejected a TypeError on the master path:
   SentinelConnectionPool pops `ssl` and selects the SSL class itself. That
   matches what this module found earlier and is why only the auth-only branch
   needed changing.

2. The Django cache's move from db 0 to REDIS_DB is correct — django-redis 5.4.0
   ignores OPTIONS["DB"], so this cache always sat on db 0 — but it is a one-way
   RELOCATION of live data, not the inert change the PR describes. CacheService
   wraps get_redis_connection("default"), so log_history_queue, the rate-limit
   counters and the dashboard caches move with it, and a rolling deploy has old
   pods on db 0 while new pods read db N. No shipped config sets a non-zero
   REDIS_DB for the backend, which is what keeps it survivable — but it should not
   be silent, so it now logs a warning naming the database and what stays behind.

Tests: Sentinel TLS on both planes, and the plaintext Sentinel unchanged — there
was no Sentinel+TLS coverage at all before. 67 green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FIX] One TLS kwargs builder, shared by both connection branches

SonarCloud python:S3776 — giving Sentinel's discovery connections TLS in
59df87e duplicated the TLS block into both branches of
_build_connection_kwargs and pushed it to complexity 17.

Extracted as _tls_kwargs, which fixes the duplication and the complexity
together. That duplication is exactly how the bug arose in the first place: the
auth-only branch returned before the TLS block, so one plane was encrypted and
the other was not. A single helper means the two cannot drift again — worth more
than the complexity score on its own.

No behaviour change; 67 green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [DOCS] Correct the database map, the USERNAME comment, and the tense

Three doc-accuracy findings. The first is the one that would have caused harm.

1. workers/sample.env told operators to set FILE_ACTIVE_CACHE_REDIS_DB in the
   WORKERS' environment, where nothing reads it — it is read only by the backend
   (settings/base.py, consumed in file_history_helper.py). Following that
   guidance sets it inertly, leaves the operator believing the pair is aligned,
   and produces exactly the silent dedup failure the same paragraph warns about.

   The same block called three keys "the whole of Unstract's on-prem database
   map". There are FOUR: REDIS_DB also selects a database — for the Django cache
   LOCATION and every REDIS_-prefixed client — so collapsing the other three onto
   a db-0-only endpoint while REDIS_DB stays non-zero still leaves clients on a
   database the endpoint does not have. It is the easiest to miss because it is
   already set, higher up the same file.

   Rewritten as a per-process table, since which ENV a key goes in is the whole
   point.

2. "USERNAME is deliberately NOT restored" sat eleven lines above code setting
   both USERNAME and DB. Both are genuinely inert — django-redis 5.4.0's
   make_connection_params reads PASSWORD and the two timeouts and nothing else —
   but a reader working out why username auth never reaches Redis found a comment
   and code that disagreed, with two dead OPTIONS keys looking load-bearing. The
   comment now says what is true: they are passed for readability and discarded,
   auth stays password-only, and the tests pin it so a dependency bump that starts
   honouring USERNAME is a visible change rather than a silent one.

3. Several comments narrated an INTERMEDIATE state of this branch as shipped
   history — "the backend learned TLS while the worker kept a hardcoded redis://",
   "create_redis_client honoured REDIS_URL from the start while this module built
   its own". Neither was ever true on main: both conditions existed only between
   commits here. Post-merge they read as a shipped incident, and this repo writes
   RCAs from exactly this kind of comment. The rationale was right and is kept —
   only the tense changed, to the hazard the code prevents rather than events that
   never happened.

67 green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FIX] One database rule, a dev recipe that is not quietly broken, and the CA caveat

Three review findings.

1. The database precedence could not be stated, because there were two rules. An
   INHERITED generic URL honoured {prefix}DB — deliberately, since the chart ships
   one REDIS_URL with CACHE_REDIS_DB=1 beside it — while a prefix's OWN url did
   not. Worse than reported: an explicit REDIS_DB beside a pathless REDIS_URL gave
   db None, neither the env value nor a sane default.

   Now uniform: a URL supplies host, port and credentials; the database is
   {prefix}DB when explicitly set, otherwise the URL's path, otherwise 0; an
   explicit db= argument beats all of them. Stated in the module docstring and in
   workers/sample.env, where an operator meets it.

   One existing expectation changed deliberately: a prefix's own URL no longer
   keeps its db against an explicitly-set {prefix}DB. That test encoded exactly
   the inconsistency being removed. Leaving {prefix}DB unset gives the URL's db,
   which is covered by its own case.

   Not changed: the reviewer also suggested dropping URL inheritance per prefix.
   That would silently DOWNGRADE the worker cache to plaintext under a rediss://
   URL, since the discrete path needs CACHE_REDIS_SSL and the scheme carries TLS
   instead — so inheritance stays and the rule is documented rather than removed.

2. docker/redis-tls/README.md put the workers' cache on db 1 while the backend
   kept FILE_ACTIVE_CACHE_REDIS_DB at its default 0 — the workers writing
   file_active:* to one database and the backend reading another, which is the
   silent dedup failure workers/sample.env warns about in this same PR. Anyone
   following it validated the TLS work on a quietly broken config. The recipe now
   sets FILE_ACTIVE_CACHE_REDIS_DB=1 and says why.

3. REDIS_SSL_CA_CERTS is forwarded into tool containers and sidecars as a PATH,
   and the runner mounts only the shared logs volume — so unless an image bakes
   the CA in at that path, load_verify_locations() raises on a file that is not
   there: the sidecar cannot build its LogPublisher and tool logs are lost.
   Kept forwarded, because the baked-in case is real and is the only way this
   works today, but the caveat now lives in runner/sample.env and at both
   allowlists rather than only in the dev README.

73 green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FIX] One TLS-query builder, and a guard against allowlist drift

Two review findings, both about the same failure mode: a second copy of something.

1. base.py hand-built the TLS-query append ten lines above calling the shared
   builder, and the two copies already disagreed — unstract.core added
   ssl_cert_reqs AND ssl_ca_certs, base.py only ssl_ca_certs, so one process held
   two verification policies for one endpoint. That is precisely the drift the
   shared builder was introduced to prevent, reintroduced in the same change, and
   it is the mechanism behind the URL-mode cert_reqs bug fixed earlier.

   Extracted as ensure_tls_query_params — "given a Redis URL, add the TLS settings
   it is missing" — called from both. build_socketio_redis_url is now the thin
   wrapper that adds the discrete-vars fallback on top. Anything handing a URL to
   a library that reads TLS only from the query string wants this; kombu and
   django-redis both do.

   The helper now uses safe="/" so a CA path stays readable rather than arriving
   percent-encoded; two expectations that pinned %2F were updated to match, and
   the result still round-trips through urlsplit/parse_qs.

2. Added the allowlist test the review asked for — the one assertion that would
   have caught the REDIS_HEALTH_CHECK_INTERVAL omission from BOTH container lists
   and the METRICS_REDIS_DB divergence between them. It reads both constants files
   as text (neither package is importable here, and the question is what the
   source declares) and asserts every variable create_redis_client reads is
   forwarded, plus that METRICS_REDIS_DB is in the tool list and deliberately NOT
   in the sidecar's, since the sidecar publishes logs and never imports sdk1.

   Mutation-verified: removing the health-check entry from the sidecar list fails
   it.

The review's other four gaps were closed by the fixes they accompanied — prefix
inheritance of ssl_cert_reqs, URL mode plus the env var, Sentinel with TLS, and
REDIS_SSL beside a plaintext URL — as was the sdk1 test that codified a blank
value as malformed. 76 green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FIX] Resolve TLS once from the URL as well as the env, and derive the guards

Eleven findings from a four-agent review of 3690a07..8616202. Nearly all of
them were introduced by that range: each previous fix was applied at one site
while the same defect class sat untouched at the next one. Every fix here is
mutation-verified — eleven mutations, each caught by a named test.

CRITICAL. A URL carrying ?ssl_cert_reqs=none got ssl_check_hostname=true
appended beside it, and Python's ssl module raises on that pair rather than
degrading: every Redis connection in the process failed at the first command,
across the client, the Socket.IO URL and the Django cache LOCATION. The
suppression keyed on the ENV value and never looked at the URL's query string —
which is the form docker/redis-tls/README.md documents, and what redis-py, kombu
and django-redis all actually honour. effective_cert_reqs() now decides it, at
all three sites. The existing test fed this exact input and passed because it
only asserted the value was present.

THE DATABASE RULE reached create_redis_client but not the Django cache, so one
REDIS_URL plus one REDIS_DB resolved to two different databases: workers RPUSH
log_history_queue to db N while the backend LPOPs an empty db 0, silently.
set_url_db_path()/url_db_path() are now shared by both sides, and a test asserts
they agree for the same environment rather than merely both being right today.

The rule also let the generic REDIS_DB cross into a prefix that brought its own
URL. workers/sample.env ships REDIS_DB=0 uncommented, so CACHE_REDIS_URL=…/1
landed on db 0. The db var at the URL's OWN level now governs; an inherited
REDIS_URL still honours the prefix's db, which is the shape the chart ships.

BLANK MEANS UNSET, everywhere this time. REDIS_SSL_CHECK_HOSTNAME was added by
the previous range with a bare == "true", so the repo's own "leave the default"
spelling read as False and quietly turned server authentication off. A blank
{prefix}SSL_CERT_REQS resolved three different ways in one process. Both now go
through one parse that accepts the usual spellings, warns by name on anything
else, and treats blank as unset.

SENTINEL + TLS was regressed by the same range. Masters are reached at the IP
SENTINEL get-master-addr-by-name returns, which SSLConnection passes as
server_hostname, so defaulting hostname verification on would have failed every
existing REDIS_SENTINEL_MODE + REDIS_SSL deployment on upgrade, through the full
ten-attempt backoff. The master plane skips it by default; discovery, which uses
the configured service name, keeps it. An explicit setting is honoured on both —
a typo is not, or it re-breaks exactly what the default protects.

THE GUARDS ARE DERIVED, NOT LISTED. REDIS_SSL_CHECK_HOSTNAME reached neither
container allowlist and was missing from the drift-guard set added in the same
commit to catch that. _SHARED is now regexed out of redis_client.py with an
explicit list of deliberate exclusions, so the next variable fails a test instead
of needing a third hand-maintained list; a second test asserts the forwarding
TUPLES, which are what actually reach the container. Deleting
Env.REDIS_HEALTH_CHECK_INTERVAL from runner.py — the omission the guard was
written for — used to leave the suite green.

Tests: core 57 -> 91, backend 15 -> 30. The backend harness now splices base.py's
real import block as a third source slice instead of hand-listing it, and sets
__name__ so the relocation warning is reachable — part of why it was untested.
That warning also had a bug of its own, caught by its first test: it compared
REDIS_DB against the URL's path, announcing a move for a plain redis://h/3 that
never moves.

Also: a blank {prefix}DB no longer escapes as a bare ValueError; the dead
cert_reqs fallback in build_socketio_redis_url is gone; four docstrings that
contradicted the code are corrected; the CA caveat sits above the constant it
describes; the TLS README's two-database recipe sets CACHE_REDIS_DB explicitly,
without which it produced the exact dedup split its own paragraph warns about;
and workers/sample.env ships the prefixed TLS vars commented out, since shipping
them set defeated the generic fallback for every deployment derived from it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FIX] Share parse_db with the backend, and tighten three guards that could not fail

A scoped review of 418e78b found no Critical or High — the first round in four
where the previous round's fixes introduced no blocking defect. These are the
three Mediums and three Lows it did find. Seven mutations, all now caught.

parse_db IS NOW SHARED. It was introduced in the last commit with a docstring
explaining why `int(os.getenv(...))` on a database var takes the process down at
import — and three sites in backend/settings/base.py still did exactly that with
the same variables: the cache db, the Sentinel branch's db, and
FILE_ACTIVE_CACHE_REDIS_DB. So the two halves disagreed on a malformed value:
workers warn and continue on db 0, the backend refuses to start. That is the
"fixed here, not there" shape this whole series keeps producing, one more time.

FILE_ACTIVE_CACHE_REDIS_DB sits ~350 lines above the derivation block, outside
both slices the exec harness runs, so no _derive() case can observe it. It is
pinned by a source check instead, which reaches all three sites at once — the
defect was never one line, it was one expression repeated wherever a database
var is read.

THE FORWARDING GUARD MATCHED ANYWHERE IN THE FILE. The last commit rewrote it to
assert the tuple rather than the constants declaration, because asserting the
declaration let a deleted tuple entry through. It searched the whole file text,
so COMMENTING OUT a tuple entry still passed — the same "present somewhere is not
present where it matters" weakness, moved one level down. It now slices the
tuple's own region and drops commented lines. Both mutations fail.

SAMPLE.ENV SHOWED THE TRAPPING VALUE. All three files displayed
`# REDIS_SSL_CHECK_HOSTNAME=true`, and an explicit value is honoured on Sentinel
MASTER connections — so uncommenting the line, which reads as "turn on the
documented default", silently flips masters from False to True and breaks exactly
the deployments the default was added to protect. They now show `false`, the only
value there is a reason to set, and say why setting it explicitly is not a no-op.
runner/sample.env was also the one block missing the Sentinel caveat.

Tests: core 91 -> 100, backend 30 -> 35. New coverage for behaviours that had no
test capable of failing: url_cert_reqs taking the FIRST repeated value (matching
redis-py's parse_url) and normalising case/whitespace; a prefixed client in
DISCRETE mode still inheriting the generic REDIS_DB, which the "at the URL's own
level" rule must not narrow; the backend's cert_reqs normalisation.

Three blank-value tests gained `assert ... not in caplog.text`. Without it "blank
means unset" is indistinguishable from "blank is unparseable, warn and fall back"
— both return the default, but only one is silent, and the other logs on every
client. One of those assertions is load-bearing in a non-obvious place:
_resolve_redis_env strips before calling parse_db, so parse_db's own .strip() is
unreachable through create_redis_client; base.py reads REDIS_DB raw, so the
backend test is the only thing that can pin it. Verified by mutation rather than
assumed — the first version of the assertion sat in the core suite, where it
could never fail.

Also corrects a test docstring that described a different test than the one it
sat on.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [DOCS] The TLS port is provider-specific, and 6380 is not a default

Both sample.env files presented `rediss://:<password>@<host>:6380/0` as the TLS
form with no note on the port. 6380 is our own docker/redis-tls convention and
Azure Cache's port; it is wrong for the other two providers these files name:

    GCP Memorystore        6378   (plaintext instances use 6379)
    AWS ElastiCache        6379   (same port; TLS is in-transit)
    Azure Cache for Redis  6380

Found by creating a TLS Memorystore instance and watching it come up on 6378
after I had written 6379 into a deployment manifest from memory. A reader
following these files has no reason to doubt the number.

The failure mode is the reason this is worth a note rather than left implicit:
a wrong port does not error. The prerequisite init containers loop on
`nc -z <host> <port>` forever, so every pod sits in Init with no event and no
log line, and it reads as a firewall or peering problem rather than a typo in a
port. That is the same "fails silently and late" class the rest of UN-4123
documents.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* [FIX] Stop tox 4.64's .venv redirect from breaking every CI run

CI went red on every branch at once on 2026-09-24, with no commit to blame:
a branch that passed on 09-23 failed on 09-24 with byte-identical errors.

`uv tool install tox --with tox-uv` is unpinned and resolves at job time, so
tox 4.63.0 -> 4.64.1 landed in CI on its release day. tox 4.64.0 ships a PEP
832 `.venv` redirect: after each run it writes a *file* named `.venv` at the
project root, holding the path of a tox env, so editors can find an
interpreter. It skips this when a real `.venv` directory already exists, which
is why no local checkout noticed and only CI, which starts clean, broke.

The repo root is also a uv workspace. uv reads `.venv` as the project
environment and refuses a file:

  error: Project virtual environment directory `.../.venv` cannot be used
  because expected directory but found a file

That is exactly the three root-workdir invocations: the unit-rig group, the
e2e-smoke group (whose failure then SKIPs the other seven e2e groups), and the
final `uv run --with coverage[toml] coverage combine`. Both tiers exit 2 and
report fails behind them. Every Python test in the unit tier passed throughout.

Disabling the redirect is the fix, not a workaround. Nothing in this repo wants
it: the redirect aims at .tox/<env>, ~97 packages of pytest/testcontainers/
coverage, while the groups pointed at that root need the ~269 package workspace
environment. A uv that honoured PEP 832 would run them in the wrong env
silently, which is worse than today's loud failure.

There is no newer tox to move to instead -- 4.64.1 is the latest release, and
it keeps the feature (4.64.1 only fixes an unrelated override bug). The
version-only alternative is pinning back to 4.63.0, which forfeits fixes and
re-breaks on the next deliberate bump.

tox and tox-uv are now pinned as well, so the next release is a bump we make
rather than one CI makes for us.

Verified locally against the real versions:
  * tox 4.64.1 before the change: `.venv` is a 9-byte file containing
    `.tox/rig`, and `uv run` at the root reproduces the CI error verbatim
  * tox 4.64.1 after: no `.venv` file, and `uv run` at the root succeeds
  * tox 4.63.0 after: accepts the key without warning, so the change is safe
    either side of the bump

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* [FIX] Install the CI toolchain from wheels only

Sonar flagged the three `uv tool install` lines this PR touches (C Security
Rating on New Code, blocking the quality gate): omitting --no-build permits a
source distribution's setup script to execute during install. The lines are
older than this PR, but editing them made them new code, so a dormant rule
became a gate failure.

Worth fixing rather than waiving. A CI runner installing the test toolchain is
exactly where an sdist build hook would be cheapest to abuse, and the flag
costs nothing here: tox, tox-uv, uv, virtualenv and the rest of the tree all
publish wheels.

Verified with uv 0.6.14 -- the version setup-uv pins, and so the one that
actually runs this command -- that --no-build is supported there, that the
resolution is byte-identical (uv==0.12.18, virtualenv==21.12.0,
tox-uv==1.36.0), and that the resulting tox 4.64.1 runs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

* UN-3487 [FIX] Require and enforce a bucket on the S3/MinIO connector

The S3/MinIO connector had no bucket field, so its reach was bounded only
by whatever the underlying credential (often a shared IAM role) could see
account-wide. Any connector could browse into any bucket that role had
access to, including other teams' data.

- `bucket` is now a required field on the connector schema, matching
  Azure's existing (but unenforced) precedent — this one is actually
  enforced.
- `MinioFS.get_fsspec_fs()` wraps the filesystem in fsspec's own
  `DirFileSystem` when a bucket is set, confining every list/read/write
  (and `test_credentials`) to that one bucket regardless of what the
  credential could otherwise reach. UCS opts out via `_REQUIRES_BUCKET`,
  since it restricts access through its own `path` setting instead.
- Fixed a related IDOR in `file_management`: `list`/`download`/`upload`
  resolved a connector by a bare `ConnectorInstance.objects.get(pk=id)`
  with no ownership/group check, org-scoped only. Any org member holding
  a connector's id could browse/read/write through it regardless of
  sharing. Now scoped through `ConnectorInstance.objects.for_user()`, the
  same queryset every other connector view already uses.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* UN-3487 [FIX] Stop double-prefixing bucket-scoped root-level operations

Greptile flagged (P1): DirFileSystem's own `.path` attribute is the
wrapped bucket prefix, but file_management_helper's root-path fallback
treated it as a connector-level default root and substituted it as the
operation path — every root-level list/upload then resolved to
`<bucket>/<bucket>/...` instead of `<bucket>/...`.

- Factored the fallback into `_default_root_path()`, which now excludes
  DirFileSystem explicitly: its `.path` never needs re-applying, every
  operation already resolves relative to it.
- Added regression tests exercising the real `FileManagerHelper.list_files`
  / `upload_file` against a bucket-scoped MinioFS with a mocked S3 backend,
  asserting the underlying call receives a single bucket prefix.
- Fixed the same root cause in the e2e ETL fixture: its connector had no
  `bucket` (now required), and its configured `folders`/`outputFolder`
  included the bucket name as a path segment — now implicit via the
  connector's own scope, per the same fix.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* UN-3487 [FIX] Update the live-MinIO integration test for the required bucket

TestMinoFS::test_minio asserted that connecting without a bucket lists
every bucket in the account — exactly the behavior this ticket removes.
Bootstrap the test bucket via a plain S3FileSystem (the connector itself
can no longer discover buckets this way), then assert the bucket-scoped
connection lists successfully.

CI signal: this is what "test (integration)" failed on in both runs, not
a new issue. The accompanying test_pg_barrier.py failures in that same
job are a pre-existing, unrelated CI DNS flake (identical on both runs,
"could not translate host name unstract-db"), not caused by this PR.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* UN-3487 [FIX] Fix DirFileSystem.walk() leaking the bucket into file names

Found by tracing the e2e ETL failure through the compose stack's own
logs (docker-compose-logs.txt from the CI run), not assumed: the worker
log showed "Failed to prepare input file and metadata:
unstract/unstract/e2e-in-.../probe.txt" — a double bucket prefix, but
from a different cause than the earlier file_management_helper fix.

fsspec's DirFileSystem.walk() relpaths the directory string it yields,
but not the `name` field inside each file/dir entry's own metadata dict
— those still carry the wrapped fs's raw, bucket-prefixed key (ls()
already fixes every entry; walk() doesn't, confirmed by reading fsspec's
own source and reproducing it directly against a mocked S3FileSystem).
Workflow-execution file discovery walks (UnstractFileSystem.list_files,
shared by backend and workers); the UI file browser lists — which is why
this was invisible to the file_management-focused testing so far.

_BucketScopedFileSystem wraps DirFileSystem and relpaths both dict keys
and each entry's `name` field for walk()/_walk(), so discovery sees the
same bucket-relative paths ls() already produced correctly.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* UN-3487 [FIX] Handle walk()'s default detail=False shape correctly

Greptile P1: the walk()/_walk() override always treated dirs/files as
detail=True dicts, calling .items() unconditionally. fsspec's own
default is detail=False (confirmed by reading AbstractFileSystem.walk's
source), which yields plain basename lists — every caller using the
default contract got an AttributeError instead of results.

Also caught, verified empirically against the real base implementation:
my own relpath was wrong on the dict-keyed (detail=True) path too. The
dict key is already a bare basename — only each entry's own `name`
field carries the bucket-qualified path. Relpath-ing the key itself
would hit `DirFileSystem._relpath`'s own assertion on real data (a
basename never starts with the bucket prefix); the prior test happened
to pass only because its mock's key and `name` field were identical,
which isn't how fsspec actually shapes walk() results.

Re-checked Greptile's second (repeated) finding on this same review —
the file_management_helper double-prefix fix from an earlier commit is
still intact: `_BucketScopedFileSystem` is a `DirFileSystem` subclass,
so the existing `isinstance(fs, DirFileSystem)` exclusion still applies.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* UN-3487 [FIX] Move the Bucket field to the bottom of the S3/Minio connector form

The connector config form renders fields in json_schema.json's property
order. Bucket landed second, right after the connector name, ahead of
the credential fields users fill in first. Moved it to the end so the
form still reads top-to-bottom the way users configure it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* UN-3487 [FIX] Wrap long S3/MinIO error text instead of overflowing the toast

Connector error messages come wrapped in a markdown code fence
(handle_s3fs_exception), which CustomMarkdown renders as a bare <pre>.
<pre> defaults to white-space: pre, so a long single-line message like
the "Invalid Secret" one never wraps — it just overflows past the
fixed-width toast instead of staying inside it. Added pre-wrap +
overflow-wrap so long connector errors wrap like the rest of the toast.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

…ack on each child's first task (#2293)

* fix: stop the file_processing worker booting the executor stack

Each pg_queue_consumer child paid ~9s of lazy initialisation on its first
task. The supervisor preforks 20 children per pod, so that is ~170
CPU-seconds per pod spent during serving rather than at startup, which
spikes fleet CPU at load onset and inflates CAST AI's WOOP p99 to 792m
against a 200m request -- putting the HPA signal's ceiling below its own
target so the fleet collapses to minReplicas under backlog.

structure_tool_task.py imports two string constants and a StreamMixin shim
from the executor package. None needs the executor registry, but every
import under `executor` booted the whole thing: executor/__init__.py
imported .worker, which imported executor.executors, which imported
LegacyExecutor and ran entry-point discovery. Measured at 9.98s cold.

Resolve celery_app lazily (PEP 562) and replace the executors package's
import-side-effect registration with an explicit, idempotent register_all().
The two entrypoints that need a populated registry -- executor/worker.py and
executor/tasks.py -- call it directly; both already carried an explicit
`import executor.executors  # noqa: F401` for exactly this purpose, so this
makes an existing intent explicit rather than changing it.

structure_tool_task.py is untouched; all three of its imports become cheap.
Deferred first-task cost goes 2.334s -> 0.004s locally, with
legacy_executor no longer imported into file_processing at all.

The new test pins the import graph rather than a duration -- a timing
assertion would be flaky on CI, while what actually regresses is an eager
import creeping back into either package init. Both cases were confirmed
failing on origin/main.

UN-4136

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: address round-1 self-review findings

Critical: restore the celery.app.trace suppression. executor/worker.py's
setLevel(WARNING) mutes the trace logger, which prints the full task result —
for execute_extraction, extracted customer document text — at INFO. That module
is on no deployed path: workers/worker.py exec-loads executor/tasks.py by path
for both executor roles and nothing runs `celery -A executor`. It ran only
because executor/__init__.py eagerly imported .worker, so making that lazy
silently re-enabled result logging on every extraction. Moved the suppression to
executor/tasks.py, which keeps it without re-importing the expensive module.

register_all(): latch before discovering rather than after. ep.load() executes
third-party code, and a plugin whose import graph reaches back into the function
would restart the entry point loop, nesting once per level (measured: 50 nested
loads where 1 was intended). The import-side-effect version got that safety free
from sys.modules. A discovery that raises now resets the latch instead of
pinning a permanently-empty list.

Correct the docstrings rather than the behaviour where the claim was the defect.
register_all() cannot repopulate a registry something else emptied — the second
import is a sys.modules hit and the decorator does not re-run. Making it
re-register explicitly broke four test files that do unguarded
ExecutorRegistry.register(LegacyExecutor) after a clear, so the guarantee is now
stated as per-process instead. Also: executor/__init__.py no longer contradicts
its sibling in the present tense; worker.py's register_all() call is gone (it
could never do work — import executor.tasks above it already registered, so the
comment claiming otherwise would have led a maintainer to delete the call that
matters); plugins/loader.py and test_plugin_loader.py's own enumeration no
longer describe the removed import side effect.

Tests: all three new cases were defective. One was vacuous — removing the latch
entirely left the suite green, because with no cloud plugins installed the first
call also returns []. Two passed only on suite ordering: an unrelated module
writes ExecutorRegistry._registry["legacy"] directly and never cleans up, and CI
shards with xdist's default per-test scheduler, so their result depended on shard
assignment. Rewritten to assert registration in a fresh interpreter, to assert
idempotency on a spy's call count rather than a return value that is [] either
way, and to snapshot/restore the process-global registry. Added coverage for
re-entrancy, for the non-latching failure path, and for the trace suppression.
All five guards are mutation-verified.

UN-4136

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: address the late round-1 findings

_reset_discovery_for_tests()'s docstring still claimed register_all()
re-registers the bundled executor when the registry has lost it. That was true
of a fix I reverted — making register_all() re-register broke four test files
that do unguarded ExecutorRegistry.register(LegacyExecutor) after a clear — and
the stale claim then contradicted register_all()'s own docstring. It sat on the
helper a test author reads before writing `clear(); reset(); register_all()`
and asserting on the registry, which returns cleanly and leaves it empty.

register_all() now returns a copy. It was handing out the latched module list,
so a caller appending to the result corrupted what every later caller saw.

Subprocess test snippets raise SystemExit instead of asserting. The child
inherits the parent environment, so under PYTHONOPTIMIZE every assert is
stripped: the snippet would print OK, exit 0, and the guard would pass having
checked nothing. Verified by mutating the trace suppression under
PYTHONOPTIMIZE=1 and confirming the test still fails.

Test coverage for three guards that had none: the LegacyExecutor __getattr__
shim (no caller in this repo or the cloud plugins uses that spelling, so a typo
in the name comparison would break every cloud plugin while OSS CI stayed
green), the breadth of the BaseException catch (the failure test drove only
RuntimeError, so narrowing the handler would have kept the suite green while
reopening the latched-empty window for the KeyboardInterrupt case it exists
for), and the copy semantics above.

Documented, not fixed: discover_executors() catches per-entry-point failures and
logs a warning, so register_all() latches [] and returns it whether no cloud
plugins are installed or every one of them failed to import. A broken plugin
wheel therefore boots clean and surfaces per-request as "No executor registered
with name 'table'". The Returns section now states both meanings rather than
only the benign one. Making the aggregate loud is a behaviour change to the
loader and out of scope for a performance PR.

UN-4136

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: address round-2 self-review findings

Every finding this round was introduced by round 1's fixes.

Drop the reset-on-failure rather than patch it. It was added so "a caller that
survives the failure retries instead of silently getting a permanently empty
list", but there is no such caller — the single call site is at
executor/tasks.py module scope, so a failure kills the boot and the process
restarts with clean module state. Worse, the retry it enabled is harmful: a
plugin whose @ExecutorRegistry.register fires before it raises is evicted from
sys.modules but left in the registry, so re-importing it raises a duplicate-name
ValueError that discover_executors swallows into a warning, leaving that
executor silently absent for the life of the process — the exact outcome the
reset claimed to prevent. Verified. Dropping it also closes the window where the
latch was armed one statement before the try, and removes the BaseException
breadth question along with the test that pinned it.

Stop the test fixture corrupting the registry it was added to protect.
clear() + update(saved) deletes a registration the test itself caused: when this
class is what first imports legacy_executor, the decorator fires during the test
so `legacy` is absent from the snapshot, and it cannot be restored because the
re-import is a sys.modules hit. Every later test in that process then saw an
empty registry; nothing failed only because five other modules re-register
defensively. Now an additive restore.

Restore test_register_all_tolerates_legacy_executor_already_imported, deleted in
round 1. Its scenario is live in production — a cloud plugin imports
LegacyExecutor at module scope and is loaded by ep.load() inside discovery — and
the subprocess replacement covers the opposite ordering, a fresh process with no
prior import. Deleting it weakened coverage for a real path.

Convert the remaining subprocess snippet to raise SystemExit. Round 1 wrote that
rule into one test file and violated it in the other in the same commit; under
PYTHONOPTIMIZE the asserts are stripped and the guard passed having checked
nothing. Confirmed by mutation under PYTHONOPTIMIZE=1.

Docstrings: the module docstring still described a register_all() call in
executor/worker.py that the same commit deleted, and tasks.py still called that
second call "harmless" — both now say executor/tasks.py is the only caller and
that worker.py's import of it is load-bearing. The Returns block promised the
names "on every call" when the re-entrant call it advertises returns the empty
placeholder, and counted two causes for an empty list when there are three. The
claim that tests clearing the registry re-register explicitly was false for four
of the five that clear it. The xdist claim is narrowed to the repo's test rig,
which is what is checkable — there is no .github/workflows here.

UN-4136

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: address round-3 findings, including a regression in the core change

The lazy import moved WHEN legacy_executor first loads, and two test files
depended on the old timing. On main, importing executor.executors registered
`legacy` at package-import time — before any fixture cleared the registry — so
an unguarded ExecutorRegistry.register(LegacyExecutor) in the test always
followed a cache hit. With the import lazy, that first import now happens inside
the test, after the clear, and the explicit registration raises a duplicate-name
ValueError. tests/test_legacy_executor_index.py and
tests/test_legacy_executor_extract.py fail alone on this branch and pass alone
on main; they survive the full alphabetical suite only because an earlier module
imports the class first, which is why this looked pre-existing. Guarded both,
matching the pattern eight sibling modules already use.

Restore the except BaseException reset removed in round 2. That removal rested
on a false premise: the handler never retried, it re-raised unchanged, and its
only effect was to stop a LATER call reporting "no cloud executors" as though
discovery had succeeded. It also preserves pre-PR behaviour — discovery ran as a
package import side effect, and a module that raises mid-import is evicted from
sys.modules, so the next import re-ran it. The duplicate-registration hazard
cited as the reason to drop it applies to re-running discovery at all, so it
predates this function and was never traded away by keeping the reset.

Restore the clearing teardown, also removed in round 2. It was made additive to
stop it deleting a registration the class itself caused, but nothing in the
suite depends on import-side-effect registration of `legacy` — every consumer
registers explicitly or assigns directly — so the snapshot restore was correct
isolation and the additive version leaked `legacy` into every later test in the
worker. Worse, the one test that ran unpatched discovery would, where the cloud
wheels are installed, have leaked all eight cloud executors into names the
sibling modules register themselves.

That test is rewritten: it arranges its own precondition instead of inheriting
one, patches discovery so it stops doing real entry-point loading, and asserts
only the property it is named for. As written it was order-dependent — it failed
deterministically after any module that clears the registry — and its registry
assertion was satisfied by its own import line rather than by register_all().

Docstrings: "the only caller" was false (the suite calls it in-process) and the
handler-removal argument was reasoned from it; restored the "production"
qualifier and the note that executor/worker.py's app is test-only. The stated
failure symptom for a broken plugin wheel held only when the plugin raises
before its decorator. Cited the rig path repo-root-relative.

All six guards mutation-verified.

UN-4136

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: address round-4 findings

Guard the two remaining duplicate-registration sites. test_legacy_executor_
scaffold.py is a proven regression from this branch, not pre-existing as an
earlier check concluded: run alongside the index and extract modules it passes
on main (52 passed) and fails here. My earlier check ran that file ALONE, where
it is broken on main too for its own reason, which hid the difference. Mutation-
verified: removing the guard reproduces the failure in that ordering.
test_plugin_migration_regression.py has the identical shape; its guard is
defensive consistency and is NOT pinned by a test — that file's own earlier
tests import legacy_executor before the helper runs, so no ordering within it
exercises the guard. Recorded rather than claimed as covered.

"This does not retry" was true of the handler and false of the mechanism:
un-arming the latch is exactly what lets the NEXT call re-run discovery, and the
comment then justified itself by saying the next import re-ran it. Both cannot
hold. It now says plainly that it re-arms a retry for a later caller, that the
re-run is not free when a plugin registered before it raised — that plugin ends
up live in the registry but absent from the returned list — and that no
production caller survives the re-raise to make that call.

The Returns block now says a non-empty list can under-report the registry after
a failed-then-re-run discovery. The _cloud_executors sentinel comment said None
means "discovery has not run"; since the handler came back it also means "ran
and raised". The empty-list placeholder moved inside the try, so an async signal
between the assignment and the try cannot leave it armed. The register_all
docstring no longer offers test_legacy_executor_scaffold.py as "the pattern to
copy" — that file carried the unguarded helper this commit fixes; it states the
guard rule directly instead.

TestExecutorsInit's docstring claimed the fixture leaves the process as it found
it. sys.modules is not restorable and this class can be the first importer of
legacy_executor, which is the state the sibling guards exist to survive.

test_failed_discovery_un_arms_the_latch is parametrised rather than looped: a
for loop stops at the first failing arm, and the KeyboardInterrupt arm is the
one the docstring argues is the point. Its docstring no longer says only
BaseException reaches the handler — an entry_points() failure is an ordinary
Exception and reaches it too, which is what the RuntimeError arm covers.

UN-4136

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: close the last duplicate-registration site; cut the over-claiming prose

tests/test_line_item_extraction.py was the last site with the hazardous
ordering — autouse fixture clears the registry, then the test imports
legacy_executor and registers it unguarded. When that import is the first in the
process the decorator fires after the clear and the explicit register raises.
Fails alone on this branch, passes alone on main. Routed through the module's
existing guarded helper. Mutation-verified. An enumeration of all 22
ExecutorRegistry.register sites under workers/tests/ says this was the last one
of its shape: 13 already guard, 2 import before they clear and are safe by
ordering.

Delete the historical narrative from the guard comment in all four files. It was
written for index/extract, where it is true, and copied into scaffold and
migration_regression, where it is false: scaffold fails alone at the merge base,
so it was never "unguarded safely", and in both of those the first import of
legacy_executor was already inside a test before UN-4136, so UN-4136 moved
nothing. Only the rule survives, which is true everywhere.

The prose in register_all() has now been wrong in four consecutive rounds, each
time by being true of the case it names and false of one it never mentions, so
this trims rather than rewrites:
- the guard rule stated one implementation as mandatory and gave a reason that
  inverted the actual cause (the hazard is the import has NOT happened yet);
  it now states the invariant, and notes clearing immediately before is equally
  valid, which is what test_summarize_operation.py does;
- the under-report note was scoped to a non-empty list, but with a single
  installed plugin the re-run returns [] — folded into the empty-list
  enumeration as "at any length, including zero";
- the handler comment claimed every already-registered plugin is evicted from
  sys.modules. Only the module that was mid-import when the exception escaped
  is. Narrowed, and the surrounding argument cut to what is checkable.

Also reverts formatter churn this branch introduced in two test files: a
`ruff format` run reflowed an aligned comment block unrelated to the guard.

UN-4136

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: correct the inverted sys.modules claim; record the duplication follow-up

TestExecutorsInit's docstring said the sibling guards "assume" the sys.modules
entry for legacy_executor is left behind. They assume the opposite: the guard is
INERT when the entry is present (the import is a cache hit, the decorator does
not fire, the cleared registry has no `legacy`, the register runs) and
load-bearing when it is absent. The previous commit fixed this same inversion in
register_all()'s docstring and missed this sentence.

Record in code that the guard idiom is hand-copied across 23 sites in 18 files,
in three spellings, and that folding it into a shared test helper is the
follow-up rather than something this PR does. Counted: 23 guard sites, 7 direct
_registry["legacy"] assignments, 1 clear-then-register. 18 of the 23 predate
this branch. Detail on UN-4136.

Correcting the record on the previous commit: it stated the
test_plugin_migration_regression guard is "NOT pinned by a test". That is wrong.
No whole-FILE run pins it, but three class subsets fail without it, and the rig
runs xdist with the default per-test `load` scheduler, so the unit of ordering
in CI is the node. Measured with the guard removed:

  whole-file:  31 passed        <- what the mutation gate was measuring
  node-level:  3 classes fail   <- the guard is load-bearing

That wrong granularity is also why the test_line_item_extraction site survived
four rounds of module-ordering A/B: a defect that only appears when a node runs
without its file-mates is invisible at module granularity. Mutation checks on
the remaining guards were re-run at node level.

UN-4136

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: cut two over-specified sentences added last round

Both are the same failure this PR keeps repeating — saying more than is true —
so both are trimmed rather than restated.

The register_all() docstring claimed the guard rule is hand-copied across "~23
sites in three spellings". The 23 is the count of the guard IDIOM, but only 16
of those are followed by ExecutorRegistry.register; the other 7 assign
_registry["legacy"] directly, which cannot raise, so the rule does not reach
them. The count also excluded the third spelling's only site, so it matched two
of the three it claimed. Dropped: a tally of test-directory sites in a
production docstring rots on the next test added, and the duplication is the
fact worth recording, not the number.

It also pointed the follow-up at UN-4136, which is the ticket for THIS change —
cited as the problem being fixed at four other sites on the branch. A reader
following it lands on the ticket this PR closes and concludes the follow-up
shipped. The follow-up is recorded on that ticket as a comment and in the PR;
the docstring now just says it is deliberately left out.

TestExecutorsInit's docstring said the sibling guards are "inert when the entry
is present and load-bearing when it is not". True of the four modules whose
helper calls register(), false of the 7 direct-assign sites, where the guard is
inert in both states. The surviving sentence — the guards are correct either
way, so leaving the sys.modules entry is harmless — is true unconditionally.

Verified while checking this: all three touched test files now differ from
origin/main by their guard alone, with no formatter churn remaining.

UN-4136

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: stop claiming ordinary plugin exceptions reach the BaseException handler

Round 5's trim replaced a correct enumeration with a broader one that is false.
ep.load() sits inside discover_executors' per-entry-point `except Exception`
(loader.py:48-56), so an ordinary exception from a plugin never escapes — it is
logged and skipped there. Only a non-Exception BaseException, or a failure of
entry_points() itself, reaches the handler.

The risk is not the wording. A maintainer who believes anything out of ep.load()
reaches this handler has no reason to keep loader.py's per-entry-point catch,
and removing it turns "one broken plugin wheel is logged and skipped" into "one
broken plugin wheel aborts discovery" — on the executor worker's boot path. The
comment now says what that handler is for rather than enumerating what escapes.

Verified both ways: read against loader.py, and by driving a fake entry point
that raises RuntimeError — register_all() returns [] rather than raising.

UN-4136

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: stop explaining the BaseException choice in prose; point at the test

Third writing of this sentence, third distinct falsehood. The latest claimed an
ordinary exception from a plugin "never reaches here". It can: entry_points()
sits OUTSIDE discover_executors' per-entry-point try (loader.py:46 vs :48), so a
wheel with malformed entry-point metadata raises an ordinary Exception that
escapes straight to this handler. Reproduced by building a dist-info whose
entry_points.txt has a garbage line — TypeError, isinstance(Exception) True,
escapes discover_executors.

So the previous wording was wrong in the other direction from the one before it,
and the one before that was wrong differently again. Each rewrite was a fresh
surface for the same defect.

Replaced with a pointer to test_failed_discovery_un_arms_the_latch, which is
parametrised over both classes that can reach the handler — an ordinary
Exception out of entry_points(), and a non-Exception BaseException out of
ep.load() — and pins them by executing rather than by asserting in prose. Both
arms pass and report independently.

The correctly-scoped statement of what the per-entry-point handler absorbs
already lives in the module docstring, which says "failed to import" rather than
the broader "broken wheel" that made the last version false.

UN-4136

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: give the test pointer a path, and SystemExit its own arm

Two below-floor items from the final review round.

The pointer to the test that pins the BaseException choice gave a bare function
name, while the file's two other test references both carry paths. A pointer's
whole value is that it resolves, and a grep that comes back empty after a rename
is indistinguishable from a typo. It now reads
tests/test_plugin_loader.py::test_failed_discovery_un_arms_the_latch.

The test docstring named KeyboardInterrupt/SystemExit as the classes escaping
ep.load() while only KeyboardInterrupt had an arm. Rather than argue which
reading was intended, SystemExit now has one — it is free, since the handler
treats them identically. Verified it discriminates: with except BaseException
narrowed to except Exception, the RuntimeError arm passes (as it must) and both
the KeyboardInterrupt and SystemExit arms fail.

The docstring also stops saying "both classes", which was only true while there
were two arms, and now describes the partition instead: an ordinary Exception
escaping entry_points(), and the BaseException-not-Exception classes escaping
ep.load().

UN-4136

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Rahul Johny <116638720+johnyrahul@users.noreply.github.com>
Co-authored-by: ali <117142933+muhammad-ali-e@users.noreply.github.com>

…requires auth (#2300)

quay.io/minio/minio and quay.io/minio/mc started returning 'unauthorized',
breaking the integration (testcontainers) and e2e (compose) CI tiers.
Mirror cgr.dev/chainguard/minio and minio-client:latest-dev to Docker Hub
under unstract/* and pin by digest. Run minio as root so existing
root-owned minio_data volumes stay writable under Chainguard's uid 65532.

…r logo and match the 2026 top-bar design (#2296)

* UN-4172 Render the cloud product switcher in place of the top-bar logo and match the 2026 top-bar design

- TopNavBar: when the platform-dropdown plugin is present it carries the
  product mark, so it replaces the logo instead of trailing it. OSS-only
  builds keep the logo.
- Top bar is 56px with 12px gutters (Figma 1638-44720); min-height keeps an
  OSS-only bar the same size.
- Avatar menu restyled to the design: 32px avatar, chevron, 40px target,
  6px spacing between right-side items.
- Switch Org: give the row its own flex layout (the old rule keyed on an
  antd class the Dropdown shim no longer emits, stacking the icon over the
  label), and stop its click from selecting the parent menu row, which
  closed the menu and the org list with it.

* UN-4172 Add a regression test for Switch Org's nested dropdown

Drives the profile menu with user-event's full pointer sequence, asserts the
org list stays open after a real click on Switch Org, then picks an org and
checks the switch request is sent. Both tests fail with the click guard
removed.

* UN-2646 [FEAT] Add LLMWhisperer image output mode to the v2 adapter

Adds an "image" output mode to the LLMWhisperer V2 X2Text adapter: it converts a
PDF to per-page images via the LLMWhisperer pdf-to-images API and returns them as
PageImageReference objects (persisted to FileStorage), never smuggled into
extracted_text, so text-mode consumers are unaffected.

- dto.py: PageImageReference + additive TextExtractionMetadata.page_images.
- constants.py: OutputModes.IMAGE, ImageOutputConfig (PDF-only), OUTPUT_MODE key.
- helper.py: get_page_images() — submit / poll / retrieve+unzip the pdf-to-images
  job and persist the page images.
- llm_whisperer_v2.py: _process_image_mode() + output-mode branch in process(),
  with PDF-only validation.
- json_schema.json: "image" enum + "Image (PDF only)" label + description.

Recovered from the earlier implementation (MFBT phase
llmwhisperer-image-output-mode-adapter) and rebased onto main, kept independent of
the document_insights/signature feature (PR #1967). 45 tests pass. MUNS-193/194/195
complete; MUNS-196 UI validation (UNS-757/758/759) remains.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* UN-2646 [FEAT] Conditional PDF-only guidance on image mode (UNS-759)

Convert the adapter json_schema's single top-level if/then into an allOf so a
second conditional can coexist with the existing low_cost one. Adds a PDF-only
guidance note (type: null field, RJSF renders it as a labelled callout) shown
only when output_mode == "image", guarded by required:[output_mode]. Purely
UI/UX; no effect on submission, validation, or backend behaviour.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* UN-2646 [FIX] Gate image PDF-only guidance to image mode (UNS-759)

The output_mode description carried the image PDF-only note unconditionally,
so it rendered for every mode (Layout Preserving / Text too). Move it into an
if/then/else on output_mode: the note shows only when Image is selected;
other modes show the neutral documentation description. The previous
type: null notice field never rendered (RJSF NullField emits nothing).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* UN-2646 [FEAT] Fail-fast PDF-only check before index dispatch (UNS-757)

When the profile's x2text adapter is in image output mode, reject non-PDF
inputs in build_index_payload (the live pre-dispatch path) so the user gets
the SDK's PDF_ONLY_ERROR at index time instead of an extraction failure inside
the executor. Message is sourced from ImageOutputConfig.PDF_ONLY_ERROR so the
early check and the SDK runtime guard stay in sync. Mirrored in the legacy
index_document path for symmetry. Adds no-DB unit tests (10 cases).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* UN-2646 [FIX] Resolve SonarCloud findings on image-output code (UNS-758)

- helper.py: drop redundant 0* from the page-image regex — int() already
  strips leading zeros, so 0*(\\d+) was ambiguous and flagged for
  super-linear backtracking. Now a linear page[_-]?(\\d+)\\.png$.
- Tests: hoist non-asserting setup out of pytest.raises blocks so each
  block has exactly one call that can throw (3 sites).
- Test: split the deterministic build_page_store_dir assertion into two
  named locals so the reliability check no longer sees identical
  expressions on both sides of == (was flagged as a bug).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* UN-2646 [FIX] Address CodeRabbit/Greptile review on image-output helper

- _send_raw_request: give timeout a finite default (IMAGE_REQUEST_TIMEOUT)
  so test_connection can no longer hang forever on a stuck endpoint.
- download_pdf_to_images_zip: consume the stream in try/finally — map
  read-time transport errors to ExtractorError and always close the
  response so a mid-stream failure cannot leak the connection.
- extract_page_images_from_zip: fail closed on an archive with no page
  images (was a silent empty success), and also catch RuntimeError
  (encrypted member) and zlib.error (corrupt member) alongside BadZipFile.
- Tests: scope the constants env patches via monkeypatch.context() so the
  module reload no longer leaks a patched value into later tests; add
  coverage for the no-page-archive hard error and the finite default
  timeout.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* UN-2646 [FIX] Make image-mode results survive indexing + cache (UNS-758)

Resolves the two pipeline gaps that capped Greptile confidence at 3/5:

1. page_images dropped in transit: the executor built the indexer payload
   from extracted_text only, so image-mode references never reached the
   indexer and the document indexed empty. The executor now forwards
   page_images (new IKeys.PAGE_IMAGES), mirroring the highlight_metadata
   pattern, so the references survive the transport.

2. Repeated remote conversions on re-run: image mode never wrote the extract
   file the Prompt Studio extraction cache gate requires (non-empty), so every
   re-run re-submitted the pdf-to-images job. Image mode now returns a short
   human-readable summary as extracted_text and writes it to the extract file
   (plus a durable <output>.page_images.json manifest of the references), so
   the cache treats the conversion as complete and the indexed document is
   meaningful instead of blank. References are never inlined into the text.

Tests: update the image-mode process assertions for the summary contract and
add coverage for the extract-file/manifest write and the summary format
(49 sdk1 image-output tests pass).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* UN-2646 [FIX] Keep image-mode cache fix, drop unused ref plumbing (UNS-758)

Scope the previous change to just the real bug fix. The executor page_images
forward and the JSON manifest sidecar were producer-side plumbing with no
consumer anywhere in the codebase, so they added unused surface without making
image references usable end-to-end. Removed both.

Kept: image mode returns a short summary as extracted_text and writes it to the
extract file, so the Prompt Studio extraction cache treats the conversion as
complete and a re-run is a cache hit instead of a re-submit of the remote
pdf-to-images job (the billing bug). Per-page references remain on
extraction_metadata.page_images and the images are persisted to FileStorage; a
Prompt Studio consumer of those references is deferred as follow-up.

49 sdk1 image-output tests pass; ruff/pycln clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* UN-2646 [FIX] Address Chandru's review on image-output mode

Correctness / robustness:
- Return the real whisper_hash in extraction_metadata (was ''), and reject
  image mode + highlight explicitly instead of silently dropping highlights.
- Store page images under {extract_dir}/{stem}/pages (per-document), not the
  per-run hash — a re-extraction now overwrites its own pages instead of
  orphaning a fresh tree in FileStorage every run.
- persist_page_images cleans up already-written pages on a mid-list failure.
- Poll loop fails closed: only explicit intermediate states continue; a
  failure/unknown/empty status raises immediately instead of hanging to the
  budget. _safe_json logs non-JSON/non-object bodies.
- verify_page_count is now live: expected count derived locally from the input
  PDF (pdfplumber); extract asserts a contiguous 1..N page set (catches
  truncation + duplicate/misnamed members) and logs skipped entries.
- Retrieve advertises accept: application/zip; submit sends Content-Type.

Layering / guard (UNS-757):
- Promote OUTPUT_MODE/image/PDF_EXTENSION/PDF_ONLY_ERROR + an is_pdf() helper
  to the shared x2text.constants (ImageOutputConstants); backend imports the
  generic surface instead of the adapter's private src.
- Move the PDF-only guard into dynamic_extractor (the single extract choke
  point, under profile_manager) so all entry points and prompt-level profile
  overrides are covered; gate on the LLMWhisperer adapter id as well as
  output_mode so other x2text adapters can't inherit the rejection.

Comments: drop ticket-id/PR provenance and the contradictory PR#536/#647
contract notes; remove the unused IMAGE_MODE_LABEL 'single source of truth'.

Tests: fix the constants-reload cross-file pollution (retry tests now pin the
real budget on helper.WhispererDefaults); add network-layer coverage
(poll success/failure/non-JSON-fast-fail/budget, mid-stream download error +
close, submit-without-hash), mid-list cleanup, image+highlight rejection, a
non-LLMWhisperer no-op, and a dynamic_extractor call-site test. sdk1: 57 pass;
backend guard: 12 pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* UN-2646 [FIX] Drop remaining PR/ticket provenance from image-output comments

Follow-up to the review round: remove the contradictory 'ASSUMED contract
(PR #647)' vs 'verified against PR #536' notes and the 'decision 2A' / UNS-743
provenance from the adapter helper + constants comments, keeping the factual
one-line reason (comments carry the why, not the ticket).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* UN-2646 [FEAT] Gate image output mode behind the cloud consumer plugin

The image output mode produces per-page billed PNGs that are consumed by
the VLM answer plugin, which ships only with Unstract Cloud. Without the
plugins.vlm_image_answer package:

- the 'image' option (and its conditional description) is stripped from
  the LLMWhisperer V2 adapter's JSON schema, so the UI cannot select it
- adapter create/update and test-connection reject image-mode metadata
  with a clear error, covering API-created adapters that bypass the UI

This prevents OSS deployments from configuring an extraction whose
output nothing can consume.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* UN-2646 [FEAT] OSS half of the VLM image-answer feature (path contract, loader, vision policy, bridge) (#2218)

* UN-2646 [FEAT] Promote page-image path contract to the shared x2text surface

Foundation for the VLM consumer (MUNS-206): the writer (adapter) and the
upcoming page-image reader must agree on the storage layout without
metadata persistence or a manifest sidecar.

- Promote build_page_store_dir ({extract_dir}/{stem}/pages) from the
  LLMWhisperer helper to unstract.sdk1.adapters.x2text.constants; the
  helper now binds the shared function directly (identity, not a copy)
- Add page-naming constants to ImageOutputConstants: PAGES_SUBFOLDER,
  PAGE_IMAGE_PREFIX/EXTENSION/PADDING, PAGE_GLOB_PATTERN and
  PAGE_NUMBER_REGEX (integer capture for natural sort — lexicographic
  ordering misorders past page 999)
- ImageOutputConfig now aliases the shared layout constants; the
  tolerant ZIP-member pattern moves to a named ZIP_PAGE_MEMBER_REGEX
  (service wire contract, distinct from the storage contract)
- New writer/reader agreement tests: path determinism across stem
  shapes, glob/regex contract incl. >999-page fixtures, and the
  writer's filename builder round-tripping through the reader's
  glob + regex

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* UN-2646 [FEAT] FileStorage page-image loader and vision-capability policy

Reader-side sdk1 utilities for the VLM consumer (MUNS-207 + the sdk1
half of MUNS-208):

- page_image_loader: discovers persisted page_NNN.png files through the
  FileStorage abstraction using the shared naming constants, orders by
  integer page index, enforces a fail-fast page cap before any bytes are
  read, base64-encodes, and shapes complete_vision content blocks with
  'Page N' labels before each image. Typed failures keep empty
  (never-extracted/purged), incomplete (post-write loss, with found vs
  missing pages), and over-cap cases distinct — remediation copy steers
  to cache-bypass re-extraction with the per-page re-billing warning.
- vision_capability: classifies a model as SUPPORTED / UNSUPPORTED /
  UNKNOWN from litellm's local model_cost registry only — never
  get_model_info, which can make network calls for self-hosted
  providers, and never bare supports_vision, which returns False for
  both non-vision and unknown models and would wrongly hard-block
  custom vision models (Ollama, proxies). Policy: hard-block only a
  definitive UNSUPPORTED; UNKNOWN warns and allows.

Tests: 33 new (in-memory S3-like double + real local FileStorage
backend, >999-page ordering, cap-before-read assertion, registry fakes
+ real-registry smoke checks). Full sdk1 image suite: 104 pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* UN-2646 [FEAT] OSS bridge for the cloud-only VLM image-answer plugin

Image-mode prompts are answered by a vision LLM via the cloud-only
'vlm-image-answer' executor plugin. This adds the OSS half (mirrors the
lookup_enrichment bridge, with the opposite error policy — lookups
degrade, image mode fails loudly):

- workers vlm_image_answer bridge: detects image mode by resolving the
  x2text adapter config through the platform service (the payload
  carries only the instance id), cached per (execution, adapter);
  dispatches to the plugin with the deterministic pages dir from the
  shared path helper; raises structured, code-prefixed errors
  (IMAGE_OUTPUT_REQUIRES_CLOUD / IMAGE_OUTPUT_MISSING /
  IMAGE_PAGE_CAP_EXCEEDED / VISION_LLM_REQUIRED) that survive the
  string-only error propagation to PS and deployment responses —
  never a silent fallthrough to the text path
- legacy_executor: image-mode branch replaces retrieval + completion
  only; type conversion, lookups, webhooks and challenge run unchanged.
  Single-pass extraction is rejected for image-mode profiles before
  delegating to the cloud single-pass plugin
- backend vlm_utils no-op bridge (lookup_utils pattern) wired into:
  profile serialization (non-blocking vision_warning), API deployment
  creation (deploy-time guard), and the extraction choke point
  (VLM answer invalidation after a fresh extraction rewrites pages/)

Tests: 13 bridge (detection/cache/dispatch/error mapping/single-pass
guard) + 8 vlm_utils (OSS no-ops + cloud delegation policies). Full
sdk1 image suite (104) and gating suite (12) stay green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* UN-2646 [FEAT] Surface backend vision_warning after profile save

Profile save responses may carry a backend-computed vision_warning
(image output mode selected with an LLM that is not verifiably
vision-capable — populated by the cloud vlm_image_answer hooks, never
present in OSS responses). Show it as a non-blocking warning toast so
the user learns about the mismatch at config time instead of at run
time.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* UN-2646 [FIX] Surface typed errors for stale listings and vanished pages

Live-testing regression: with pages purged from object storage, the
loader raised a raw FileNotFoundError instead of the typed
incomplete-set error — fsspec's directory cache served discovery a
stale listing in the long-lived worker, so the contiguity check passed
and the purge only surfaced at read time, bypassing the error mapping.

- discover_page_images now refreshes the backend's listing cache first
  (duck-typed invalidate_cache; no-op for backends without one)
- load_page_images maps a read-time FileNotFoundError (TOCTOU: page
  vanished after discovery) onto PageImageSetIncompleteError with the
  cache-bypass + re-billing remediation
- in-memory test double now raises FileNotFoundError like real backends

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* UN-2646 [FIX] Sniff PDF content when the storage name has no extension

Live deployment-path regression: workflow executions store the source
file under an extension-less name (e.g. SOURCE), so the extension-only
PDF guard false-rejected every API-deployment input — a real .pdf
upload failed with the PDF-only error.

_validate_pdf_only now checks the extension first and falls back to
content sniffing (%PDF- magic bytes via FileStorage) when the name has
no .pdf suffix; unverifiable content still rejects (fail-closed). No
extra read on the common .pdf-named path.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* UN-2646 [FIX] Raise the gating rejection with a dict detail, not a bare string

Live-testing regression: validate_image_output_allowed raises from
inside AdapterInstanceSerializer.to_internal_value, where DRF folds the
detail into its per-field error mapping — a bare-string detail crashed
error collection (ValueError: dictionary update sequence) and surfaced
as a 500 'Something went wrong' instead of the clean 400 with the
'available only on Unstract Cloud' message. Use a field-keyed dict
detail, and log the rejected adapter/mode for diagnosability.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* UN-2646 [FIX] Address Greptile review: run-scoped mode cache and byte budget

Two review findings on the consumer path:

- Run-scoped detection cache (P1): IDE payloads carry no execution_id,
  so every IDE run shared one ('', adapter) cache entry — an adapter
  switched between text and image output kept serving the stale mode
  until worker restart. The cache key now scopes to execution_id or,
  for IDE runs, the run_id (deduplicating the N per-prompt resolutions
  within one run — the cache's actual purpose — with no cross-run
  reuse), and skips caching entirely when no scope id exists.
- Aggregate byte budget on image loading (P2): the page cap bounds the
  COUNT of images, not their size. load_page_images now enforces a
  50MB (default, disable-able) raw-byte budget while reading, raising
  the typed PageImageSetTooLargeError with page-range remediation; the
  bridge maps it to a distinct IMAGE_PAGES_TOO_LARGE error code.

Tests: run-scoped vs no-scope cache behavior, byte-budget stop-at-page
accounting, and the new error-code mapping.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* UN-2646 [FIX] Enforce the byte budget from storage metadata before any read

Greptile follow-up: the read-time budget still pulled each full object
into worker memory before checking cumulative size, so one pathological
page could spike RAM before rejection. The budget is now enforced twice:
first from storage size metadata (object HEAD — zero bytes transferred)
before any read, so an oversized set is rejected with no image bytes in
memory; the read-time accounting stays as a belt-and-braces guard for
backends without size metadata and for stat/read races.

Tests: metadata pre-check rejects with zero reads; no-size() backends
still enforced at read time; unreadable metadata falls back cleanly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* UN-2646 [FIX] Address review: extract-path keying, stamped detection, image-mode scoping

Addresses Chandrasekharan's review on the consumer path:

1. Pages directory keys on a never-rewritten extract path (blocker):
   summarize-as-source and smart-table runs rewrite the payload
   file_path before the answer step, so the reader looked in the wrong
   directory and surfaced IMAGE_OUTPUT_MISSING with a paid remediation
   that could not help. The payload builders (IDE single/bulk and the
   structure tool task) now stamp an explicit extract_file_path,
   captured before any rewrite; the bridge derives pages/ from it, with
   the old file_path as fallback for older payloads.

2. Text-mode prompts no longer depend on the platform service: the
   backend stamps the x2text adapter's output_mode per prompt (it
   already holds the decrypted metadata), so detection is payload-only
   for stamped runs; the run-scoped platform resolution remains as the
   fallback for unstamped payloads (API deployments, older payloads).

3. Challenge and evaluation are skipped in image mode with a visible
   log line — both verify an answer against retrieval context, which
   image mode does not have; running them billed a doomed second LLM
   call. A vision-aware challenge is a later-phase decision.

4. Retrieval adapters are no longer constructed for image-mode
   prompts: detection now runs before adapter init, which proceeds
   LLM-only (embedding/vector DB skipped) when image mode is detected.

5. Profile-save toasts no longer swallow each other: one alert renders
   — 'Saved — check LLM compatibility' with the warning when present,
   plain success otherwise (the alert store holds a single entry, so
   two synchronous calls batched into showing only the last).

6. vlm_utils logs a distinct warning when plugins.vlm_image_answer is
   present but backend_hooks fails to import — 'cloud hooks broken' is
   no longer indistinguishable from 'running OSS'.

Bridge API split accordingly: detect_image_mode_config (stamp fast
path + platform fallback) and run_vlm_image_answer (dispatch only,
keyed on extract_file_path). Tests: 23 bridge (stamped/unstamped
detection, cache scoping, extract-path keying) all green; sdk1 130;
backend 20.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* UN-2646 [FIX] Bound page-image reads to the remaining byte budget

Greptile's follow-up was right: the metadata pre-pass fell back to an
unbounded full-object read when the size lookup failed, so one
pathological object could still spike worker memory before the budget
check. Replaced both layers with a single stronger mechanism — bounded
reads: every page is read with length = remaining budget + 1 via
FileStorage.read, giving a hard allocation ceiling of budget + 1 bytes
for the whole loop, independent of object sizes or the backend's size
metadata. The metadata pre-pass is gone (also Chandrasekharan's call —
one mechanism, less code).

Also folds in the review nits: dead PAGE_GLOB_PATTERN removed (readers
list + regex, never glob) and the duplicated not-found message
extracted to one helper.

Tests: bounded-read guarantees pinned (single 10MB object → exactly 51
bytes read; aggregate reads <= budget + 1) — 127 sdk1 + 23 bridge pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* UN-2646 [FIX] Gate the platform fallback by execution source; fix CI test fallout

The merged consumer branch put the pre-existing workers suites in front
of the image-mode bridge for the first time (they never ran on the
stacked PR), and ~105 tests failed on one mechanism: unstamped payloads
sent the detection fallback to the platform service, whose URL came
from a MagicMock shim.

The semantic fix, not a test hack: IDE payloads are always stamped by
the current backend, so an unstamped IDE payload is a pre-upgrade
in-flight run — detection now treats it as text mode instead of making
every legacy IDE prompt depend on a platform call. The platform
resolution remains only for non-IDE (deployment) payloads, which are
built worker-side without adapter metadata — that real dependency is
stubbed once in the workers test conftest (the same place the suite
already mocks celery and the shim), with image-mode tests opting in via
the payload stamp. The single-pass guard gets the same semantics
(stamp first, resolve only for non-IDE).

Also fixes the sdk1 collection error: the loader test imported the
shared fixture bare (PYTHONPATH-dependent) instead of via the tests
package like its siblings.

Bridge tests: 26 pass, incl. unstamped-IDE-never-calls-platform and
stamped-guard-never-calls-platform pins.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* UN-2646 [FIX] Reset the stable pages dir before persisting a new page set

A re-extraction that yields fewer pages than a previous run left in the
stable pages directory would otherwise keep the old trailing images,
which the reader serves back as part of the new document (Greptile P1).
The writer now clears the directory wholesale before writing, and a
failed reset raises instead of risking stale pages reaching the vision
LLM — consistent with this feature's fail-loud policy.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* UN-2646 [FIX] Stamp x2text output mode onto single-pass payloads

build_single_pass_payload was the one payload builder that skipped
_stamp_x2text_output_mode, so IDE single-pass payloads arrived
unstamped and the executor's single-pass guard — which treats an
unstamped IDE payload as a pre-upgrade text-mode run — never fired.
Image mode + single-pass would have silently answered every prompt
against the one-line extraction summary once the cloud consumer lands.

Stamps tool_settings alongside X2TEXT_ADAPTER, matching the other
three builders. Pinned by tests that drive build_single_pass_payload
with all collaborators patched and assert the stamp lands in the built
executor payload (image, non-image, and non-LLMWhisperer cases), so
deleting the stamp call fails the suite.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* UN-2646 [FIX] Fail closed when the VLM plugin is installed but broken

A half-broken cloud install (plugins.vlm_image_answer package present,
backend_hooks failing to import) previously logged a warning and
degraded to no-ops while the adapter gating — probing only the package —
kept image output mode enabled. Re-extracting an image-mode document
would then rewrite its page images while the answers stored against the
old pages were never invalidated (Greptile P1).

Both surfaces now share one fail-closed verdict:
- The gating probe requires backend_hooks to import, so a broken
  install strips image mode from the schema and rejects new saves.
- vlm_utils exports VLM_HOOKS_BROKEN, and dynamic_extractor's image-mode
  guard (the single extraction choke point) rejects image-mode
  extraction outright in that state — pages can never be rewritten
  while stale answers survive. Text-mode profiles are unaffected.

Tested across all three surfaces: probe verdicts for absent, broken and
healthy installs (forced via sys.modules so the local cloud overlay
cannot skew them), the choke-point rejection incl. text-mode passthrough,
and the OSS not-broken baseline.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* UN-2646 [FIX] Order invalidation before the success marker; propagate hook errors

Greptile flagged that a runtime failure in the re-extraction
invalidation hook was swallowed after the success marker had already
committed — so a retry would cache-hit past the failed invalidation and
stored answers could stay stale against rewritten page images, silently
and permanently.

Today this is unreachable (the cloud hook is purely informational:
Prompt Studio has no read-side answer cache; output rows are
overwritten per run, the same semantics text-mode re-extraction has
always had). But the contract now makes it structurally impossible for
any future hook that performs real invalidation:

- The bridge no longer swallows hook exceptions — the hook owns its
  error policy, and a raised error fails the re-extraction loudly.
- dynamic_extractor invokes the hook BEFORE committing the
  extraction-success marker, so a failed invalidation leaves the marker
  unset and a retry re-runs extraction + invalidation instead of
  cache-hitting past a stale-answer state.

Zero behavior change for the current hook; the bridge tests now pin
delegation and propagation separately.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* [MISC] Retrigger CI: review-thread confirmations added for the two flagged P1 threads

No code change. The two threads Greptile flagged as resolved-without-
explanation now carry developer confirmations (fixing commit b49587a
on the invalidation thread; intentional-deferral rationale on the
concurrent-reads thread). This empty commit re-fires the required
Greptile Review check so the score reflects that state.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* UN-2646 [FIX] Verify the pages dir is empty after reset, not just that rm returned

Greptile caught a real gap in the directory reset: FileStorage.rm's
S3-compatibility fallback (MissingContentMD5 → per-object deletes) only
WARNS on individual deletion failures, so rm can return successfully
while old page files survive. A survivor numbered within the new page
range would silently join the replacement set — exactly the stale-page
defect the reset exists to prevent.

The writer now verifies the prefix is actually gone after rm (through a
fresh listing, invalidating fsspec's dircache first) and raises
ExtractorError on survivors instead of writing a contaminated set.
Regression test drives an rm that silently leaves a file behind; the
in-memory fixture's rm now also drops directory entries, matching real
backends.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* UN-2646 [DOCS] Document the accepted concurrency contract at the writer and reader

The overlap between a same-document re-extraction and an in-flight
prompt execution is a designed, accepted limitation: the reader fails
with a typed, retryable error (IMAGE_OUTPUT_MISSING) rather than ever
mixing old and new pages into one answer. This was agreed in review
(atomic directory replacement does not exist on object storage; a
generation-versioned scheme was considered and rejected as out of
scope) but the code never said so — record the contract in the writer
and reader docstrings so it reads as intent, not oversight.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* [MISC] Retrigger review: flagged threads reopened with developer responses in place

No code change. The two threads previously flagged as
resolved-without-response are now unresolved and carry the developer
confirmations (fixing commit b49587a on the invalidation thread;
intentional-deferral rationale on the concurrent-reads thread) so the
review run reads them in their answered, open state.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* UN-2646 [TEST] Pin the no-op-rm variant of the post-reset survivor guard

The guard itself (verify the pages prefix is gone after rm, raise
ExtractorError on survivors) landed in c457331 together with a
partial-delete regression test. This adds the degenerate variant the
review asked for verbatim — rm as a complete no-op — asserting the
write is rejected and the old set is left untouched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* UN-2646 [FIX] Mark extraction failed when VLM answer invalidation raises

A success marker for the config hash can already exist when the cache-hit
path falls through to re-extraction (text file missing). If the invalidation
hook then raised, that marker survived and the retry cache-hit against the
freshly written text, skipping invalidation. Mark the hash failed before
re-raising so the retry re-runs extraction and invalidation.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* UN-2646 [FIX] Remove extracted text when the failure marker cannot be written

If VLM answer invalidation fails and marking the hash failed also fails,
the stale success marker survives. Delete the freshly written extract file
so a cache hit on that marker falls through the file-missing path and
re-extracts instead of skipping invalidation.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

…ontend test group required (#2302)

* UN-4183 Run frontend typecheck and vitest in CI; make the frontend test group required

- bun.lock: record TypeScript 7's native compiler packages, without which
  tsc cannot start on any platform, so typecheck never actually ran
- Fix the one type error that surfaced in antd-inputs.tsx
- ci-frontend-lint.yaml: add typecheck and vitest jobs (gate frontend-only PRs)
- ci-test.yaml: install frontend deps on the unit leg
- groups.yaml: drop optional from the frontend vitest group

* UN-4183 Address review: job-level path filter, --ignore-scripts, bun run test

- Filter frontend/** in a changes job instead of the pull_request trigger, so
  typecheck/vitest report skipped (not missing) on other PRs and can be made
  required checks
- bun install --ignore-scripts (Sonar S6505)
- bun run test --run instead of bunx vitest (Sonar S8543)

* UN-4183 Fail open when the frontend path filter job fails

A skipped job satisfies a required check, so a failed changes job must make
the frontend checks run, not skip.

…hild has loaded (#2301)

UN-4136 Serve /ready from the PG consumer so a startupProbe can wait for every child to load

A prefork PG consumer pod went Ready ~20s after start while its N children
were still running their own `import worker` bootstrap in parallel (3-8
cores for 1-2 minutes). Kubernetes' HPA only discards CPU sampled before a
pod turns Ready, so it read that burst as load: on integration it drove
api-file-processing 2 -> 30 pods in ~75s, and CAST AI's HPA calibration
raised the CPU request to 1.1-1.4 cores from it.

/health cannot express "still loading": the heartbeats are seeded fresh so
liveness does not trip during a slow import. Add a separate /ready:

- LivenessServer takes an optional ready_fn and serves /ready (/readyz):
  200 when it returns True, 503 "starting" otherwise or if it raises. With
  no ready_fn the path stays a 404, so a probe aimed at the reaper fails.
- The supervisor keeps a shared per-child "loaded" flag. A child sets its
  slot after `import worker` and building its consumer; reap() clears it.
  /ready is 200 only when every slot is set, and the JSON reports
  loaded_children. Each child also logs how long it took to load.
- The single-process consumer answers 200: it binds the port only after
  loading.

Liveness is unchanged. The chart's startupProbe on /ready is a separate
change in unstract-cloud.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

…d remove dead eslint-disable comments (#2303)

* UN-4184 Enable Biome recommended, a11y and hooks rules and replace dead eslint-disable comments

Turn on Biome's recommended set, the React domain, the whole a11y group and
noUnusedImports. Rules that already had violations are pinned to warn so CI
stays green; clean rules keep their default severity. Baseline and the
warn-to-error plan are in frontend/docs/biome-rule-ratchet.md.

Remove every eslint-disable comment (Biome ignores them). Justified ones become
biome-ignore with a reason; the rest are dropped and surface as warnings.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* UN-4184 Address review: drop unjustified CustomSynonyms suppression, clarify ratchet doc

The rows memo's delete handler also reads sessionDetails and details.tool_id,
so the biome-ignore reason was wrong; let it surface as a warning (UN-4202).
State that every rule in the ratchet table ends at error, including
noUnusedImports whose default is warn.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

…tree (#2304)

Cloud CI runs this suite against the OSS tree with the cloud plugins copied
in, so plugins/vlm_image_answer is importable and
test_cloud_package_absent_in_oss fails on every cloud run now that #2210 has
merged and unstract-cloud#1690 supplies the plugin.

Skip the class wholesale rather than that one test: once the hooks are real,
the remaining "returns None" / "does not raise" assertions stop describing
no-ops and pass for reasons they never meant to check. Delegation in that
tree is already covered by the cloud_hooks tests.

Verified both ways: OSS-only tree 10 passed; with plugins/vlm_image_answer
copied in, 5 passed and 5 skipped.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

* UN-2238 [FIX] CSP: allow third-party origins found in report-only sweep

Sweep of the report-only policy on a live deployment (browsing + per-directive
probes) turned up hosts the shipped bundle loads but no directive allows:

- style-src/font-src: cdn.jsdelivr.net (Monaco CSS + codicon)
- img-src: cdn.jsdelivr.net (emoji-datasource), ProductFruits, GTM, GA,
  reCAPTCHA assets, q.stripe.com
- media-src: cdn.productfruits.com (new directive; default-src had no blob:)
- connect-src: unpkg.com, api.productfruits.com, GA4 regional endpoints,
  reCAPTCHA api2, m.stripe.network
- frame-src: googletagmanager ns.html, m.stripe.network

ProductFruits' animations.css was the one violation observed in normal use; the
rest belong to code paths and flows that were not exercised.

Drops the bare `wss:` wildcard: socket.io connects to window.location.origin
(GetStaticData getBaseUrl) and 'self' covers same-origin ws/wss per CSP3,
verified with a ws:// probe against nginx serving this policy.

Adds .claude/skills/csp-check so the policy can be re-checked against a build or
a deployment when a frontend dependency changes.

* UN-2238 [FIX] Enforce the frontend CSP instead of only reporting it

Renames the header from Content-Security-Policy-Report-Only to
Content-Security-Policy. The policy value is byte-identical to the one the
previous commit landed after the origin sweep -- the only change is that a
resource from an unlisted origin is now blocked rather than logged.

That sweep predated the shadcn/React 19 frontend migration, so it was rechecked
against both bundles before flipping:

- the OSS build (`bun run build`, 111 chunks): every external host it references
  is in the policy
- the live us-central deployment (363 chunks, which includes the cloud plugin
  code the OSS build does not have): same

The one host either scan turned up that no directive allows is react.dev, which
appears only in React 19's error-message links and is never fetched -- added to
scan_origins.py's IGNORED set rather than to the policy.

Also refreshes the csp-check skill: it described a report-only policy and a
`npm run build`/frontend/dist layout that is now `bun run build`/frontend/build.

* UN-2238 [FIX] Address review: make the CSP gate trustworthy, widen GA4 hosts

Praveen's correctness pass found the static gate could pass without checking
anything, and that two policy entries do not survive enforcement.

scan_origins.py
- `--dist` is relative to the shell, and the documented `--dist frontend/build`
  run from scripts/ resolved to a directory that does not exist. rglob on a
  missing directory yields nothing rather than raising, so the run printed
  "Scanned 0 bundle files" and then "Every external host ... appears in the
  policy" and exited 0. Now: a missing --dist is a hard error, an empty scan is
  a failure, a URL whose index names no /assets/ chunk (a login wall) is a
  failure, and a chunk that fails to fetch for any reason but 404 is a failure.
  404 stays benign because RELATIVE_ASSET_RE also matches worker paths a chunk
  only names as a string (/assets/pdf.worker.js on us-central).
- allowed_hosts() dropped the path from path-scoped sources, so
  `https://www.google.com/recaptcha/` was read as a grant on the whole host and
  any www.google.com URL scanned as covered. It now keeps the prefix and matches
  URL paths, and understands `*.` as the browser does (subdomains only, so
  `*.google-analytics.com` does not cover a bare google-analytics.com).
- index.html and /config/runtime-config.js are now scanned; neither is under
  /assets/ and the second carries the operator-set branding URLs.

nginx.conf
- GA4 picks a regional collection host at runtime, so pinning
  region1.google-analytics.com + analytics.google.com drops the rest. Switched
  the Google analytics hosts to the wildcards Google publishes for CSP.
  Signals/Ads endpoints are deliberately still not granted, and the comment says
  so, because enabling those in the GTM UI has to come back through this file.
- Recorded that VITE_CUSTOM_LOGO_URL / VITE_FAVICON_PATH are governed by img-src
  and must be same-origin paths; both fail silently otherwise.

Verified: matcher unit cases for the recaptcha/gstatic path scoping and the GA4
wildcards; --dist scan of a fresh build (113 files) and --url scan of us-central
(368 files, up from 363 now that the entry documents are included) both clean.

* UN-2238 [FIX] Address Greptile: a missing real chunk must fail the scan

Two ways the gate could still clear a deployment it had not checked.

- 404 was blanket-benign, so a genuinely missing /assets/ chunk was skipped
  with the rest of its origins. The queue now records where each path came
  from: a chunk index.html links is required and any failure on it, 404
  included, fails the scan; a path found by following RELATIVE_ASSET_RE inside
  a bundle stays speculative, because that regex also matches worker paths a
  chunk only names as a string (/assets/pdf.worker.js on us-central).
- read_dist counts index.html, so a directory holding nothing but index.html
  passed the emptiness check having inspected no bundle at all. It now
  requires at least one .js.

Also wraps fetches in a 3-attempt retry. A single transient TLS reset was
enough to redden a 368-chunk --url scan, which would have taught everyone to
ignore the gate. An HTTP status is an answer, not a blip, so it is not retried.

Verified: --dist scan of a fresh build clean; an index.html-only directory now
exits 1; --url scan of us-central clean at 368 files with the pdf.worker.js 404
still tolerated.

* UN-2238 [FIX] Parse the policy the way the browser does; probe the wildcards

A second review pass over the whole PR, not just the last commit.

extract_policy.py feeds both the scanner and the browser probe, so when it
reads a different policy than nginx serves, every check downstream is checking
a fiction. Two ways that happened:

- `HEADER_RE.search()` took the first match anywhere in the file. A
  commented-out add_header won over the live one, and a -Report-Only header
  placed above the enforcing one won too. The second is the realistic case: a
  Report-Only header alongside the enforcing one is how you trial the next
  policy change, and is the shape this repo was in until this PR. Comment
  lines are now stripped, the enforcing header is preferred, and two of the
  same kind is an error rather than a silent pick.
- Duplicate directives were parsed last-wins; browsers honour the first and
  ignore the rest, so `img-src 'self'; ...; img-src https://evil.example` made
  the gate clear a source the browser never applies. First now wins, with a
  warning naming the one being ignored. Latent today -- the policy has 13
  directives and no duplicates -- but it is a 1.6 KB hand-edited line.

probe.js could not test a wildcard source at all: `https://*.example.com/...` is
not a hostname, so the request died before CSP saw it and the probe read clean
whether or not the deployment served the wildcard. It now substitutes a
concrete label, which is a real test -- CSP is evaluated before DNS, which is
why the csp-control.invalid controls get reported without resolving. This
matters as of the previous commit, which is what put wildcards in the policy.

nginx.conf: font-src was the only directive that never got the Product Fruits
hosts. It renders into our DOM rather than a cross-origin iframe, so @font-face
in its stylesheet resolves under our font-src and would have rendered tofu in
tours and checklists. Added, pending confirmation with check 3 with a tour open.

Also records in the skill that `sessionDetails.picture` is the same shape of
hole as the logo and favicon: an absolute URL the auth backend may supply, with
no IdP host in img-src. It does not reproduce today -- neither the OSS
AuthenticationService nor the cloud Auth0 plugin populates it, so the initials
branch always renders -- but the field is plumbed end to end, and if it is ever
filled the avatar is blocked and shows a broken-image glyph rather than falling
back to initials, because `picture` would be truthy.

Verified: the three parser cases (commented-out, Report-Only first, duplicate
directive) each now resolve the way the browser would; --dist scan of a fresh
build still clean.

* UN-2238 [FIX] Port, non-fetch directives, and the hangs the retry could cause

Third review pass, all in the scanner.

- URL_RE stopped at a `:port`, so `https://example.com:8443/x` scanned as bare
  `example.com` with no path. A CSP source with no port grants 443 only, so the
  browser blocks while the gate reported allowed -- and the swallowed path also
  defeated path-scoped checking. Port is now captured and matched, with a new
  PORT verdict.
- allowed_sources() walked every directive, so a host in form-action, base-uri
  or frame-ancestors read as fetchable from any directive. Nothing can be
  loaded on their strength; they are skipped, along with the reporting ones.
- urlopen had no timeout, and the 3-attempt retry added on top meant a
  deployment that accepts a connection and never answers would hang the gate
  forever. 20s.
- The (path, required) queue popped LIFO while deduping on a bare set, so when
  the same path arrived from index.html and from inside a bundle, whichever was
  popped first decided how a 404 was treated -- the provenance distinction
  could be lost for exactly the path it was written for. Required-ness now
  lives in a dict keyed by path, so the order cannot matter.
- The index fetch was outside any try, so a DNS or TLS failure there ended in a
  traceback rather than the script's own incomplete-scan message.

--dist now defaults to this repo's frontend/build, resolved from the script
rather than the shell. The relative path was only ever needed because there was
no default, and it is what made the vacuous-pass bug reachable in the first
place.

SKILL.md: dropped two sentences narrating states no shipped version has (that
the policy "was report-only until then", that a mistyped --dist "used to" pass)
and corrected the exit-code rule, which stopped being "any chunk that cannot be
fetched" when 404s on speculative paths became tolerable.

Verified: matcher cases for port (js.stripe.com:8443 -> PORT) and for the
navigation directives (checkout.stripe.com no longer in the allowed map) on top
of the existing path and wildcard cases; --dist scan clean at 113 files, --url
scan of us-central clean at 368.

* UN-2238 [MISC] Record why the US PostHog hosts are in IGNORED

They are posthog-js's built-in defaults, not documentation strings like the
rest of the list, and they are inert only because api_host is pinned to the EU
host in frontend/src/index.jsx. Move that pin and they become live hosts the
policy blocks, with IGNORED keeping the gate quiet about it -- which is the one
thing this list must never do silently.

unpkg.com was raised in the same pass as dead weight, on the grounds that
nothing in the OSS bundle references it. Left as is: the us-central scan finds
it in PdfViewer-*.js, so it is the LLMWhisperer PDF worker in the cloud plugin
and dropping it would block that viewer. The OSS bundle is a subset of what the
policy has to cover.

…ed ones (#2305)

UN-4185 Declare implicit frontend deps, drop unused ones, move test libs to devDependencies

Declare prop-types, lodash, clsx and @react-pdf-viewer/thumbnail, which were
imported but only resolved transitively. Remove zod, @hookform/resolvers,
js-yaml, cron-validator, http-proxy-middleware, @fontsource/geist-sans, jsdom
and react-syntax-highlighter, none of which OSS or cloud plugins import after
the paired cloud change. Move @testing-library/* to devDependencies.

… pg_barrier dispatch (#2308)

[MISC] Raise the rig Postgres lock limit and mock dispatch in pg_barrier enqueue tests

The integration tier fails intermittently for two unrelated reasons.

Backend: every xdist worker migrates its own test database in one
transaction, holding a lock per table and constraint. The rig's Postgres
runs with the default max_locks_per_transaction=64, so the shared lock
table overflows at random ("out of shared memory") and errors hundreds of
backend tests at setup. Start the container with 256.

Workers: five TestPgBarrierEnqueue tests never mocked
queue_backend.dispatch.dispatch. Since UN-4078 made the PG queue the only
transport, enqueue really dispatches the headers, opening a connection
from DB_* env (default host unstract-db) instead of the TEST_DB_* test
database. Wrap them in the same patch their neighbours use. Also drop a
duplicated _barrier_pg_decrement import.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

…sly (#2299)

* UN-4123 [FIX] A credential-free REDIS_URL no longer connects anonymously

In URL mode the resolved password was never passed to the client: credentials
had to be embedded in the URL, and a {prefix}PASSWORD set beside a
credential-free URL was silently ignored. The client connected ANONYMOUSLY and
the endpoint answered NOAUTH on the first command, which reads as a broken
server rather than a dropped password.

This was not exercised by the live managed-Redis testing on UN-4123, because
both configurations tried there avoided it: the URL-mode run embedded the
password in the URL, and the discrete run had no URL at all. The gap sits in the
third combination, which is the one worth recommending.

WHY IT IS WORTH RECOMMENDING. A URL ends up in places a password should not:
the endpoint helper's error messages, ArgoCD's ComparisonError condition, and —
under ESO — the ExternalSecret's spec.target.template.data, which is not a
Secret and is not redacted. All three were raised in review of the chart-side
PR, and all three exist because the password is in a URL. Keeping it in its own
key closes the class rather than patching each place the URL is printed; this
change is what makes that configuration usable.

Credentials FILL A GAP the URL leaves and never override one.
ConnectionPool.from_url ends with kwargs.update(url_options), so an existing
rediss://:<password>@host URL keeps behaving exactly as before — verified for
both the :pw@ and user:pw@ spellings.

GATED ON THE PASSWORD, with the username riding along. A username alone is not a
credential, and values.yaml ships REDIS_USER: default: filling that in on its own
would make redis-py send AUTH to the in-cluster server, which has none, turning
a working default deployment into a failing one. I wrote that regression into
the first version of this change and caught it by testing the chart's own
default posture rather than by reading the diff.

Six cases added, covering both URL spellings, the gap-filling case, the username
pairing, the username-alone regression, and the legitimately anonymous server.
Mutation-verified: removing the fallback fails two, ungating the username fails
the third.

The three sample.env recipes now show the password as a separate key, and say
that an existing URL-embedded credential still works.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FIX] Carry the credential fallback to the other two Redis consumers

The first commit fixed create_redis_client and stopped there, which left the
recommendation it enables half true. unstract has THREE Redis consumers and a
credential-free URL reached only one of them:

  create_redis_client      backend, workers, runner, platform-service   fixed
  build_socketio_redis_url kombu / Socket.IO                            ANONYMOUS
  Django cache LOCATION    django-redis                                 ANONYMOUS

Both now fill the same gap, on the same terms: credentials are added only when
the URL carries none, and a URL bearing them still wins.

SOCKET.IO. kombu takes a URL and nothing else — no connection kwargs — so a
separately supplied password cannot reach it any other way; the credentials go
into the URL string itself. The password is then in that string, unavoidably.
That is acceptable here and not in a values file: this URL is built in-process
and handed straight to the client, rather than written into a manifest, a Secret
template or an error message, which is the distinction the whole change is
about. An anonymous publisher against an authenticated server does not fail
loudly — Socket.IO events simply stop arriving.

DJANGO CACHE. django-redis reads OPTIONS["PASSWORD"] and feeds it to
ConnectionPool.from_url, which ends with kwargs.update(url_options) — so setting
it is safe even when the URL has its own. It is still skipped when the URL
carries credentials, because passing them twice invites the two to disagree.

Five cases added. The settings harness now splices the urllib import from source
instead of hand-writing it: the hand-written copy had to be remembered the
moment settings started using another name from that module, and the symptom was
a NameError in every case rather than one clear failure. That is the second time
a hand-copied import in this harness has broken; the unstract.core import block
was the first.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FIX] Two credential-resolution findings from review

BOTH ARE IN MY OWN FALLBACK, and both are cases the fallback gets wrong rather
than cases it misses.

A URL CARRYING ONLY A USERNAME NO LONGER DROPS THE PASSWORD. The check keyed on
"@" being absent from the netloc, which is not the same question: redis://alice@host
carries an @ and no password. The fallback was skipped, the client got a username
with no password, and could not authenticate. Keyed on urlsplit().password being
None instead, which asks it directly. apply_url_credentials now uses the same
test and keeps a username the URL already carries.

CREDENTIALS RESOLVE AT THE URL'S OWN LEVEL, by the same rule the database
already follows. A prefix that brought its OWN url may point at a DIFFERENT
endpoint — an anonymous one, say — while the generic REDIS_PASSWORD is set for
the primary. Letting that password reach across made the prefixed client send
AUTH to a server with none, turning a working connection into a failing one. An
INHERITED url is the same endpoint as the generic one, so the full fallback
chain still applies there. For env_prefix "REDIS_" the two levels are the same
variable and nothing changes.

That the database rule and the credential rule now say the same sentence is the
point: "at the URL's own level" is one idea an operator can hold, rather than
two resolution orders that happen to differ.

Five cases added, covering the username-only URL, the prefix that must not
inherit, the prefix's own password, the inherited url that still should, and
the Socket.IO builder following the same rule.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FIX] One credential helper for all three consumers, and the netloc bug in it

A four-agent standardised review of this PR found two Criticals, both in code I
added in the two commits before it. Every fix below is mutation-verified.

apply_url_credentials CORRUPTED A URL THAT CARRIED A USERNAME. parts.netloc
INCLUDES userinfo, and the function prepended to it — so redis://alice@host with
a separately supplied password became redis://alice:pw@alice@host. Userinfo
splits on the LAST "@", so kombu parsed the password as "pw@alice". Not a
missing credential, a WRONG one: the server answers WRONGPASS, the publisher
dies, and socketio.Server is built with logger=False so Socket.IO events simply
stop. The docstring claimed this function existed to prevent exactly that.

It now rebuilds the netloc from the host part, and reuses a username the URL
already carries VERBATIM rather than re-quoting it — quoting an already-encoded
username turned al%40ice into al%2540ice.

THE DJANGO CACHE KEYED ON "@" BEING ABSENT — the precise heuristic the core
comment rejects, 170 lines away, in the same PR. A username-only URL left the
cache anonymous while create_redis_client authenticated: one process, one
endpoint, two outcomes, which is the sentence this work exists to delete.

Rather than align the third hand-rolled gate, it is gone: the cache LOCATION now
goes through apply_url_credentials, the same helper the Socket.IO URL uses. That
also closes a gap neither gate could — django-redis 5.4.0 discards
OPTIONS["USERNAME"], so an ACL username could not reach this cache at all and it
authenticated as `default` while the other consumers used the configured user.
It reads REDIS_USER straight from os.environ rather than the module-level
fallback of "default", so the cache and the client send the same AUTH form.

EMPTY IS ABSENT. redis://:@host parses to password "", and both redis-py
(`if url.password:`) and kombu (`unquote(password or "") or None`) read that as
no password — so `is None` declined to fill a gap the libraries agree is a gap.
That shape is what Helm emits from redis://:{{ .Values.password }}@host with an
empty value. Truthiness in all three places now.

A DEAD GUARD REMOVED. `and not parts.username` could not fail: from_url ends
with kwargs.update(url_options), so a URL username wins regardless. An
unfalsifiable guard is where a later reader deletes something load-bearing.

TESTS 250 -> 257, and the existing ones were not strong enough. The socketio
credential test asserted a SUBSTRING, which passes on the corrupt URL — that is
why the Critical survived a test written for the same shape. Assertions are on
parsed fields now. Added: username-only URLs on every consumer, double-encoding,
a password containing @ / : (unencoded it reparses with host "ss" — a different
server), the empty-password shape, the {prefix}USERNAME spelling, and the cache
carrying an ACL username. Six mutations verified, including the netloc concat.

COMMENT CORRECTIONS. "kombu takes a URL and NOTHING else — no connection
kwargs" is false in both places it appeared: KombuManager forwards
connection_options to kombu.Connection, which takes userid= and password=. The
URL is where TLS must go; credentials ride along by choice, not necessity. And
"never written into an error message" was wrong — the backend stores the result
as settings.SOCKET_IO_MANAGER_URL, which Django's SafeExceptionReporterFilter
does not redact. Two past-tense comments describing a mid-branch state as
shipped history are now present tense.

The three sample.env recipes now show REDIS_USER= blank, because this change
makes that value load-bearing in URL mode for the first time while the files
still ship REDIS_USER=default a few lines above.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FIX] Round-2 review: one resolver for usernames, and blank means unset

Second standard-review round. Round 1's two Criticals held up under attack —
apply_url_credentials was fed 16 URL shapes round-tripped through urlsplit,
redis-py and kombu, and all three agree on every one. What round 2 found is a
layer down: the credential CHAIN, and tests that could not fail.

* base.py re-spelled the username chain instead of asking core for it, and
  dropped a leg. The resolver accepts REDIS_USER and REDIS_USERNAME;
  platform-service ships the second, so one shared Redis-credential secret can
  inject it — and the Django cache then authenticated as `default` while
  create_redis_client in the SAME process authenticated as the configured ACL
  user. Where `default` is disabled, the cache alone fails. That is verbatim
  the divergence the round-1 comment says the change exists to delete, so the
  chain now lives in one exported helper both sides call.

* BLANK MEANT SET for credentials, which broke this PR's own recipe.
  workers/sample.env ships `CACHE_REDIS_PASSWORD=` uncommented, and
  `os.getenv(prefixed, os.getenv(generic))` hands back that blank rather than
  consulting REDIS_PASSWORD. An operator following the documented managed-Redis
  recipe — REDIS_URL without credentials plus REDIS_PASSWORD — got url_password
  "" on every prefixed client, apply_url_credentials no-opped, and the worker
  caches connected ANONYMOUSLY. The module already documented blank-means-unset
  for parse_db and _parse_bool; host, port, password and user now follow it.

  This changes a test that asserted the old behaviour. It was a characterisation
  test that said so — "documents a trap rather than endorsing it… this test
  fails if that behaviour ever changes, so the chart-side guarantee can be
  revisited". This is that revisit: the guarantee it rested on (the chart never
  renders an empty credential) is not one sample.env keeps.

* PORT was the one var with no error contract — int() straight off os.getenv,
  so a blank REDIS_PORT= raised at client construction naming neither the
  variable nor the prefix. parse_port now mirrors parse_db.

TESTS — the round-1 lesson had not actually been applied. Four assertions still
compared substrings of a constructed URL, including one two lines below the
test I fixed in round 1, carrying a comment explaining the very bug it was
still vulnerable to. Each is replaced by parsed fields, and the password one by
an unquote() round-trip, since a hostname guard catches only the host half of a
doubled userinfo.

Two mutations survived round 2 and now fail:

* reversing url_user/username precedence — no test set a URL username AND
  REDIS_USER at once, which is the only environment that tells them apart, and
  values.yaml ships REDIS_USER: default so the pairing is ordinary.
* deleting `not parts.password` — harmless for the password (from_url ends with
  kwargs.update(url_options)) but load-bearing for the USERNAME rider, so it
  turned a one-argument AUTH into a two-argument one while the Socket.IO URL
  for the same config kept sending one. The comment above it argued the guard
  was unfalsifiable, which is true of the password half and false of the other
  — i.e. it documented the deletion the suite could not catch.

The _derive harness executes two source ranges with a ~415-line gap between
them, and anything in that gap is invisible to every assertion in the file:
adding `REDIS_PASSWORD = ""` at base.py:251 leaves all 44 tests green while
shipping a broken cache. It now asserts its own coverage — every Redis line in
base.py must fall inside a spliced range — and fails naming the line that
escaped. Verified against that exact insertion.

Also: test_the_shipped_default_user_alone_adds_nothing passed REDIS_PASSWORD=""
and so returned at the `if not password` guard before the username was read,
making the test named for that choice blind to it.

Core 120 -> 126 (263 in the dir), backend 42 -> 44. Six mutations verified
caught. The exposure note now names CACHES["default"]["LOCATION"] as the second
unredacted landing place for the password, and says why it is a deliberate
trade rather than an oversight.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FIX] Round 3: finish the blank-means-unset convention, and stop stripping

Review of the round-2 commit. Two of the three findings are defects that commit
introduced, and one of them made the change actively worse than before it.

* BLANK-MEANS-UNSET WAS APPLIED TO CREDENTIALS BUT NOT TO TLS. Round 2 made a
  blank {prefix}PASSWORD/USER/HOST fall through to the generic level; the four
  TLS vars kept shadowing. So `CACHE_REDIS_SSL=` beside `REDIS_SSL=true` now
  gave the worker cache a real AUTH over an UNENCRYPTED socket — the credential
  round 2 taught it to inherit, on the wire in clear. Before round 2 the
  password was suppressed too and the client simply failed to authenticate.
  Half a convention was worse than none. All four TLS vars go through env_chain
  now, and the comment claiming the rule held "throughout this module" is true
  again rather than aspirational.

* env_chain STRIPPED what it returned, and backend/settings/base.py does not.
  A password or username from a file-mounted secret ends in a newline, so core
  truncated it while the settings module kept it, and the Socket.IO URL and the
  cache LOCATION — built ten lines apart — carried different secrets. Emptiness
  is now tested on the stripped value and the RAW value returned, so the two
  agree again. That is the property round 2 claimed to establish.

* url_username_from_env, the helper round 2 added specifically to unify this
  chain, was itself the one place not honouring the rule: plain `or` with no
  blank handling, so ` alice ` and `appuser\n` resolved differently from every
  other read of the same variable. It calls env_chain now.

Also:

* base.py still did a bare int() on REDIS_PORT, so a blank REDIS_PORT= raised
  during Django settings import — the backend alone failing to start while
  every worker came up fine on 6379. parse_port was exported in round 2 and
  simply not wired up here.

* parse_port and parse_db named the PREFIXED variable in their diagnostics even
  when the bad value came from the generic one, sending an operator to grep for
  a key that is not set. env_chain_named returns the winning name, and the port
  fallback is logged at error rather than warning: the fallback is a PORT, so on
  a host serving both 6379 and 6380 a typo lands the client on the other server
  rather than failing.

TestContainerAllowlists caught something real and deserves the credit: it
derives the container env allowlist by scanning `os.getenv` calls in the source,
so rewriting those reads to go through env_chain made the whole TLS set vanish
from the guard silently. That is the same drift class as the two test harnesses
fixed in round 2. The derivation now scans the arguments of every env-reading
call, still scoped to those call names so a variable merely named in a docstring
is not demanded of every container.

Core 126 -> 137 (274 in the dir). Five mutations verified caught, including both
blank-TLS shadows and the strip asymmetry.

NOT fixed here, recorded deliberately: unstract-cloud's values.yaml pins
CACHE_REDIS_USERNAME: "" with a comment stating the shadowing is intentional and
"must not be fixed". That reasoning is now void — the pin is inert rather than
load-bearing. The wire change is benign (Redis 6+ treats `AUTH default <pw>` and
`AUTH <pw>` alike, and redis-py retries one-arg anyway), but the comment belongs
in the cloud PR, not this one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FIX] Round 4: the SSL flag parses the same in both consumers

Round 3 broadened REDIS_SSL parsing in core and left backend/settings/base.py on
its own `== "true"`. _TRUE_LITERALS accepts 1, yes and on, so the two halves of
one process disagreed about whether TLS was on:

  REDIS_SSL=1  ->  every create_redis_client consumer connects rediss://
                   the Django cache builds a redis:// LOCATION
                   and `if REDIS_SSL` never runs, so CONNECTION_POOL_KWARGS
                   carries no cert_reqs, no check_hostname, no CA either

Against a TLS-only managed endpoint the backend cache alone fails; against one
accepting both it authenticates in clear while everything else is encrypted. In
the same process SOCKET_IO_MANAGER_URL — built by core — came back rediss://.
That is verbatim the "one endpoint, one process, two policies" failure this
module was centralised to prevent, reintroduced by centralising only one side.
resolve_ssl_enabled is exported and base.py calls it.

Also:

* The fifth read of SSL_CHECK_HOSTNAME was missed by round 3's conversion. It
  decides whether the value was EXPLICIT, so a blank prefixed value fell
  through for the value but not for the explicitness — on Sentinel + TLS a
  global REDIS_SSL_CHECK_HOSTNAME=true was honoured on the discovery
  connections and silently dropped on the master one.

* parse_port treated "" as unparseable and logged at ERROR. base.py hands it
  the raw env value rather than env_chain's None, so a bare REDIS_PORT= — the
  repo's own "leave the default" spelling, and the wording of the comment round
  3 added right above it — produced a startup error line in the backend and
  nowhere else.

* host is stripped explicitly. env_chain returns the raw value so a credential
  keeps whatever whitespace it was given, but urlsplit() drops a trailing
  newline from a URL host while redis.Redis(host=...) keeps it, so a
  file-mounted REDIS_HOST would fail DNS on the discrete path and succeed on
  the URL one.

Core 137 -> 139 (276 in the dir), backend 44 -> 57. The new backend class
parametrises every true and false literal across both consumers, which is what
pins them together; nothing previously exercised any literal but "true".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* UN-4123 [FIX] Pin the property, not the bug: a consumer-parity sweep

Four review rounds produced the same finding four times in different clothes —
a setting centralised in unstract.core with the parallel read in base.py left
behind. The "@" credential heuristic, the REDIS_USERNAME spelling,
blank-means-unset, and the SSL true-literals were each fixed with a test for
that one setting, and each time the next round found the next one.

So this pins the property instead. TestTheBackendAgreesWithCoreOnEverySharedSetting
runs 19 environments an operator would actually write and asserts that where the
Django cache ends up — host, port, db, username, password, TLS — is where
create_redis_client in the same process ends up. A setting centralised on one
side only now fails without anyone having to predict which setting it will be.

It found a real divergence on its first run, and not one of the four above: the
DISCRETE path cannot carry an ACL username. django-redis discards
OPTIONS["USERNAME"], so with REDIS_HOST + REDIS_USER + REDIS_PASSWORD the cache
authenticated as the built-in `default` while every other client in the process
authenticated as the configured user — and where `default` is disabled, the
cache alone failed. That is the identical bug the URL branch was fixed for,
left standing on the path the module docstring calls primary.

The comment beside it said this was deliberate ("auth stays password-only as the
built-in `default` user ... this cache never sends one"), and a test pinned it as
a "stated invariant" so a django-redis bump would be visible. Passing a key the
library throws away is not an invariant worth pinning; reaching the same server
as everything else is. Both branches now build the LOCATION through
apply_url_credentials.

The password still goes to OPTIONS["PASSWORD"] when no username is in play:
that key matches Django's SafeExceptionReporterFilter and is masked in a
settings dump, while LOCATION is not — so the extra exposure is paid only where
django-redis leaves no alternative.

Backend 57 -> 77. Three mutations verified caught by the sweep, including two it
was not specifically written for.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FIX] Round 5: the sweep asks django-redis instead of imitating it

Round 5 found no Critical and no High — the recurring "centralised on one side
only" class has stopped on the path the parity sweep covers, confirmed against
an exhaustive 2160-configuration matrix. These are the Mediums it did raise.

* The sweep reimplemented django-redis's precedence. It parsed the LOCATION
  with urlsplit and hardcoded "userinfo wins, OPTIONS['PASSWORD'] backfills".
  That is correct for 5.4.0 — but it is a second copy of library behaviour
  inside the one test whose stated job is to survive changes it cannot predict,
  so a bump that reversed the precedence would leave it green while the cache
  diverged. It now builds the dict from a real ConnectionFactory, so the rule is
  never restated. (Its pools are memoised by LOCATION, so they are cleared per
  case; leaving that out makes three cases answer with an earlier case's pool,
  verified.)

* An ACL username with NO password reached neither side usefully and reached
  them differently: apply_url_credentials returns the URL untouched when there
  is no password, so the cache went anonymous, while core set username=alice and
  redis-py packed `AUTH alice None` and raised DataError on the FIRST command —
  a type error from inside the library, nowhere near the values file that caused
  it. Redis has no one-argument ACL AUTH, so this is not a configuration that
  exists; it is now reported at startup and the username dropped, which also
  makes the two sides agree. The sweep's 19 cases paired a username with a
  password every time, so it could not see this; two cases added.

* The db relocation warning gave the discrete-mode explanation on both paths —
  it fires in URL mode too, where the previous db came from the URL's own path
  and OPTIONS['DB'] was never the mechanism. The db numbers were right in every
  case; only the stated reason was wrong. Reason dropped, numbers kept.

The Sentinel branch is now marked as OUTSIDE the sweep, with the four
divergences from create_redis_client that live there spelled out — the
"default" username default, the ignored REDIS_USERNAME spelling, no TLS at all,
and a different default port. All four are pre-existing on main and this PR only
replaced an int() there; changing behaviour in a branch no test in this file
executes is its own ticket, not this one.

Backend 77 -> 79. Two mutations verified caught.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FIX] Extend the cache/client parity to Sentinel mode

The parity sweep stopped at the Sentinel branch, and I recorded the four
divergences living there as "its own ticket". That was the wrong call twice
over: a guarantee that stops at a mode boundary is a half-truth, and the reason
I gave for deferring — "changing code no test in this file executes" — was not
true. _derive's slice spans BOTH branches; it just needed an env selecting this
one. "No test executes it" and "no test was written for it" are different
statements, and only the second was ever the case.

Four divergences from create_redis_client, all pre-existing on main:

* THE SENTINEL PORT. The module-level REDIS_PORT defaults to 6379 — the
  STANDALONE port — while core's Sentinel path resolves with
  default_port="26379". With REDIS_PORT unset this cache looked for sentinels
  on the wrong port while every other client found them. The chart always sets
  REDIS_PORT, which is why it stayed hidden.

* THE USERNAME, twice. REDIS_USER defaults to "default" at module scope, so
  this cache sent a two-argument ACL AUTH where core sends the one-argument
  form; and it read REDIS_USER directly, so the REDIS_USERNAME spelling
  platform-service ships was ignored. Both now come from the shared resolver,
  and the same value reaches SENTINEL_KWARGS, the LOCATION and the Socket.IO
  URL.

* TLS REACHED NEITHER CONNECTION. A TLS-only Sentinel deployment got a
  plaintext cache while core encrypted both the discovery and the master
  connections. Carrying it in SENTINEL_KWARGS alone would encrypt discovery and
  leave the master in clear, so it goes to CONNECTION_POOL_KWARGS too — core
  builds both from one env dict for exactly this reason.

BEHAVIOUR CHANGES for existing Sentinel deployments, stated rather than buried:
an unset REDIS_PORT now resolves 26379 instead of 6379; no username is sent
unless one is configured (Redis 6+ treats `AUTH default <pw>` and `AUTH <pw>`
as the same command, and redis-py retries one-arg regardless, so this is a wire
change rather than an auth change); and REDIS_SSL now applies here. Flag off is
byte-identical — asserted.

Backend 79 -> 87. Four mutations verified caught.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FIX] Sentinel's two TLS planes verify by different rules

Valid review finding, and a parity bug in the commit that was meant to
establish parity: 9019ac1 built one TLS dict and copied it into both
SENTINEL_KWARGS and CONNECTION_POOL_KWARGS.

The two planes are not the same connection. Discovery reaches REDIS_HOST, a
name whose certificate can match, so it verifies like any other client. The
MASTER reaches whatever `SENTINEL get-master-addr-by-name` returns, and
SentinelManagedConnection assigns that address straight to self.host, which
SSLConnection passes as server_hostname — an IP that no DNS SAN covers, and one
that changes on failover, so pinning an IP SAN is not an answer either. core
turns checking off there unless the operator asked explicitly (_tls_kwargs's
sentinel_master branch). This cache did not, so it failed certificate
verification against the very master create_redis_client connects to.

The second half of the finding is also real: with REDIS_SSL_CERT_REQS=none the
copied default of True produced CERT_NONE plus hostname checking, and the
pinned redis-py 5.2.1 does NOT coerce that pair — it assigns check_hostname
verbatim, so it reaches Python's ssl module, which rejects it. Verified against
the installed version rather than assumed; 6.x does coerce, which is why the
sibling repo's fix looks different. The standalone branch already guarded this
three hundred lines below; the Sentinel branch did not.

The master-plane rule is exported from core as
resolve_sentinel_master_check_hostname rather than re-derived here — the same
reasoning as every other shared setting in this PR, and the reason it is a new
helper instead of a flag on resolve_ssl_check_hostname is that core's own call
site applies the cert_reqs rule immediately afterwards and would double-apply
it.

Backend 87 -> 92. Two mutations verified caught.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

…or (#2315)

* UN-4123 [FIX] The default Redis user without a password is not an error

6c0a69d added an ERROR when a username is set without a password, to replace
redis-py's opaque "DataError: Invalid input of type: 'NoneType'" on the first
command with something that names the values file. The diagnostic is right for a
named ACL user. It is wrong for "default".

"default" is Redis's own built-in user and `AUTH default <pw>` is equivalent to
`AUTH <pw>`, so "default with no password" is not a misconfiguration -- it is the
absence of ACL usage, which is the normal state for the in-cluster Redis that has
no password at all. values.yaml and sample.on-prem.values.yaml both ship
REDIS_USER: default, so the error fired on every default on-prem install.

Measured on two staging namespaces before this fix: ~274 lines in 20 minutes in
one and 400+ in the other, across eight pod types -- roughly 20k ERROR lines a
day per namespace. Nothing was broken; the username SHOULD be ignored and was.
The cost is log volume and real errors buried under it, which is what the
diagnostic was meant to prevent.

The username is still dropped either way, so behaviour is unchanged. Only the
logging is conditional, and a NON-default username without a password still
reports -- that is the case redis-py turns into the opaque DataError.

Found by deploying rc.423 to a vanilla in-cluster on-prem namespace. Both managed
configurations set REDIS_USER: "" explicitly, so neither could surface it; only
the shipped default does, which is what most on-prem installs run.

Mutation-checked: removing the exemption fails the three silence cases and
nothing else.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-4123 [FIX] Address review: the default-user exemption is case-sensitive

Redis ACL usernames are case-sensitive, so `DEFAULT` is a named user distinct
from the built-in `default`. Lowercasing the comparison suppressed the
missing-password diagnostic for it -- a real mistake made harder to diagnose,
which is the opposite of the point. Compare exactly.

The .strip() stays. env_chain returns the RAW value deliberately (stripping it
there truncated a password once, per the comment at its definition), so a
hand-edited values file can carry padding around a username that was meant to be
the built-in one. Whitespace is an editing artifact; case is semantic.

test_a_case_distinct_default_still_errors covers it, and "DEFAULT" is out of the
silence parametrize. Mutation-checked: restoring .lower() fails that test alone.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

…of losing the whole worker pod (#2311)

* UN-4223 Kill and re-fork only the hung child in the PG consumer supervisor

One child whose task hangs stops publishing heartbeats, which made the fleet's
/health 503 and a liveness kill drain every sibling for up to the grace period.
The supervisor now SIGKILLs just that child once it is silent past
HEALTH_STALE_SECONDS, so its lease lapses, the message redelivers, and the slot
is re-forked. Off when HEALTH_STALE_SECONDS is unset; CHILD_WATCHDOG=false
disables it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* UN-4223 Make the child watchdog opt-in and close the gaps the review found

- CHILD_WATCHDOG now defaults off: docker-compose has no healthcheck, so
  HEALTH_STALE was never enforced there and may sit below a long batch. The
  chart enables it where a liveness probe already killed at that threshold.
- poll_once stamps the heartbeat before each queue read, so a multi-queue
  consumer's age covers one task, which is what HEALTH_STALE is sized for.
- A child that never finishes loading is judged by time since fork.
- The child publishes its heartbeat before marking itself loaded.
- pg_consumer_child_watchdog_kills_total on /metrics.
- The loop iteration is _monitor_tick, now under test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* UN-4223 Correct two comments that still describe a per-cycle heartbeat stamp

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* UN-4223 Give a slow bootstrap its own floor and count only real watchdog kills

Review comments on #2311:
- An unloaded child is judged against max(HEALTH_STALE, 600s): HEALTH_STALE is
  sized for task runtime and can be 180s, while an import under a CPU cap can
  take minutes.
- A child that exits before the SIGKILL lands no longer counts as a kill.
- Split a composite test assertion (Sonar S9073).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

…enAI and Azure OpenAI (#2310)

* UN-4224 [FIX] Stop sending temperature to GPT-6 models on Bedrock, OpenAI and Azure OpenAI

GPT-6 models (luna, sol, astra, 6.1-sol) reject the `temperature`
parameter. Our LLM adapters always pass one (the pydantic default, or 1
when reasoning is on), and LiteLLM forwards it on the Bedrock Mantle
route, so every GPT-6 Luna call on Mantle failed with "temperature not
permitted for this model". GPT-5.6 was never affected because LiteLLM
has a GPT-5 rule that drops a non-1 temperature; that rule matches
`gpt-5` names only. The Converse route (`us.`/`global.` profile ids)
already drops it.

- Add a `gpt-6` stem to `_SAMPLING_DEPRECATED_MODEL_STEMS`, the strip
  already used for Claude Opus 4.7+. The Bedrock, Azure AI Foundry,
  Vertex and Anthropic adapters call it on their return path. The
  trailing-edge anchor keeps `gpt-5.6-*` (normalised to `gpt-5-6-*`),
  `gpt-60` and `gpt-oss-*` out.
- Call the strip from the native OpenAI adapter too.
- Azure OpenAI: the deployment name need not name the model, so detect
  GPT-6 from the optional Model field as well. `LLM` sets `cost_model`
  aside and re-validates without it, where Azure's default temperature
  of 1 would return, so pin `temperature` to None instead of popping it
  (LiteLLM omits a None temperature) and stop reasoning from forcing 1.

LiteLLM is intentionally not upgraded here: the first release whose
bundled registry carries the GPT-6 Mantle entries (1.104.0) needs
boto3>=1.43.1, which conflicts with the boto3 1.34 / s3fs 2024.x pins.
See UN-4224 for the routing gap that leaves on air-gapped deployments.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* UN-4224 [FIX] Keep the Azure GPT-6 strip when callers pass temperature per call

Review (Greptile P1): for an Azure OpenAI deployment whose name hides
GPT-6, the only record of the model that survived `LLM`'s re-validation
was the pinned `temperature: None`. `complete()` merges per-call kwargs
over the stored ones, so a caller passing `temperature=` (the cloud
agentic_table / agentic_extraction workers do) overwrote that marker and
the temperature reached a model that rejects it.

Detect from the real model id instead of a caller-writable marker.
`LLM._revalidate` now feeds the `cost_model` it set aside back into
re-validation, after the per-call kwargs so they cannot displace it, and
the four completion paths use it. Azure prefers that `cost_model` as the
original model id, so detection holds on every pass and the sampling
params are simply popped like every other adapter -- the `None` pin and
its sticky-marker check are gone.

Other adapters are unaffected: none declares `cost_model`, Pydantic
ignores unknown keys, and every call site already pops `cost_model`
before LiteLLM sees the kwargs.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* UN-4224 [FIX] Upgrade LiteLLM to 1.104.0 and the boto3 / fsspec storage stack

LiteLLM 1.104.0 is the first release whose bundled model registry carries
the GPT-6 Bedrock Mantle entries, so air-gapped deployments (which fall
back to the bundled registry) now route `openai.gpt-6-luna` to Mantle and
price GPT-6 calls instead of recording $0.

LiteLLM >= 1.100 hard-requires boto3 >= 1.43.1, which drags the storage
stack with it; these move together:

- litellm 1.96.2 -> 1.104.0
- boto3 / botocore 1.34.x -> 1.43.106, pinned exactly in backend, workers
  and sdk1: it must stay inside the botocore range aiobotocore 3.9.2
  accepts (1.43.101-1.43.106)
- s3fs / fsspec 2024.10.0 -> 2026.9.0 (s3fs drops its `[boto3]` extra)
- gcsfs 2024.10.0 -> 2026.10.0, adlfs 2024.7 -> 2026.8.0
- google-cloud-storage 2.9.0 -> 3.16.0: every gcsfs compatible with
  fsspec 2026.x needs google-cloud-storage 3.x. Our direct use (Client,
  bucket, get_blob, md5_hash, upload_from_*) is unchanged in 3.x.

Follow-on fixes:
- workers: bound `requires-python` to <3.13 like every other project. The
  open bound made uv resolve Python 3.14, where google-api-core >= 2.27
  needs protobuf >= 6.31 but the connectors' secret-manager / bigquery
  pins cap it below 5. The image runs Python 3.12.
- MinioFS: drop the UN-3487 `walk()` override. fsspec 2026.x's
  `DirFileSystem.walk()` relpaths each entry's `name` itself, so the
  override relpathed twice and tripped fsspec's assertion. The UN-3487
  regression test now guards the upstream behaviour.
- cohere embed timeout patch: re-pointed to 1.104.0 after re-diffing
  upstream; the sync `embedding()` still builds an untimed HTTPHandler.

Known gap, deliberately not fixed here: boto3 >= 1.36 stops sending
Content-MD5 on DeleteObjects, which MinIO older than RELEASE.2025-01-20
rejects. Verified live against the enterprise chart's
bitnami-minio:2024.12.18: single-file deletes (recursive=False, raw
fsspec) fail with MissingContentMD5, directory cleanup survives through
the UN-3421 fallback. MinIO 2026-09-22 (OSS compose) is unaffected.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* UN-4224 [FIX] Re-add Content-MD5 on S3 DeleteObjects for MinIO older than 2025-01-20

botocore 1.36+ sends a CRC32 flexible checksum instead of Content-MD5 on
DeleteObjects. MinIO older than RELEASE.2025-01-20 -- including the
enterprise chart's bitnami-minio:2024.12.18 -- rejects that with
MissingContentMD5, and s3fs routes every `rm` through DeleteObjects, so
with the boto3 1.43 upgrade single-file deletes (recursive=False, raw
fsspec) and connector deletes failed on those servers. The UN-3421
fallback only covered recursive deletes through sdk1 FileStorage.

Register a `before-call.s3.DeleteObjects` handler that adds Content-MD5
over the final (XML-escaped) body. It is appended to botocore's
BUILTIN_HANDLERS, so every session created afterwards gets it -- boto3,
botocore and the aiobotocore sessions s3fs creates. AWS S3 and new MinIO
accept both headers; S3 Express (which rejects MD5) is skipped.

`AWS_REQUEST_CHECKSUM_CALCULATION=when_required` does not help (the
operation requires a checksum), and botocore's deprecated
`conditionally_calculate_md5` skips requests that carry a flexible
checksum, so neither could be reused.

unstract-connectors does not depend on sdk1, so each carries an
identical, idempotent copy: sdk1 loads it from the file-storage helper,
connectors from the MinIO connector. Whichever loads first registers.

Verified live on the new stack against bitnami-minio 2024.12.18 and
MinIO 2026-09-22: FileStorage.rm(recursive=False), raw fsspec single and
bulk rm, and MinioFS rm (file and recursive) all succeed with no
fallback warnings.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* UN-4224 [FIX] Keep one copy of the DeleteObjects Content-MD5 hook

SonarCloud failed the quality gate on duplicated new code (39.2% vs 3%):
connectors carried a verbatim copy of sdk1's hook and its tests, on the
assumption that connectors does not depend on sdk1.

It does at runtime: `unstract_file_system.py`, the base class every
filesystem connector extends, imports `unstract.filesystem`, which
depends on sdk1. So the MinIO connector now imports
`unstract.sdk1.patches.s3_delete_objects_md5` directly, and the copy and
its duplicate tests are gone. Importing the connector still registers the
handler (verified), and connector deletes against MinIO 2024-12-18 still
succeed.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* UN-4224 [FIX] Address review: keep the storage wire format stable, cover GPT-6 gateways

Review findings from the PR review kit (Deepak), all reproduced first.

Storage (sdk1 `patches/storage_compat.py`, renamed from
`s3_delete_objects_md5.py`):
- [major] botocore >= 1.36 defaults `request_checksum_calculation` to
  `when_supported`, so over HTTPS every PutObject / UploadPart went out as
  `Content-Encoding: aws-chunked` with a CRC32 trailer -- including to
  Unstract Cloud Storage (GCS's S3 interop API) and TLS MinIO. Captured on
  the wire for boto3 put_object / upload_fileobj and s3fs pipe_file.
  `S3_CHECKSUM_CONFIG` (`when_required` for requests and responses) restores
  boto3 1.34's plain payload-signed requests and is now passed explicitly to
  every S3 client we create: sdk1 FileStorage (explicit config_kwargs still
  win), MinioFS (and so UCS), and the two boto3 clients in
  `backend/connector_v2/unstract_account.py`. DeleteObjects still requires a
  checksum, so the Content-MD5 hook stays.
- [minor] gcsfs 2026.x swaps `GCSFileSystem` (and fsspec's `gcs`) for the
  experimental gRPC-backed `ExtendedGcsFileSystem` unless
  GCSFS_EXPERIMENTAL_ZB_HNS_SUPPORT=false. Defaulted (operators can still
  opt in) before gcsfs is imported; the GCS connector loads the module too.

LLM:
- [major] The OpenAI-compatible adapter never stripped GPT-6 sampling
  params. `gpt-6` joins the reasoning-model detector (which also now accepts
  `.` so `gpt-6.1-sol` and `gpt-5.6-*` match), and validate() returns
  through `_strip_deprecated_sampling_params` for unrecognised aliases.

Tests:
- [minor] Azure GPT-6 is now tested through a real LLM on all four paths
  (complete, complete_vision, stream_complete, acomplete) with LiteLLM
  captured, replacing the helper that hand-copied LLM.__init__. Both
  mutations from the review (call sites bypassing `_revalidate`; per-call
  kwargs applied after `cost_model`) now fail.
- [minor] Fresh-interpreter wiring tests: importing only the sdk1 storage
  helper, or only the MinIO / GCS connector, applies the settings. Wire
  tests through FileStorage -> s3fs -> aiobotocore on an HTTPS endpoint pin
  plain uploads and Content-MD5 on DeleteObjects.

Verified live against MinIO 2024-12-18 (chart) and 2026-09-22 (OSS): sdk1
and connector writes, reads and every delete path succeed.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* UN-4224 [FIX] Install s3fs and gcsfs in the sdk1 test group

CI's unit-sdk1 group syncs only the `test` dependency group (no extras),
so `tests/test_storage_compat.py` failed to import aiobotocore. Its wire
tests drive FileStorage -> s3fs -> aiobotocore, and its wiring tests check
gcsfs's class selection; at runtime both come from the `aws` / `gcs`
extras. Add them to the test group at the extras' versions, as was done
for boto3.

Reproduced CI's environment locally (`uv sync --group test`, no extras):
760 passed, 2 deselected (slow/integration). Lockfiles relocked; the
diff is the dependency-group metadata only, no package version changes.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: ali <117142933+muhammad-ali-e@users.noreply.github.com>

* UN-4224 [FIX] Fix GCS connector test connection on gcsfs 2026.x

Since #2310 moved gcsfs from 2024.10 to 2026.10, test connection fails
for every Google Cloud Storage connector with
`HttpError: Required parameter: project, 400`. This is the rc.425
regression behind the staging "GCS ETL Pipeline" UI test (Submit stays
disabled because test connection fails).

`test_credentials()` called `info("/")`. gcsfs 2024.10 turned that into
`GET b?project=<project>` (list buckets); gcsfs 2026.10 sends `GET b/`
with no project, even when one is configured, and GCS rejects it. Use
`ls("")`, which on 2026.10 sends exactly the old `GET b?project=...`
request -- same permission, same behaviour. It is the only root-level
`info` call on gcsfs in either repo.

Verified against real GCS (project unstract-staging) with gcsfs 2026.10:
the connector's test_credentials() now returns True; the rc.425 code
fails with the same 400 seen in staging. A regression test records the
gcsfs request and asserts a bucket listing that carries the project; it
fails if `info("/")` comes back.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* UN-4224 [FIX] Make the GCS connector test independent of import order

Review (Greptile): the test imported `gcsfs.GCSFileSystem`, which resolves
to the experimental ExtendedGcsFileSystem when gcsfs is imported before
storage_compat sets GCSFS_EXPERIMENTAL_ZB_HNS_SUPPORT=false -- e.g. when
this file runs alone -- so it could exercise a different client from
production. Import `gcsfs.core.GCSFileSystem`, the class production runs,
and pin it with a test. Passes on its own with the env var unset and even
with the experimental class forced on.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…res legible (#2322)

The cap default was a placeholder (20) that became the effective limit in
every environment because it was never configured. Hari's call on the
standup: 300, with clear errors as the companion work.

- DEFAULT_PAGE_CAP 20 -> 300 (the agreed product limit for single-call
  image answering).
- DEFAULT_MAX_TOTAL_BYTES 50MB -> 14MB. 50MB raw is ~67MB base64, larger
  than EVERY provider request limit (Bedrock/Gemini-inline 20MB, Google
  Cloud 30MB, Anthropic 32MB), so the provider rejected the request before
  our own budget check could fire. 14MB raw fits the strictest.
- Reject an over-cap PDF BEFORE submitting to pdf-to-images, which bills
  per converted page. Previously the cap was only checked at answer time,
  so a 164-page document was converted and billed in full before being
  refused (reported by Johny).
- Reword both errors: name the platform knob and the fact that image mode
  cannot window, instead of pointing at the adapter's 'pages to extract'
  setting — which is what sent a tester hunting through a field they had
  never set.
- configured_page_cap() in sdk1 is the single source of truth for the cap,
  read by both the extraction pre-check and the cloud consumer plugin, so
  the two checks cannot disagree.
- New IMAGE_REQUEST_TOO_LARGE bridge code for the band the cap cannot
  cover: a model's own per-request limit can be lower than 300 (e.g. 100
  images on a 200K-context model).

Tests: sdk1 772 pass (incl. 8 new: env accessor, reworded messages, and
the pre-submit cap with its billed-call assertion), bridge 27 pass.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

…indexing); fix re-extraction of names with [ ] (#2324)

* UN-2646 [FIX] Image mode: fail oversized documents at indexing; fix re-extraction of names with [ ]

Two image-mode extraction fixes reported from staging.

1. Byte budget enforced at extraction, not only at prompt time (Gopal).
   A 90-page document indexed successfully, then failed only when a prompt
   ran ('more than 14 MB'), after the user had built prompts around it.
   verify_page_bytes() now checks the converted page set against the
   budget right after download, before anything is persisted, so the
   document fails at indexing. The conversion itself is still billed —
   image sizes are unknowable until pages are rendered — and the error
   says so. The answer-time check stays as the backstop, and both read
   the new configured_max_total_bytes(), so they agree by construction.

2. Re-extraction failed on GCS for documents named with [ ] * ? (Vikash).
   'Failed to clear previous page images ... provider=gcs' on a document
   named '... [CDIC] ...'. The page directory is named after the document,
   and fs.rm(dir, recursive=True) hands it to fsspec, which expands it as a
   glob on gcsfs/s3fs: '[CDIC]' became a character class, matched nothing,
   and raised FileNotFoundError. LocalFileSystem uses a plain rmtree and
   never globs, which is why this was invisible locally. The best-effort
   cleanup after a failed write had the same flaw and only logged it, so it
   silently left partial pages behind.

   New FileStorage.rm_exact(): find() + rm_file() per object, then
   directories deepest-first. Verified against gcsfs 2026.10.0 and s3fs
   2026.9.0 source that rm_file is a single delete-by-exact-key on both.
   Both page-store delete sites use it.

Tests (sdk1 791 pass, 15 new):
- ObjectStoreLikeMemoryFS models gcsfs/s3fs delete semantics exactly
  (globbing rm, exact-key rm_file, detail=False ls); plain
  MemoryFileSystem inherits a globbing rm_file and the wrong ls default,
  so it would model the wrong backend.
- Re-extraction into a '[CDIC]' directory, partial-write cleanup, and an
  extraction-to-answer round trip through the real loader. The first two
  FAIL on the previous code, with exactly the production error.
- rm_exact on LOCAL and object-store semantics, for [ ] * ? and plain
  names, nested trees, and a single file (a literal match only, never the
  glob's matches).
- Existing reset-invariant tests now fault-inject rm_exact, the method
  production actually calls.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* UN-2646 [FIX] Address review: loader reads configured limits; rm_exact surfaces real rmdir errors

- load_page_images now defaults page_cap and max_total_bytes to the
  configured values (env override or default) instead of fixed constants,
  so the answer-time check always agrees with the extraction-time checks.
  Explicit values and None still win.
- rm_exact tolerates only FileNotFoundError from rmdir (a prefix that
  vanished with its last object). Permission and not-empty errors now
  reach the caller instead of being reported as a successful delete.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* UN-2646 [FIX] Plain-language image-mode limit errors; pin rm's wrong-folder delete

- Rewrite the four image-mode limit errors (page cap and byte budget, at
  indexing and at answer time) in user terms: what is wrong, the numbers,
  what to do. Drop the operator-only env var names and the "however many
  pages it has" phrasing. Cap errors keep the note that "Pages to extract"
  does not apply in image mode.
- Add tests for the second failure mode of plain rm() on object storage,
  seen against MinIO and a GCS emulator: when another document's folder
  matches the name as a glob, rm() deletes that folder and leaves the
  target in place. rm_exact leaves it alone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* UN-2646 [MISC] Keep one call inside pytest.raises in the partial-write cleanup test

Addresses SonarCloud python:S5778.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* UN-2646 [FEAT] Reject image-mode PDFs over the page cap at upload

When the project's default profile is LLMWhisperer in image mode,
Prompt Studio's upload now counts each PDF's pages before storing
anything and rejects one over the page cap (400) with a plain message.
Image mode can never answer such a document, so the user learns at
upload instead of at indexing.

- Uses the SDK's page counter and configured_page_cap(), so a document
  accepted at upload also passes the extraction-time pre-check.
- All files in a request are checked before any is stored.
- Text modes, other x2text adapters, non-PDFs and unreadable PDFs pass
  through; extraction still checks them.
- Extracts _is_image_output_mode, now shared with the PDF-only guard.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* UN-2646 [FIX] Address code review: exact deletes for document files; safer upload check

- Prompt Studio document delete no longer globs the document's name.
  On GCS/S3, rm("Report [Final].pdf") missed itself and deleted
  "Report F.pdf" instead, and the related-files glob matched the other
  document's extract files (both seen on MinIO). Use rm_exact for the
  source and related files and the extract-text cleanup, and escape the
  name in the related-files glob.
- Upload check: decide image mode once per request and fail open if the
  default profile's adapter config can't be read (e.g. an old encryption
  key) so uploads keep working; sniff each file's type once.
- Count pages through a public SDK helper that reads the upload stream in
  place instead of copying the whole file into memory.
- Use LLMWHISPERER_ADAPTER_PREFIX instead of a second literal.
- rm_exact tolerates an object already gone between listing and delete.
- Correct the comment that claimed multi-file uploads are all-or-nothing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* UN-2646 [FIX] Delete a failed upload's partial file by its exact name

write_streaming's cleanup used fs.rm, which expands the uploaded
document's name as a glob on GCS/S3: for "Report [Final].pdf" it missed
the partial file or deleted "Report F.pdf" instead. Use rm_file, an
exact delete on every fsspec backend.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

* UN-2896 [MISC] Deprecate and remove the LLMWhisperer V1 adapter

Adds a single DEPRECATED_ADAPTERS registry that drives every guard, seeded
with LLMWhisperer V1, and deletes the V1 adapter package it retires.

Guards (adapter-type agnostic, so future deprecations are a one-line entry):
- excluded from supported_adapters, so it cannot be picked for creation
- POST /adapter/ and /test_adapters/ reject a deprecated adapter_id
- profile manager rejects pointing a profile at one; existing profiles on a
  deprecated adapter stay editable in their other fields
- platform-service rejects execution off the is_available column, which is
  every SDK adapter lookup's single choke point

Backfill migration marks existing V1 instances unavailable across all orgs
(0003 used .first(), which marked only one row per adapter).

Removes the V1 package, its icon, its dead env vars (POLL_INTERVAL,
MAX_POLLS, STATUS_RETRIES -- V2 uses WAIT_TIMEOUT/MAX_RETRIES/RETRY_*) and
the workflow-execution plumbing that forwarded them into tool containers.

Claude-Session: https://claude.ai/code/session_01DXuiGyUwXyU1EVQBeMHppe

* UN-2896 [FIX] Self-review: close the remaining deprecated-adapter paths

- platform-service: the adapter_instance route's blanket `except Exception`
  re-wrapped every APIError as a 500, so both the new deprecation error and
  the pre-existing "not found" reported as server errors and logged a
  traceback. Re-raise APIError untouched, as the neighbouring route does.
- Move the adapter_id check into AdapterInstanceSerializer.validate:
  adapter_id is writable, so update/partial_update could set a deprecated id
  that create rejected.
- Default profile creation and the project-import warning gated on is_usable
  alone, letting a deprecated default land in a new profile without passing
  through the serializer.
- set_default_triad accepted a deprecated adapter as a user default straight
  from the API.
- New is_adapter_selectable() states the rule once: usable, available, and
  not deprecated.
- DefaultTriad: disable deprecated options instead of dropping them. Filtering
  emptied adapterList for an org whose adapters are all deprecated, which
  gated the effect that loads the current defaults, and left the current
  default rendering as a bare UUID.

Claude-Session: https://claude.ai/code/session_01DXuiGyUwXyU1EVQBeMHppe

* UN-2896 [FIX] Silence Sonar S117 on the migration's get_model calls

Matches the repo's existing convention for this rule in data migrations.

* UN-2896 [FIX] Don't re-validate unchanged defaults in set_default_triad

The Default Triad UI submits all four defaults on every save, so validating
every submitted value locked a user out of changing any default once one of
theirs had been deprecated underneath them. Skip unchanged values, matching
ProfileManagerSerializer.validate, and cover it with a regression test.

Also drops the duplicated adapter name from the deprecation message and
promotes the inline logger in serializers to module level.

* UN-2896 [MISC] Correct the V2 adapter's env var docs

The README still listed ADAPTER_LLMW_POLL_INTERVAL and ADAPTER_LLMW_MAX_POLLS,
which only V1 ever read, and the WhispererEnv docstring quoted a 300s default
where the code uses 900. Document the four vars V2 actually resolves.

* UN-2896 [MISC] Revert the V2 env var doc edits

Out of scope: V2's own stale env table is pre-existing debt in a V2 file,
not V1 residue this PR is removing. Worth a separate tidy.

* UN-2896 [TEST] Cover the deprecated-adapter execution path

The 400-not-500 behaviour for an adapter already wired into a project had
no test; platform-service only had middleware and route coverage.

* UN-2896 [MISC] Drop the set_default_triad deprecation guard

Beyond the ticket's scope: the UI already disables a deprecated option, and
running one returns a clean 400 from platform-service. Restores the function
to its original form, removing the re-validation that locked a user out of
changing any default once one of theirs had been deprecated.

---------

Co-authored-by: harini-venkataraman <115449948+harini-venkataraman@users.noreply.github.com>

…l-not-found hint (#2325)

* UN-4248 [FIX] Render LLM provider errors as a readable line with status, class and a model-not-found hint

* UN-4248 [FIX] Self-review: cover litellm's other provider error shapes and never raise from the formatter

* UN-4248 [FIX] Self-review: no HTTP status without a response, multi-word provider tags, single period before the model hint

* UN-4248 [FIX] Address SonarCloud: string-based tag and hint stripping, pytest.raises in tests

…s restart (#2319)

* UN-4238 [FIX] Keep Prompt Studio results reaching the UI after a Redis restart

python-socketio 5.9.0's KombuManager listener only retried on OSError and
KombuError. A Redis restart raises redis.exceptions.ConnectionError, which is
neither, so the listener thread in each backend process died for good: the
pod kept accepting browser sockets but never delivered another pub/sub
event, and Prompt Studio results showed only after a manual refresh.

Upgrade to 5.17.0 (python-engineio 4.13.1 -> 4.14.0). Since 5.11.0 the
listener thread restarts itself, and since 5.16.3 _listen() retries on any
exception with backoff. A regression test pins that behaviour.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* UN-4238 [FIX] Give each gunicorn worker its own Socket.IO pub/sub host_id

python-socketio 5.11+ drops pub/sub messages carrying its own host_id,
assuming the publisher already delivered them locally. The backend runs
gunicorn with --preload, so log_events.py builds the Socket.IO server once
in the master and every forked worker inherited the same host_id: a Prompt
Studio result emitted by one worker was silently discarded by the worker
holding the browser's socket, leaving the prompt card stuck until refresh.

Regenerate host_id in each forked child via os.register_at_fork. Works with
or without --preload; a test forks and asserts the child's id differs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* UN-4238 [FIX] Harden the fork test and satisfy D403 under the pinned ruff

The forked child now runs inside try/finally os._exit, so an error there
can never unwind into the child's copy of pytest, and the parent asserts
the child's exit status. The docstring's first line is reworded so it is
capitalised (D403 under ruff 0.3.4).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

…gs (#2321)

* fix: remove eval() on DB error messages and exec() in settings

ExceptionHelper.extract_byte_exception ran eval(str(e)) on exceptions
from the MSSQL and MySQL connectors. That text comes from the remote,
user-configured database server, so untrusted input reached eval. Read
the details from e.args instead, and fall back to str(e) for any other
shape rather than raising inside the exception handler.

Replace the exec() loop that set four SOCIAL_AUTH_* settings in
backend/settings/base.py with explicit assignments.

UN-4239

* fix: unwrap single-tuple driver args and harden the no-eval test

Address self-review on #2321:
- extract_byte_exception now also handles an exception whose single
  argument is a (code, details) tuple, which the old eval-based code
  accepted (pymssql raises this shape from connect()).
- test_message_is_not_evaluated used a payload that could never run
  inside the helper, so it missed eval reintroduced behind a fallback.
  Use a payload with an observable side effect instead.

UN-4239

…eam over Redis (#2316)

* UN-4077 [MISC] Stop requiring Celery broker credentials when logs stream over Redis

Nothing connects to RabbitMQ once LOG_TRANSPORT=redis, yet the backend
settings and the tool sidecar refused to start without CELERY_BROKER_*,
blocking removal of the broker credentials from the Helm chart.

- Backend: read the broker vars optionally; CELERY_BROKER_URL is built
  exactly as before when the base URL is set, else "" (Celery connects lazily).
- Tool sidecar: require the broker vars only on the Celery log transport,
  via unstract-core's now-public use_redis_log_transport().
- Runner: forward the broker vars to the sidecar only when set.
- run-worker.sh: CELERY_BROKER_BASE_URL is no longer a required var.

LOG_TRANSPORT semantics and its "celery" default are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* UN-4077 Keep the broker required wherever the Celery log transport needs it

- Backend: broker settings stay required on the Celery log transport and are
  optional on Redis; a set base URL still needs its credentials. The rules live
  in backend/celery_broker.py so they are unit-tested without Django.
- run-worker.sh: CELERY_BROKER_BASE_URL is required unless every requested
  worker is a PG-queue member.
- WorkerConfig: no missing-broker error on the Redis log transport.
- Sidecar test: each broker setting is checked as missing on its own.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* UN-4077 Require the broker for PG-only workers on Celery logs and test the settings wiring

- run-worker.sh: CELERY_BROKER_BASE_URL is required when any non-PG worker is
  requested or LOG_TRANSPORT is not "redis"; the help line states the rule.
- Backend settings: the broker requirement and reads now run after the .env
  load, so a .env-only LOG_TRANSPORT=redis is honoured.
- Tests import the real settings module in a subprocess to pin what it
  enforces, and the URL cases assert literal expectations.
- celery_broker docstring no longer claims the module imports in isolation.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* UN-4077 Require no broker settings on the Redis log transport

- Backend: on the Redis log transport no CELERY_BROKER_* setting is required,
  even when a base URL is configured without its credentials; the Celery
  transport still requires all three.
- WorkerConfig: restore main's broker validation and drop its test, since the
  skip also silenced the warning for Celery workers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

…ger deadlock on httpcore's pool lock (#2318)

* UN-4237 [FIX] Close streamed LLM responses so garbage collection cannot deadlock executor workers on httpcore's pool lock

LiteLLM's sync CustomStreamWrapper has no close() and stops iterating
before the HTTP body ends, so every streamed completion left its response
checked out of httpcore's pool until the garbage collector finalised it.
When that happened on a thread inside ConnectionPool.handle_request, the
finaliser's PoolByteStream.close() waited on the same non-reentrant lock
the thread held, and the worker hung forever with its lease still renewing.

Add close_stream(), which closes the stream and the provider response
behind it (litellm's ModelResponseIterator, the OpenAI SDK Stream, plain
generators), and call it in a finally in collect_with_retry and
iter_with_retry: on success, on error and on early exit. The retry backoff
now runs after the stream is closed.

Tests run the real LLM.complete() path against a local Anthropic SSE
server with GC disabled, and force a collection inside the pool lock to
reproduce the deadlock; they fail against the previous code.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VQoJFFiRZyDKyu14pg5hDb

* UN-4237 Close the Responses-API stream too, and warn when a close fails

Addresses the standardized review on #2318.

- close_stream now walks the stream's handle attributes to a bounded depth
  with a visited set, instead of one fixed level. On litellm's Responses-API
  bridge (responses/<model> and the models routed there) the HTTP response
  sits under an iterator with no close(), one level below where the old walk
  stopped, so an error after content or an early close left the pool slot
  checked out.
- A close that raises is logged at WARNING with the traceback: httpcore marks
  its stream closed before closing the socket, so that failure can hold the
  slot for good.
- Comments and docstrings no longer claim an abandoned generator is covered:
  only a caller that closes it is. stream_complete's docstring says callers
  that stop early must close it.
- Tests: socket tests for the Responses-API bridge (error after content,
  early close, drain), and unit tests for a generator completion_stream
  (Bedrock), an OpenAI-SDK-shaped Stream, the Responses chain, cycles, and
  the warning. Each was checked against a mutation of the fix.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VQoJFFiRZyDKyu14pg5hDb

* UN-4237 Keep close_stream from raising when reading close itself fails

Addresses Greptile's review on #2318: the close attribute was read outside
the try, so a handle whose close property raised would stop the cleanup
before the remaining handles and could replace the original stream error in
the caller's finally. The lookup now sits inside the guarded block, with a
test that fails without it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VQoJFFiRZyDKyu14pg5hDb

* UN-4237 Address review: traceable close-failure warning, version-agnostic comments

From Chandru's review on #2318:

- A failed close logs only its own traceback: close_stream runs in a finally
  while the provider error is propagating, so Python chained that error onto
  the close failure and exc_info printed it a second time.
- close_stream takes the caller's logger and adapter description, and both
  retry helpers pass theirs, so a leaked pool slot can be traced to the
  provider that leaked it.
- Comments: the explanation lives once, in close_stream's docstring (the only
  ticket reference left); the rest point at it. Dropped litellm version pins
  and "before the fix" wording, and described the Responses-API chain the
  same way everywhere (wrapper -> completion_stream -> streaming_response ->
  response).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VQoJFFiRZyDKyu14pg5hDb

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: ali <117142933+muhammad-ali-e@users.noreply.github.com>

chore: remove leftover debug console.log calls in Prompt Studio

DocumentManager logged file blobs, selected document and fetch metadata on
every render/fetch, and ImportTool logged dropped files. These end up in
users' consoles and in PostHog console-log capture.


Claude-Session: https://claude.ai/code/session_01K1H42KtrPNCMvAZxYmt1r1

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

…ne path (#2239)

* UN-3057 [FIX] Repair Prompt Studio projects left ownerless by the clone path

Since UN-2202, `_is_resource_owner` consults only ResourceMembership OWNER
rows for resources that expose `memberships` (CustomTool does) and no longer
falls back to `created_by`. The Prompt Studio clone path never created that
row, so every project cloned after 0009_absorb_shared_users ran has no owner
at all: still visible (the clone copies the parent's `shared_to_org`) and
profiles can still be created, but `IsParentToolOwner` denies every mutation
on them — deleting an LLM profile returned 403.

The clone helper itself is fixed in unstract-cloud; that stops new breakage
but cannot help rows already written. This adds the repair:

- `repair_ownerless_owner_rows()` grants `created_by` an OWNER row on
  resources that have zero OWNER rows. Only ownerless resources are touched,
  so a creator deliberately replaced by a co-owner is not resurrected, and a
  null creator or null organization is skipped. Idempotent.
- It iterates `_base_manager`: several resources' default manager is
  org-scoped by `UserContext`, which is unset during a migration and would
  silently filter every row out. Same guard `tenant_account_v2.signals` uses.
- Migration 0011 applies it to CustomTool; reverses to a no-op.

Regression tests pin all three branches (repair, leave-alone, skip). Verified
red-green: with the helper body stubbed to a no-op the behaviour test fails
and the two guard tests still pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* UN-3057 [FIX] Promote an existing non-owner creator row to OWNER in the ownerless backfill

Membership is unique per (user, resource), so get_or_create returned a creator's existing VIEWER row unchanged and left the project ownerless. update_or_create promotes it; only projects with zero OWNER rows reach this path. The repaired count now includes promoted rows.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* UN-3057 [FIX] Re-point platform-key owners after the ownerless repair, and keep the migration comments generic

Review follow-up on #2239:

- Migration 0011 now calls `repair_platform_key_ownership` after the
  ownerless repair, so an OWNER row written for a platform key's service
  account is re-pointed to the key's creator (service accounts are filtered
  out of every owner surface). It depends on `tenant_account_v2.0005` and
  `platform_api.0004` rather than `tenant_account_v2.0006`, so a backport to
  a release that predates 0006 cannot leave an inconsistent history.
- The `_base_manager` comment now states what holds under `migrate`
  (historical models' managers are unscoped) instead of the request-time
  org-scoping, which does not apply there.
- Module docstrings describe what the migration and test do; the ticket
  history lives in the PR.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Update backend/prompt_studio/prompt_studio_core_v2/migrations/0011_repair_ownerless_custom_tools.py

Signed-off-by: Chandrasekharan M <117059509+chandrasekharan-zipstack@users.noreply.github.com>

---------

Signed-off-by: Chandrasekharan M <117059509+chandrasekharan-zipstack@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Chandrasekharan M <117059509+chandrasekharan-zipstack@users.noreply.github.com>

… error message (#2331)

* UN-4069 [FIX] Raise upload limit on prompt_output to 10 MB with clear error

Agentic table extraction output with highlight coordinates can exceed
Django's default 2.5 MB DATA_UPLOAD_MAX_MEMORY_SIZE, causing a generic
500 that retries uselessly. Use request.read() to bypass the global
limit on this view only, enforce a 10 MB cap, and return 413 with an
explicit size message so the error is actionable in the UI.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* UN-4069 [FIX] Update prompt_output tests for request.read() change

The view now uses request.read() instead of request.body, so mock
requests need read.return_value and META set alongside body.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

…#2327)

* feat: capture adapter test-connection and save results in PostHog

The existing adapter events fire on click (intent_add_*, test_connection_*,
intent_success_add_* on the Submit click), so a failed add and an abandoned
one look the same in funnels. Add adapter_test_connection_result and
adapter_save_result with result success/invalid/error. The error text is
sent only from SaaS prod, because it can echo customer endpoints.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K1H42KtrPNCMvAZxYmt1r1

* fix: test the SaaS-only gate on adapter error text

Move the gate into saasOnlyErrorProps so it can be tested without
rendering ConfigureDs, and send only string alert content: subscription
errors carry an array that would stringify to [object Object].

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K1H42KtrPNCMvAZxYmt1r1

* feat: capture Prompt Studio run results in PostHog

ps_prompt_run only records the click; ps_prompt_run_result records the
outcome (status, operation, prompt count, elapsed seconds) so a stall
after a few runs can be split into failing runs vs abandoned ones.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K1H42KtrPNCMvAZxYmt1r1

* fix: send task_id with ps_prompt_run_result for deduping across tabs

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K1H42KtrPNCMvAZxYmt1r1

* docs: trim PostHog event comments

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

* UN-1924 [FIX] Reject unsupported files by sniffing MIME in API storage staging

API-deployment uploads were gated on the multipart Content-Type, which the
caller supplies and nothing verifies, with a fallback to
application/octet-stream that is itself in AllowedFileTypes. Any file passed
that check, reached the bucket, and failed at extraction with an error that
did not name the cause.

Detect the type from the file's own bytes with libmagic before writing
anything, matching the filesystem source path, and report a rejected file as
a failed entry in the API response instead of staging it under a placeholder
hash that later surfaced as an empty-file error.

* UN-1924 [FIX] Terminalise an API execution when every file is rejected

Rejecting files at staging means the dispatch set can now be empty, which
reached a path that was previously unreachable: the API worker's
_unified_api_execution short-circuits an empty file set and returns
status COMPLETED without ever writing that status back, so the row kept the
status it was dispatched with and the caller polled a PENDING execution
forever.

Skip the dispatch entirely when staging yields nothing, marking the execution
COMPLETED and returning the per-file rejection entries, and make the worker's
own short-circuit persist the status so an empty set from any other caller
cannot strand an execution either.

* UN-1924 [FIX] Address review findings on unsupported-file rejection

Resolving an 8 KiB sample alone rejected every legacy Office upload larger
than the window: libmagic reads .doc/.xls/.ppt through the OLE2 directory
sector at the end of the file, so the sample only ever showed the container
(application/x-ole-storage), which is not allow-listed. Reproduced on a
710 KB .doc and a 1.2 MB .xls, on the API path and the UI execute endpoint
alike. Container samples now escalate to a full-file classification, using
the upload's temp path when Django has spilled it to disk.

Also from review:

- Isolate the terminal status write on the all-rejected path so the rate
  limit slot and staging dir are released even if it raises, matching the
  staging-failure path above it.
- Report the execution's stored status instead of asserting COMPLETED; the
  row can be missing or the terminal guard can refuse the change, and
  claiming success only hides a stranded execution behind a 200.
- Write total_files/failed_files alongside the status, since a terminal row
  with a NULL failed_files reads as a clean success to is_failure_run and to
  run history.
- Distinguish an empty dispatch from a total conversion failure in the
  worker: convert_file_hash_data swallows per-file errors and returns {} for
  both, so the second was being reported as a zero-file success.
- Scope the guard comment on update_execution_completed to the PG transport;
  the legacy path applies the status unconditionally.

Tests: pin the short-circuit to the staging result rather than the upload
list (the previous test passed with the original bug reintroduced), cover
the cleanup-on-DB-error path, the container escalation, and the empty-upload
branch. Mutation-checked: reverting the escalation, gating the short-circuit
on file_objs, and dropping the cleanup isolation each fail the suite.

* UN-1924 [FIX] Release API rate limit slots by org id, not model instance

release_slot formats its argument into the Redis key, and acquire_slot built
that key from str(organization.organization_id). Passing the Organization
instance produced a different key, so the ZREM removed a non-member: it
returns 0 and raises nothing, leaving the slot held for the full TTL and
throttling every other API-deployment call for that org.

All three call sites in this module were affected, including the one added
for the all-rejected path. The two correct call sites in the codebase
(undispatched_sweep.py, models/execution.py) already pass the id string, and
the former carries a comment describing this exact trap.

The same instance-instead-of-id call remains in api_deployment_views.py and
in two places in mcp_server/tools/execution.py; those are outside this
change's surface and are left for a separate fix.

* UN-1924 [FIX] Fail one undetectable upload instead of the whole request

MIME detection reads the upload, so a broken stream raises inside the staging
loop and aborts every remaining file in the request. Rejection is already
per-file for an unsupported type; an unreadable one now behaves the same way.

The message says detection failed rather than naming a type, since an I/O
fault and an unsupported format need different follow-ups.

* UN-1924 [FIX] Acknowledge results and notify subscribers on an all-rejected run

The all-rejected early return reaches a terminal status without going through
WorkflowHelper, so it skipped two things the dispatch path does.

It serves the per-file results in its own response but never marked them
consumed, so a follow-up GET /status served them a second time with 200 where
the contract is 406. The synchronous dispatch path acknowledges at exactly
this point.

It also never reached PipelineUtils.update_pipeline_status, the only
dispatcher of API deployment notifications, so an all-rejected request
alerted nobody where a dispatched-and-failed run would have.

Both are wrapped so a failing webhook cannot turn a handled rejection into a
500, and the response is unchanged.

set_result_acknowledge loses its underscore: it is now called from another
module, so it is part of the contract rather than an internal detail.

* UN-1924 [FIX] Keep acknowledgement and notification independent

Sharing one try block meant a failed acknowledgement returned before the
notification ran, so an all-rejected execution could reach a terminal state
without alerting API deployment subscribers. They are independent
obligations; neither now depends on the other succeeding.

* UN-1924 [MISC] Drop formatter churn on two untouched assertions

Reformatted by a newer ruff than the v0.3.4 the pre-commit config pins, in
the opposite direction to how the file already reads. Restores both to the
committed formatting so the diff is additive.

* UN-1924 [FIX] Mirror LLMWhisperer's file type gate

The check exists so a file Unstract accepts is one LLMWhisperer can extract.
Any divergence is a bug in one of two directions: accept something it cannot
read and the failure just moves downstream to a 415, or reject something it
can read and a working file is lost.

Reconciled against the gate itself — Util.is_valid_file_type_from_path in
unstract-llm-whisperer, not the public docs page, which lists formats the gate
does not name and omits how it actually decides.

- Any text/* is accepted. That is how html, xml, tsv, rtf and markdown reach
  the extractor: as text, not as listed formats. They were being rejected here
  despite LLMWhisperer handling them.
- Dropped application/octet-stream. Nothing libmagic cannot name is extractable,
  and this entry is why a zip renamed .pdf reached LLMWhisperer and came back
  415 — the symptom this ticket was raised for.
- Dropped the macro-enabled xlsx type, which LLMWhisperer's gate does not list.

The two enums now match member for member, 20 MIME types. An empty upload has
no type to judge, so detection returns None and the gate is skipped rather than
passed through a placeholder.

LLMWhisperer additionally re-identifies octet-stream and zip with Magika and
falls back to scanning for a %PDF- header. We have neither, so this gate is
marginally stricter than theirs for unidentifiable bytes.

* UN-1924 [FIX] Recognise a PDF that does not start at offset 0

libmagic's PDF rule matches only at offset 0, so eight bytes ahead of the
header are enough to make a real PDF sniff as application/octet-stream.
Dropping octet-stream from the allow-list therefore started rejecting
stream-wrapped PDFs — and this allow-list also gates the filesystem connector
and the worker, so those files would have begun failing in ETL and Task
pipelines that process them today.

LLMWhisperer scans the same 1 KB window for the same reason, after giving
octet-stream and zip a Magika second opinion first. Without Magika this
covers the PDF case, which is the one that reaches us; anything else libmagic
cannot name is still rejected.

Applied at all three gates that share the list: API staging, the filesystem
source, and the worker's own check.

Also: the worker's two new terminal branches wrote the execution status but
never updated the pipeline, so a run that ended there left the pipeline's last
run stale and alerted no subscriber. They now notify the way the sibling
empty-files branch does.

* UN-1924 [FIX] Harden rejection reporting and the PDF rescue

Three faults in the staging loop, all raised in review.

The PDF rescue promoted on a bare `%PDF-` substring anywhere in the first
kilobyte, so any unidentifiable blob carrying that text near its start could be
staged as a PDF — a way past the gate, in a check whose whole purpose is to
close one. It now leaves a recognised zip alone and re-classifies from the
marker's own offset, so the promotion has to be corroborated rather than
guessed. Stricter than LLMWhisperer's equivalent scan, which is the safe
direction for something that moves a file into the allow-list.

Detection caught `Exception`, which turned any systemic fault — a broken
libmagic database, an unreadable upload temp dir — into a per-file rejection.
Every file in every request would fail that way, and the request still answers
200 COMPLETED, so a platform outage would read to callers as "your files are
invalid" and to metrics as success. Now narrowed to the faults that really are
about this upload's bytes; anything else propagates and fails the request.

Reporting a rejection wrote to the result cache inside the per-file loop
through an unguarded pipeline execute, so a transient Redis fault while
rejecting one bad file would fail the whole execution and discard the good
files already staged — contradicting the invariant the suite claims in
test_supported_files_survive_a_rejected_sibling. Both call sites now go
through one reporter that degrades to a log line.

Tests cover all three, plus the disk-backed detection branch, which no test
reached: SimpleUploadedFile has no temporary_file_path, so every existing
container test exercised only the in-memory path — while real legacy Office
uploads, the files that branch exists for, are usually over the 2.5 MB
threshold and take the other one.

* UN-1924 [FIX] Require a PDF version header before rescuing a file

Re-classifying from the marker's offset was not the safeguard it looked
like: libmagic's PDF rule is only the magic bytes, so "%PDF- not a pdf"
still came back application/pdf and the file was still promoted into the
allow-list.

A real header carries a version, so require %PDF-<n>.<n> before treating the
marker as evidence. The offset re-classification stays as a second check.

The test that was supposed to cover this passed for the wrong reason - its
blob sniffed as image/x-tga and never reached the rescue at all. It now
asserts the bytes are octet-stream first, so it fails if it stops exercising
the branch.

* UN-1924 [FIX] Resolve zip containers, which libmagic will not name from a buffer

Measured on the shipped libmagic 5.46: magic.from_buffer never returns
application/zip at any sample size, not even given every byte of the file —
only magic.from_file does. So the zip half of the escalation set could never
match, and the value a zip container actually reports from a buffer,
application/octet-stream, was neither allow-listed nor escalated. A valid
.docx whose first member is not [Content_Types].xml — what re-zipping and
streaming writers produce — was therefore rejected at upload, having worked
on main under the octet-stream allowance.

octet-stream now counts as undecided like the named wrappers, one shared set
so the API and connector paths cannot disagree, and classification always
goes through a path: an in-memory upload is spilled to a temporary file
rather than classified from a buffer that cannot answer the question.

from_file still only says 'application/zip' for a repackaged OOXML file, so
the archive's own entries decide — the same markers libmagic's msooxml rule
looks for. That recognises the document without taking on an ML classifier;
it can only return types already in the allow-list, so it widens what is
recognised and never what is permitted.

Tests now feed real container bytes with libmagic unmocked. Every existing
container test scripted libmagic's answers, which is precisely why a detector
asking a question libmagic cannot answer looked green.

* UN-1924 [REFACTOR] Move the shared MIME gate logic into unstract/core

The backend and the workers were each carrying a verbatim copy of the
inconclusive-type set, the PDF rescue and the zip inspection, kept together
by a 'keep in step' comment — which the quality gate then failed at 8.2%
duplication on new code against a 3% limit.

unstract/core already hosts exactly this kind of cross-cutting predicate
(is_failure_run), so the logic lives there now and both packages import it.
The allow-list enums stay where they are; only the interpretation of
libmagic's answer is shared, which is the part that must not drift.

identify_zip_container takes the allow-check as an argument rather than
importing an enum, so the shared module stays free of either package.

* UN-1924 [FIX] Bound the ODF mimetype read so a zip bomb cannot expand through it

Looking inside an archive to name the document meant reading its mimetype
member whole. That member comes from an unvalidated upload, and read()
decompresses whatever the archive declares, so a compact bomb could expand
into backend memory during synchronous staging and fail the request before
anything was dispatched.

An ODF mimetype member holds one media type string, so anything larger is
not what is being looked for: the declared size is checked first and the read
is bounded regardless.

* UN-1924 [FIX] Stop an archive nominating its own file type

Reading the ODF mimetype member and accepting any allow-listed value it
named handed the decision to the upload: a zip declaring application/pdf
would have been staged as a PDF and dispatched, which is precisely the
deferred extraction failure this gate exists to prevent.

The member can identify which ODF document an archive is; it cannot choose a
format. Only the three ODF types are honoured now, and only when the archive
meets the conditions the spec requires of a real one — first entry, stored
uncompressed, and small. A member bolted onto an arbitrary zip meets none of
them.

identify_zip_container no longer takes the allow-check, because every value
it returns is now a fixed constant rather than a string from the archive.

* UN-1924 [MISC] Leave one throwing call inside the raises block

Building the upload inside pytest.raises meant two calls could satisfy it,
so the test would still pass if constructing the file were what raised
rather than staging it. Sonar flags the shape (python:S5778) and it is the
same way a test passes for the wrong reason that bit two other cases here.

The upload is built outside the block, and the expected message is matched,
so only the staging call can satisfy it. Verified by mutation: widening the
narrowed except back to Exception fails this test with DID NOT RAISE.

* UN-1924 [MISC] Pin the zip-carrying-a-PDF-marker case

An entry stored uncompressed puts %PDF-1.7 in the archive's literal leading
bytes, so a scan of the head sees it exactly as it would in a real document.
Nothing pinned that the wrapper is resolved before the marker is consulted,
which is the order that keeps it from becoming a way past the gate.

Worth having as a test rather than a reasoned argument: checking the helper
on its own suggests the marker wins, and only the real entry point shows the
escalation to from_file naming the archive first.

* UN-1924 [FIX] Three small inconsistencies in the new terminal paths

The worker's COMPLETED arm wrote total_files=0 and left the other two
counters NULL — the exact shape update_execution_completed exists to avoid,
since a terminal row with a NULL failed_files reads as a clean success.
Nothing ran, so both are now written as zero.

Its ERROR arm named the reason under "message" while the function's other
two ERROR returns use "error", so a consumer reading .get("error") to learn
why an execution failed got None for this variant alone.

When the all-rejected status write raises, the response carried no error at
all, unlike the sibling staging-failure path — a bare 422 with no reason,
while the row is still PENDING so a follow-up GET /status says something
else. The swallowed exception is now reported.

Also corrects a comment that claimed a field-scoped save() is safe against
concurrent writers: it is, for the columns, but save() re-runs
_handle_execution_cache() regardless of update_fields, which another comment
in the same file already explains. No live bug at this call site — the row is
freshly created and undispatched — but the comment said otherwise.

* UN-1924 [FIX] Count and keep files rejected before dispatch

A partially-rejected run reported as a clean success: rejected files are not
dispatched, so the worker's batch aggregate never saw them, and a run that
turned away a file still finished failed_files=0. That reads as success to
is_failure_run, so subscribers who asked to hear about failures heard
nothing, and the only trace was a cache entry deleted on the first status
poll.

A rejected file now gets a terminal ERROR file execution carrying its real
MIME type and the reason. The execution serializer already derives
successful/failed by counting those rows, so run history and the UI become
correct for free, and the rejection survives long enough to answer a support
question.

The stored counters are then floored at what the rows say when the worker
reports its aggregate. That only ever corrects upward, and only to a number
already true in the database, so runs with nothing rejected are untouched —
which is what lets this stay inside the backend rather than changing the
counter contract the ETL and Task pipelines share.

The row write is guarded: bookkeeping must not cost a caller the good files
staged alongside the bad one. Content is hashed for the row's identity, since
the API path has no file_path and two rejects would otherwise collide.

Mutation-checked: stubbing the row write fails both new tests.

* UN-1924 [FIX] Keep two rejections apart when the bytes are identical

The row identity was the content hash alone, so the same bytes uploaded
twice under different names folded into one row — recording one rejection
instead of two, under-counting failed_files, and losing a durable record the
caller was owed. The API path has no file_path, so this value is all that
separates them.

The name is folded into the identity. Deliberately no longer a plain content
hash: a rejected file was never processed and should not look to file
history as though it had been.

The test missed this because it used different bytes for the two files, so
it never exercised the collision. It now sends identical bytes under
different names, and fails on a content-only identity.

* UN-1924 [MISC] Drop a line-number cross-reference from a comment

The comment pointed at undispatched_sweep.py:245 to explain the trap, and
nothing keeps that honest — it was already off by two lines when written.
Both reviewers flagged it.

Says the constraint concisely instead: pass the id string, never the model
instance, because release_slot builds its key from what it is given.

* UN-1924 [MISC] Shorten the new code comments

Many of them read like review-thread replies: they recorded line numbers,
paths into other repos, libmagic versions and the reasoning behind earlier
attempts. Those details rot, and two were already wrong — a cited method name
that does not exist, and a version-specific measurement that does not hold on
a newer libmagic.

Each now says why the code does something unusual, in a line or two. The
reasoning itself is not lost; it is in the PR and the commit history, which
is where it stays true.

* UN-1924 [MISC] Clear the two Sonar findings

Cognitive complexity on _update_file_aggregates had gone to 17 against a
limit of 15, so the flooring moved into its own method. Reads better split
anyway: one method decides which counters are being written, the other
reconciles them with the file rows.

The composite assertion in the rejection test is split, so a failure says
which half went wrong.

Note the extracted signature annotates with a quoted "list[str]": the
enclosing viewset defines its own list() action, and a class-body annotation
resolves in the class namespace, so the bare form binds to that method and
raises on import. Caught by importing the module rather than parsing it.

* UN-1924 [FIX] Require relationships before naming a zip an Office document

Marker directories alone were enough, so an archive carrying
[Content_Types].xml and a word/ member was named a document without being
loadable. Greptile was right about this one, and my own live run had already
shown it: an archive of exactly that shape was accepted here and then
rejected by the extractor, which is the failure this gate exists to prevent.

A real package always carries _rels/.rels, so that is required now. Only
affects the repackaged shape, which is what this code resolves — with
[Content_Types].xml first, libmagic names the package and this never runs.

The test fixture was the incomplete shape, which is why it failed extraction
when deployed. It is a real package now, and the incomplete one is pinned as
rejected.

* UN-1924 [FIX] Let neither recognised wrapper be renamed by its payload

The rescue returned early for application/zip but not for
application/x-ole-storage, so an OLE2 container libmagic could not classify
further was promoted to a PDF by a %PDF- marker sitting in its first
kilobyte. Both are wrappers libmagic positively identified, and neither
should be overruled by its own contents.

Also fixes a CI-only failure of my own making: a guard assertion pinned
from_buffer to application/octet-stream, which is build-dependent — CI
reports application/zip for the same bytes. It now asserts the property the
test needs, that the archive is not named a document and so gets inspected.
That disagreement settles an earlier one on this PR: the reviewer's
application/zip measurement was right and my local one was the outlier.

Mutation-checked: narrowing the guard back to zip alone fails the OLE case.
The first version of that test could not have caught it — a single mocked
from_buffer answered the rescue's own re-classification too, hiding the
promotion.

* UN-1924 [FIX] Require a declared main part, and one row per rejected part

Two holes, both confirmed rather than argued.

Member names alone were enough to name a zip an Office document, so an
archive carrying the right names with nothing behind them was accepted and
then failed conversion. Confirmed against the deployment: an archive of
exactly that shape came back error_in_ingestion. A real package declares its
main part's content type, so that declaration is required now — matched as
bytes, since the input is untrusted and an XML parser is more surface than
this check warrants.

Rejected rows keyed on content and name alone, so two byte-identical parts
sent under one name collapsed into a single record and the counts came up
short. The upload's position in the request is part of the identity now, so
every rejected part keeps its own row.

The test fixture was the shape that fails conversion, which is why it needed
the live check to expose. It declares a main part now, and the
names-only archive is pinned as rejected.

Mutation-checked: dropping either requirement fails its test.

* UN-1924 [FIX] Correlate the declared main part with what relates it

Requiring the marker was too loose: an archive with main+xml as stray text,
an empty _rels/.rels and any word/ member still passed as a document.

A package must now declare its main part's content type, relate that same
part from the root, and contain it. The reported type comes from the
declaration rather than guessed from a member-name prefix.

Still bytes and bounded reads, not an XML parser — the input is an untrusted
upload. Attributes are matched in any order and either quoting style, so a
real package is recognised however its XML is written.

Checked against three LibreOffice artefacts and four near-miss shapes: stray
marker, declared-but-unrelated, relationship disagreeing with the
declaration, and declared part absent from the archive. All four rejected,
all three real packages identified.

* UN-1924 [FIX] Parse package declarations instead of matching their bytes

Matching raw bytes kept admitting archives that are not documents: an XML
comment satisfied the Override match, and the ODF branch trusted the mimetype
member on its own. Tightening the patterns was not converging — a comment, a
namespace prefix or an escaped attribute each defeat matching, so the
declarations are parsed now.

OOXML must declare its main part, relate that same part from the root, and
contain it. ODF must have a manifest declaring the same media type for the
package root. The reported type comes from the declaration, not from a
member-name prefix.

Reads stay bounded, and a member carrying a DTD is refused rather than
parsed, so entity expansion never reaches the parser. Stdlib only:
defusedxml is present for the backend and the workers but is not declared by
unstract-core, and this did not warrant adding a dependency there.

Checked against four genuine LibreOffice artefacts and seven near-miss
shapes, including both bypasses Greptile found and an entity bomb.

The ODF cases are asserted against the helper directly: libmagic names that
archive shape itself, so staging never escalates it on this build — but its
container verdicts differ between builds, which is exactly why the branch
should not be left loose.

* UN-1924 [MISC] Split the OOXML check to clear the complexity gate

Parsing the declarations took _identify_ooxml to cognitive complexity 18
against a limit of 15. Finding the declared main part and confirming what
relates it are separate questions, so they are separate functions now.

All eleven verification shapes still behave the same.

* UN-1924 [FIX] Refuse a DTD whatever encoding it is written in

The guard searched the raw bytes for ASCII markers, but the parser detects
the document's encoding — so a DTD written in UTF-16 was invisible to the
search and still expanded once parsing began. Confirmed: a 1.1 KB archive
carrying a UTF-16 entity chain was accepted as a document.

The member is now checked against every encoding the parser accepts, so the
DTD is found regardless of how it was written.

Mutation-checked: restricting the search back to UTF-8 fails the UTF-16
case.

…ble highlights (#2333)

* UN-4069 [FIX] Raise upload limit on prompt_output to 10 MB with clear error

Agentic table extraction output with highlight coordinates can exceed
Django's default 2.5 MB DATA_UPLOAD_MAX_MEMORY_SIZE, causing a generic
500 that retries uselessly. Use request.read() to bypass the global
limit on this view only, enforce a 10 MB cap, and return 413 with an
explicit size message so the error is actionable in the UI.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* UN-4069 [FIX] Update prompt_output tests for request.read() change

The view now uses request.read() instead of request.body, so mock
requests need read.return_value and META set alongside body.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* UN-4069 [FIX] Raise prompt_output upload limit to 50 MB

Dense tables with 1000+ rows and per-cell highlight coordinates can
produce payloads of 10-20 MB, exceeding the previous 10 MB cap.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* UN-4069 [MISC] Fix stale docstring: 10 MB → 50 MB

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Signed-off-by: harini-venkataraman <115449948+harini-venkataraman@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

…#2334)

* UN-1412 [FIX] Mirror API deployment errors into message.error

Every 4xx/5xx raised on the deployment execute and status endpoints now
also carries {"message": {"execution_status": "ERROR", "error": ...}},
the key successful and failed executions already use. The existing
type/errors keys and status codes are unchanged, so clients parsing them
keep working.

* UN-1412 [FIX] Self-review: keep the status read untouched, match client error text

On GET, message is the result payload and the published Python client
copies it out as extraction_result, so an error block there would read
as a result. Limit the message block to the execute endpoint and its
spec, and join details without attr, as the client already renders a
refused request.