Skip to content

[Change Control] An agent can propose an import configuration, and a steward reviews it in the editor - #693

Draft
markus-moser wants to merge 36 commits into
2026.xfrom
feat/change-control-proposals
Draft

[Change Control] An agent can propose an import configuration, and a steward reviews it in the editor#693
markus-moser wants to merge 36 commits into
2026.xfrom
feat/change-control-proposals

Conversation

@markus-moser

@markus-moser markus-moser commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Folds in #691 (editor renders from a configuration it is given) and #692 (stored ↔ form addressing), both closed.

A configuration change an agent makes lands immediately, so nobody sees the mapping it drops until an import runs with it. This makes an import configuration a Change Control subject: an agent proposes it, the change rides a change set, and a data steward reviews it in the importer's own editor before anything is stored. Optional throughout — everything loads behind interface_exists(SubjectHandlerInterface::class) / class_exists(McpTool::class) with ClassExistenceResources, so an installation without Change Control or an MCP host is unaffected.

What it adds

ImportConfigSubjectHandler, ImportConfigDetailHydrator, ImportConfigShape the stored document as a subject; the editor's sections as review slots; a merge shape of its own, because the engine's default reads any keyed array holding type/path as an element reference and would make every section one leaf
ImportConfigReviewSurface the proposed configuration mounted read-only in the editor, every changed field marked in place (mapping rows included, dropped rows left in place struck through); a rail that maps the change per section and opens it in the editor, landing on the first changed field; a configuration that does not exist yet gets a one-glance card instead (source → class, mapping count, run mode) and no per-field marks; history mode diffs against the recorded base
list_import_configs, get_import_config, propose_import_config MCP tools (pimcore-data-importer-read / -propose) with the data-importer-configuration skill. The propose tool folds the agent's document over the stored one (lists replaced whole), normalises a wrapped or index-keyed mapping list, refuses invented sections, refuses any adapter/strategy type the installation does not register (ImportConfigVocabulary, read off the service tags), refuses a change behind a switch that is off, and creates a configuration under a name nothing is stored under — inactive unless told otherwise; a copy of a stored document keeps the Data Hub's workspaces/schema envelope and the inert values its source carries, so a create lands on the first call
preview scope the preview endpoints take a change-set scope: the file lives beside the live one, the permission gate accepts a configuration that is only proposed so far, and the review passes the scope through a context — copy/upload, record paging, column headers and the transformation preview all work inside the modal
ConfigurationPathMapper, DataImporterConfigEditor from #692 / #691: addressing between the stored tree and the editor's form; the editor rendered from a configuration it is handed

Decisions

  • Slot addresses are document paths. The merge takes them back as exclude paths, so they name the stored tree; the surface translates for the fields it marks.
  • mappingConfig rides one address, whole. Positional addresses into a list are not stable; rows still diff individually for display (by mappingId, falling back to label + source columns).
  • A configuration is approved whole. The rail is a map, not a checklist: no per-field include/exclude.
  • PHPStan/Codeception without the optional bundles: the contract implementations are excludePaths, the optional symbols an ignoreErrors pattern; the tests that need Change Control skip when it is absent.

Verified live

Against the demo (admin), through the Configuration agent: read → propose → review → approve, reject, refine, history; a copy created under a new name; an invented publishing strategy and a strategy change behind an off switch both refused and re-proposed correctly. Preview data copied from the source and uploaded inside the review, written to the change set's own file with the live file untouched; paging, column headers and the transformation preview read it. A second full pass through the Configuration agent: propose, approve (written), refine, reject, history, and a create with scoped preview data; the three things it caught (a relabelled mapping row read as removed+added, a jump left the field below the fold, the mapping step overflowed the modal) are fixed.

Gates

PHPStan level 5, php-cs-fixer, check-types, lint: clean. Codeception unit: 27 + 11 + 11 + 7 tests; CI green on the current head.

Overlaps with #529 (PimcoreDataImporterExtension, composer.json, tool namespace); whichever merges first, the other rebases.

🤖 Generated with Claude Code

markus-moser and others added 24 commits September 9, 2026 16:15
…iven

The detail view both fetched the configuration and rendered it, so the tab tree
could only ever show what the detail endpoint returned.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
transformBackendToForm() flattens general.* and the mapping list is addressed by
mappingId when stored but by index in the form, so a dotted address into the
stored document does not name the same field in the editor.

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

A configuration change an agent makes lands immediately, so nobody sees the
mapping it drops until an import runs with it.

Registered only when the Change Control bundle is installed: the handler and the
hydrator implement its interfaces, and the review surface resolves its registry by
id, so an installation without it is unaffected.

Slot addresses are document paths, not form paths - the merge takes them back as
exclude paths. mappingConfig rides one address, whole: a positional address into a
list stops being stable the moment a row is inserted.

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

A review payload carries only the changed subset of the state. Mounted on that
alone the editor rendered every untouched select empty, so the proposed leaves
now lay over the configuration the importer's own endpoint returns.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The review offered a run button, import logs, and Add/Autofill/Delete on the
mapping list - none of which a reader can act on. It also disabled the Advanced
button, which is the one control a reader needs.

Runtime sections come off through showRuntime; the mapping affordances ask a new
read-only context, since a disabled form greys out inputs but leaves plain
buttons live.

The change rail groups by the section the editor shows the field in, folds when
the change set is large, and drops the enumeration entirely for a create - listing
every field of a new configuration says only "all of it" at length.

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

A row now names a place in the editor and carries the reader there: the tab
switches, the Data Setup step switches, the pane scrolls and the field is marked.
Grouped tab -> section, with counts, a state filter and a field search.

The editor composes ContentLayout/Content/FormKit/Tabs itself rather than calling
data-hub's BaseDetailView. That wrapper holds no logic - it passes
defaultActiveKey, which keeps the active tab as its private state and puts it out
of reach of anything that wants to navigate to a field.

The scroll waits two frames: a Data Setup step is hidden with display:none, so a
field in an inactive step has no box to measure until the step switches and the
browser lays it out again.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three MCP tools: list_import_configs, get_import_config and
propose_import_config, the last one opening a change set instead of
writing. The tool folds the agent's document over the stored one (lists
replaced whole, never merged by index), unwraps a wrapped or index-keyed
mapping list, refuses invented sections, and strips the volatile keys the
subject strips — a document the editor cannot render never reaches the
review. A skill tells the agent to read before it proposes.

The subject gets its own merge shape: the engine's structural default reads
any keyed array holding `type` or `path` as an element reference, which made
every section one leaf and an untick in the review a no-op.

The review surface diffs against the recorded base once the change set is
resolved, drops its checkboxes there, and speaks the bundle's translations.
A non-list mapping config degrades instead of taking the surface down.

PHPStan excludes the Change Control contract implementations and ignores the
optional Agent/Studio-MCP symbols; the tests that need those bundles skip
without them.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…rry their own marks

The rail loses its box, pills, filters and search: a caption, the tabs in
editor order, one row per changed field with its checkbox and mark, the
mapping rows underneath. It sits flush with the modal's own gutters, which
the surface was doubling.

The mapping step now marks its rows the way the form marks its fields: an
added row, a changed one with the target it had, and a dropped row left in
place, struck through. The editor shows the list in review order.

A configuration that does not exist yet reviews too: its draft is recorded
whole instead of filtered against a tree that is not there, and the editor's
steps, which read a configuration by name, find the proposed one in the
cache instead of a 404.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The propose tool now checks every adapter and strategy type — loader,
interpreter, the resolver strategies, cleanup, data targets, transformation
operators — against the service tags the factories are built from, and
refuses with the allowed list. The editor's selects cannot hold anything
else; a proposal must not either.

The review rail decides nothing any more: no checkboxes, no include state,
no "was" hints — a map only. Sections in editor order, each field named the
way the editor names it (Publish Strategy, not "type"), the mark beside it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A name nothing is stored under creates one: the document is the whole
configuration, checked for what the importer cannot run without (loader,
format, target class, the resolver strategies) and for a name a file and a
YAML key accept. It starts inactive unless the proposal says otherwise —
a pipeline nobody switched on must not run because it was reviewed. The
skill tells the agent to copy the closest existing configuration rather
than write one from memory.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ewed field stays visible

Cleanup strategy is only read while cleanup is on, delta check and cleanup
only with an ID field. A proposal that changes one of them while its switch
stays off would put a mark on a field the editor folds away and the import
ignores; the tool refuses it and names the switch. Where a change set does
carry such a field, the processing step keeps it on screen for the review.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…l rows read as controls

Studio's own Content spinner while the change set loads, no count reported
before it has been read, and rail rows bordered and filled so they read as
something to press rather than as text on the page.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…them is a listing

Each section headline — at body size — carries one arrow that opens that
tab and step. The fields beneath are no longer controls: a plain listing of
name and mark, one hairline apart. The per-field scroll-to-anchor goes with
it; the section is where the reader lands.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@markus-moser markus-moser changed the title [Change Control] An import configuration can be proposed instead of written [Change Control] An agent can propose an import configuration, and a steward reviews it in the editor Sep 11, 2026
markus-moser and others added 3 commits September 11, 2026 14:38
…s read as a list

The arrow is an ornament on a headline that is the button; the fields under
it are a bulleted, hairline-separated list.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ays it is new

The create marks its proposal isNew, so the card and the review title say
"new" instead of "changed". The rail no longer replaces the map with a
summary for a create: the same sections and fields, every one added, with
the subject's own name, type and path left out — those are identity, not a
change anyone weighs. The list drops its bullets and indent.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
markus-moser and others added 9 commits September 11, 2026 13:57
The review used to mount the importer with its preview toolbar disabled, so a
proposed mapping could only be read, never tried against a record. The preview
endpoints now take a change-set scope: the file lives beside the live one
(`_cs-<uuid>-<user>.import`), the permission gate accepts a configuration that
is only proposed so far, and the review passes the scope through a context so
copy/upload, record paging, column headers and the transformation preview all
work inside the modal. Configuration names are validated in one place
(`Settings\ConfigurationName`), which the scoped file path and the propose
tool share.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Every leaf of a configuration that does not exist yet is "added", so listing
them said only "all of it" at great length, and marked every field in the
editor the same way. The rail now shows the pipeline — source above target
class, with the mapping count — and how it runs as tags; the editor beside it
has the sections and fields. The field list stays for a changed configuration.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Three things a live review round turned up. A row without a mappingId that was
only renamed showed as one row dropped and another added; it now also matches
by source and target. Opening a section from the rail left the changed field
below the fold; the jump now scrolls to the first mark that is laid out. The
mapping step's minimum width overflowed the editor beside the rail in a 90vw
modal on a 1200px screen; 720px still holds both columns.

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

Every stored document carries `workspaces` and `schema` around its sections, so a
create built from get_import_config's output — the very thing the tool asks for —
was refused as "not part of an import configuration" until the agent stripped
them by trial. They are known now, on a create as on a change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The inert-setting check compares a proposal against the stored document; on a
create the baseline is empty, so every setting reads as a change and a copy of
a configuration whose cleanup strategy sits behind an off switch — as the
editor writes it — was refused until the agent stripped the value by trial.

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

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
3 New Critical Issues (required ≤ 0)
24 New Major Issues (required ≤ 0)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants