Skip to content

Commit 113626c

Browse files
committed
Lift legacy UserToolSource representations on read
Stored DynamicTool.value rows on long-lived servers predate the YAML narrowing in ec5cfe6 and carry internal-model fields the strict schema rejects, causing 500 ResponseValidationError on GET /api/unprivileged_tools (Sentry GALAXY-TEST-588ZYT7JSX3V0). Adds lift_user_tool_source(value) which validates against the strict UserToolSource and returns one of three statuses: - "ok": clean row, parsed model. - "lifted": extra_forbidden-only drift; offending paths are stripped and the model is re-validated. Dropped paths are reported. - "invalid": any other schema violation; raw dict is returned with a compact error summary so the endpoint stays up under future tightening (e.g. stricter container constraints). UnprivilegedToolResponse.representation becomes Union[UserToolSource, dict] with new representation_status and representation_errors fields. Status is also surfaced via Warning, X-Galaxy-Deprecated-Fields, and X-Galaxy-Schema-Errors response headers (computed per request, not part of the OpenAPI schema). The same lift is applied in /api/tools/{id}/raw_tool_source. Frontend: UserToolPanel reads representation defensively and shows "needs update" / "schema error" badges.
1 parent d1e450a commit 113626c

7 files changed

Lines changed: 377 additions & 16 deletions

File tree

client/src/api/schema/schema.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23754,7 +23754,23 @@ export interface components {
2375423754
* @example 0123456789ABCDEF
2375523755
*/
2375623756
id: string;
23757-
representation: components["schemas"]["UserToolSource-Output"];
23757+
/** Representation */
23758+
representation:
23759+
| components["schemas"]["UserToolSource-Output"]
23760+
| {
23761+
[key: string]: unknown;
23762+
};
23763+
/**
23764+
* Representation Errors
23765+
* @default []
23766+
*/
23767+
representation_errors: string[];
23768+
/**
23769+
* Representation Status
23770+
* @default ok
23771+
* @enum {string}
23772+
*/
23773+
representation_status: "ok" | "lifted" | "invalid";
2375823774
/** Tool Format */
2375923775
tool_format: string | null;
2376023776
/** Tool Id */

client/src/components/Panels/UserToolPanel.vue

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,21 @@ const currentItemId = computed(() => {
5050
return match ? match[0] : undefined;
5151
});
5252
53+
function repField(tool: UnprivilegedToolResponse, key: string): string | undefined {
54+
// representation may be a typed UserToolSource (status ok/lifted) or a raw
55+
// dict (status invalid). Both shapes still hold name/id/etc by string key,
56+
// but the union type loses property access — read defensively.
57+
const rep = tool.representation as Record<string, unknown> | undefined;
58+
const value = rep?.[key];
59+
return typeof value === "string" ? value : undefined;
60+
}
61+
5362
function cardClicked(tool: UnprivilegedToolResponse) {
5463
if (props.inPanel) {
5564
emit("unprivileged-tool-clicked", tool);
5665
}
5766
if (props.inWorkflowEditor) {
58-
emit("onInsertTool", tool.representation.id, tool.representation.name, tool.uuid);
67+
emit("onInsertTool", repField(tool, "id"), repField(tool, "name"), tool.uuid);
5968
} else {
6069
router.push(`/?tool_uuid=${tool.uuid}`);
6170
}
@@ -78,13 +87,31 @@ function newTool() {
7887
}
7988
8089
function getToolBadges(tool: UnprivilegedToolResponse) {
81-
return [
90+
const badges = [
8291
{
8392
id: "version",
84-
label: tool.representation.version ?? "",
93+
label: repField(tool, "version") ?? "",
8594
title: "Version of this custom tool",
8695
},
8796
];
97+
if (tool.representation_status === "lifted") {
98+
badges.push({
99+
id: "status",
100+
label: "needs update",
101+
title:
102+
"This tool's stored definition uses conventions that are no longer valid; " +
103+
"they are ignored on read. Re-save the tool to clean it up.",
104+
});
105+
} else if (tool.representation_status === "invalid") {
106+
badges.push({
107+
id: "status",
108+
label: "schema error",
109+
title:
110+
"This tool's stored definition does not satisfy the current schema. " +
111+
"Open it in the editor to repair.",
112+
});
113+
}
114+
return badges;
88115
}
89116
90117
function getToolSecondaryActions(tool: UnprivilegedToolResponse) {
@@ -141,7 +168,7 @@ function getToolSecondaryActions(tool: UnprivilegedToolResponse) {
141168
:active="tool.uuid === currentItemId"
142169
:badges="getToolBadges(tool)"
143170
:secondary-actions="getToolSecondaryActions(tool)"
144-
:title="tool.representation.name"
171+
:title="repField(tool, 'name') ?? tool.uuid"
145172
:title-icon="{ icon: faWrench }"
146173
title-size="text"
147174
:update-time="tool.create_time"
@@ -150,7 +177,7 @@ function getToolSecondaryActions(tool: UnprivilegedToolResponse) {
150177
<template v-slot:description>
151178
<Heading class="m-0" size="text">
152179
<small class="text-muted truncate-n-lines two-lines">
153-
{{ tool.representation.description }}
180+
{{ repField(tool, "description") }}
154181
</small>
155182
</Heading>
156183
</template>

client/src/composables/agentActions.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,8 @@ export function useAgentActions() {
144144
return;
145145
}
146146

147-
toast.success(`Tool "${data.representation.name}" saved successfully!`);
147+
const repName = (data.representation as Record<string, unknown> | undefined)?.name ?? data.uuid;
148+
toast.success(`Tool "${String(repName)}" saved successfully!`);
148149
unprivilegedToolStore.load(true);
149150
router.push(`/tools/editor/${data.uuid}`);
150151
} catch (err) {

lib/galaxy/tool_util_models/__init__.py

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
Dict,
1010
List,
1111
Optional,
12+
Tuple,
1213
Union,
1314
)
1415

@@ -21,6 +22,7 @@
2122
model_validator,
2223
RootModel,
2324
Tag,
25+
ValidationError,
2426
)
2527
from typing_extensions import (
2628
Annotated,
@@ -155,6 +157,95 @@ class YamlToolSource(_DynamicToolSourceBase):
155157
DynamicToolSources = Annotated[Union[UserToolSource, YamlToolSource], Field(discriminator="class_")]
156158

157159

160+
# ---------------------------------------------------------------------------
161+
# Schema-drift "lift" for stored DynamicTool.value rows.
162+
#
163+
# The strict `UserToolSource` schema is the single source of truth for both
164+
# input and output. Stored rows from older Galaxy versions may carry fields
165+
# that the current schema rejects (e.g. legacy internal-model fields after the
166+
# YAML narrowing) or values that fail tightened constraints (e.g. a future
167+
# `container` regex). `lift_user_tool_source` validates against the strict
168+
# schema and:
169+
# - on success: returns ("ok", parsed_model, []).
170+
# - on `extra_forbidden`-only failure: strips the offending paths and
171+
# re-validates. Returns ("lifted", parsed_model, dropped_paths).
172+
# - on any other failure: returns ("invalid", original_dict, error_summary).
173+
# The endpoint exposes this so legacy/broken rows don't crash the API.
174+
# ---------------------------------------------------------------------------
175+
176+
LiftStatus = Literal["ok", "lifted", "invalid"]
177+
178+
179+
def _navigable_path(value: Any, loc: tuple) -> Tuple[Optional[Any], List[Any]]:
180+
"""Walk `loc` against the structure of `value`, skipping steps that don't
181+
correspond to a real key/index (pydantic inserts discriminator literals
182+
like `"data"` into the loc for tagged unions). Returns the parent
183+
container of the leaf and the cleaned path components, or (None, []) if
184+
the path can't be resolved."""
185+
cur: Any = value
186+
*prefix, leaf = loc
187+
cleaned: List[Any] = []
188+
for step in prefix:
189+
if isinstance(cur, list) and isinstance(step, int) and 0 <= step < len(cur):
190+
cur = cur[step]
191+
cleaned.append(step)
192+
elif isinstance(cur, dict) and step in cur:
193+
cur = cur[step]
194+
cleaned.append(step)
195+
# else: skip — discriminator tag or stale path
196+
cleaned.append(leaf)
197+
return cur, cleaned
198+
199+
200+
def _format_loc(value: Any, loc: tuple) -> str:
201+
_parent, cleaned = _navigable_path(value, loc)
202+
return ".".join(str(p) for p in cleaned if p != "representation")
203+
204+
205+
def _strip_path(value: dict, loc: tuple) -> bool:
206+
"""Remove the field at the given pydantic error `loc` from `value`. Returns
207+
True if a key was actually removed."""
208+
parent, cleaned = _navigable_path(value, loc)
209+
if not cleaned or not isinstance(parent, dict):
210+
return False
211+
leaf = cleaned[-1]
212+
if leaf in parent:
213+
del parent[leaf]
214+
return True
215+
return False
216+
217+
218+
def lift_user_tool_source(
219+
value: dict,
220+
) -> Tuple[LiftStatus, Union["UserToolSource", Dict[str, Any]], List[str]]:
221+
"""Validate `value` against the strict UserToolSource, lifting drift where
222+
safe. See module docstring above for the contract.
223+
"""
224+
import copy
225+
226+
try:
227+
return ("ok", UserToolSource.model_validate(value), [])
228+
except ValidationError as e:
229+
errors = e.errors()
230+
231+
extra_forbidden = [err for err in errors if err.get("type") == "extra_forbidden"]
232+
other = [err for err in errors if err.get("type") != "extra_forbidden"]
233+
if extra_forbidden and not other:
234+
stripped = copy.deepcopy(value)
235+
dropped: List[str] = []
236+
for err in extra_forbidden:
237+
loc = tuple(err["loc"])
238+
if _strip_path(stripped, loc):
239+
dropped.append(_format_loc(value, loc))
240+
try:
241+
return ("lifted", UserToolSource.model_validate(stripped), dropped)
242+
except ValidationError as e2:
243+
errors = e2.errors()
244+
245+
summary = [f"{_format_loc(value, tuple(err['loc']))}: {err.get('msg', err.get('type', ''))}" for err in errors]
246+
return ("invalid", value, summary)
247+
248+
158249
class ParsedTool(ToolSourceBaseModel):
159250
id: str
160251
version: Optional[str]

lib/galaxy/webapps/galaxy/api/dynamic_tools.py

Lines changed: 77 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@
22
from datetime import datetime
33
from typing import (
44
Any,
5+
Literal,
56
Optional,
67
Union,
78
)
89

10+
from fastapi import Response
911
from pydantic import BaseModel
1012

1113
from galaxy.exceptions import (
@@ -29,7 +31,10 @@
2931
from galaxy.tool_util.parameters import input_models_for_tool_source
3032
from galaxy.tool_util.parameters.convert import cwl_runtime_model
3133
from galaxy.tool_util.parser.yaml import YamlToolSource
32-
from galaxy.tool_util_models import UserToolSource
34+
from galaxy.tool_util_models import (
35+
lift_user_tool_source,
36+
UserToolSource,
37+
)
3338
from galaxy.tool_util_models.dynamic_tool_models import (
3439
DynamicToolPayload,
3540
DynamicUnprivilegedToolCreatePayload,
@@ -48,6 +53,22 @@
4853
DatabaseIdOrUUID = Union[DecodedDatabaseIdField, str]
4954

5055

56+
def _set_lift_headers(response: Response, status: str, errors: list[str]) -> None:
57+
"""Surface lift status as response headers so consumers can react without
58+
parsing the response body. Headers are computed dynamically per request and
59+
are intentionally not declared in the OpenAPI schema."""
60+
if status == "ok" or not errors:
61+
return
62+
if status == "lifted":
63+
compact = ",".join(errors)
64+
response.headers["X-Galaxy-Deprecated-Fields"] = compact
65+
response.headers["Warning"] = f'299 - "Some conventions are no longer valid; ignored on read: {compact}"'
66+
elif status == "invalid":
67+
compact = "; ".join(errors)
68+
response.headers["X-Galaxy-Schema-Errors"] = compact
69+
response.headers["Warning"] = f'299 - "Stored tool no longer satisfies schema: {compact}"'
70+
71+
5172
class UnprivilegedToolResponse(BaseModel):
5273
id: EncodedDatabaseIdField
5374
uuid: str
@@ -56,7 +77,32 @@ class UnprivilegedToolResponse(BaseModel):
5677
tool_id: Optional[str]
5778
tool_format: Optional[str]
5879
create_time: datetime
59-
representation: UserToolSource
80+
# Either a strict UserToolSource (status="ok" or "lifted") or the raw
81+
# stored dict (status="invalid"). Consumers narrow on `representation_status`.
82+
representation: Union[UserToolSource, dict[str, Any]]
83+
representation_status: Literal["ok", "lifted", "invalid"] = "ok"
84+
representation_errors: list[str] = []
85+
86+
87+
def _build_unprivileged_tool_response(d: dict[str, Any]) -> UnprivilegedToolResponse:
88+
"""Run the lift over the stored representation and assemble the response.
89+
This is the single place where the lift result is unpacked, so the helper's
90+
invariants (status='ok' → strict model, 'lifted' → strict model + dropped
91+
paths, 'invalid' → raw dict + error summary) are enforced exactly once."""
92+
raw_representation = d.get("representation") or {}
93+
status, representation, errors = lift_user_tool_source(raw_representation)
94+
return UnprivilegedToolResponse(
95+
id=d["id"],
96+
uuid=d["uuid"],
97+
active=d["active"],
98+
hidden=d["hidden"],
99+
tool_id=d.get("tool_id"),
100+
tool_format=d.get("tool_format"),
101+
create_time=d["create_time"],
102+
representation=representation,
103+
representation_status=status,
104+
representation_errors=errors,
105+
)
60106

61107

62108
@router.cbv
@@ -66,17 +112,38 @@ class UnprivilegedToolsApi:
66112
dynamic_tools_manager: DynamicToolManager = depends(DynamicToolManager)
67113

68114
@router.get("/api/unprivileged_tools", response_model_exclude_defaults=True)
69-
def index(self, active: bool = True, trans: ProvidesUserContext = DependsOnTrans) -> list[UnprivilegedToolResponse]:
115+
def index(
116+
self,
117+
response: Response,
118+
active: bool = True,
119+
trans: ProvidesUserContext = DependsOnTrans,
120+
) -> list[UnprivilegedToolResponse]:
70121
if not trans.user:
71122
return []
72-
return [t.to_dict() for t in self.dynamic_tools_manager.list_unprivileged_tools(trans.user, active=active)]
123+
result = [
124+
_build_unprivileged_tool_response(t.to_dict())
125+
for t in self.dynamic_tools_manager.list_unprivileged_tools(trans.user, active=active)
126+
]
127+
# Aggregate header: any tool with drift surfaces it on the index call.
128+
worst = "ok"
129+
aggregate: list[str] = []
130+
for tool in result:
131+
if tool.representation_status == "invalid":
132+
worst = "invalid"
133+
elif tool.representation_status == "lifted" and worst == "ok":
134+
worst = "lifted"
135+
aggregate.extend(tool.representation_errors)
136+
_set_lift_headers(response, worst, aggregate)
137+
return result
73138

74139
@router.get("/api/unprivileged_tools/{uuid}", response_model_exclude_defaults=True)
75-
def show(self, uuid: str, user: User = DependsOnUser) -> UnprivilegedToolResponse:
140+
def show(self, response: Response, uuid: str, user: User = DependsOnUser) -> UnprivilegedToolResponse:
76141
dynamic_tool = self.dynamic_tools_manager.get_unprivileged_tool_by_uuid(user, uuid)
77142
if dynamic_tool is None:
78143
raise ObjectNotFound()
79-
return UnprivilegedToolResponse(**dynamic_tool.to_dict())
144+
tool = _build_unprivileged_tool_response(dynamic_tool.to_dict())
145+
_set_lift_headers(response, tool.representation_status, tool.representation_errors)
146+
return tool
80147

81148
@router.post("/api/unprivileged_tools", response_model_exclude_defaults=True)
82149
def create(
@@ -86,7 +153,10 @@ def create(
86153
user,
87154
payload,
88155
)
89-
return UnprivilegedToolResponse(**dynamic_tool.to_dict())
156+
# Just-created tools are validated through strict input, so the lift
157+
# is a no-op here, but going through the same builder keeps behavior
158+
# uniform.
159+
return _build_unprivileged_tool_response(dynamic_tool.to_dict())
90160

91161
@router.post("/api/unprivileged_tools/build")
92162
def build(

lib/galaxy/webapps/galaxy/api/tools.py

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,10 @@
6363
ToolParameterT,
6464
)
6565
from galaxy.tool_util.verify import ToolTestDescriptionDict
66-
from galaxy.tool_util_models import UserToolSource
66+
from galaxy.tool_util_models import (
67+
lift_user_tool_source,
68+
UserToolSource,
69+
)
6770
from galaxy.tools.evaluation import global_tool_errors
6871
from galaxy.tools.fetch.workbooks import (
6972
FetchWorkbookCollectionType,
@@ -878,7 +881,24 @@ def raw_tool_source(self, trans: GalaxyWebTransaction, id, **kwds):
878881
trans.response.headers["language"] = tool.tool_source.language
879882
if dynamic_tool := getattr(tool, "dynamic_tool", None):
880883
if dynamic_tool.value.get("class") == "GalaxyUserTool":
881-
return UserToolSource(**dynamic_tool.value).model_dump_json(
884+
status, lifted, errors = lift_user_tool_source(dynamic_tool.value)
885+
if status == "lifted" and errors:
886+
compact = ",".join(errors)
887+
trans.response.headers["X-Galaxy-Deprecated-Fields"] = compact
888+
trans.response.headers["Warning"] = (
889+
f'299 - "Some conventions are no longer valid; ignored on read: {compact}"'
890+
)
891+
elif status == "invalid":
892+
compact = "; ".join(errors)
893+
trans.response.headers["X-Galaxy-Schema-Errors"] = compact
894+
trans.response.headers["Warning"] = f'299 - "Stored tool no longer satisfies schema: {compact}"'
895+
# Return the raw stored value so callers can still inspect /
896+
# repair the YAML manually.
897+
import json
898+
899+
return json.dumps(dynamic_tool.value)
900+
assert isinstance(lifted, UserToolSource)
901+
return lifted.model_dump_json(
882902
by_alias=True,
883903
exclude_defaults=True,
884904
exclude_unset=True,

0 commit comments

Comments
 (0)