[Change Control] An import configuration can be proposed instead of written - #693
Draft
markus-moser wants to merge 13 commits into
Draft
[Change Control] An import configuration can be proposed instead of written#693markus-moser wants to merge 13 commits into
markus-moser wants to merge 13 commits into
Conversation
…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>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #691 and #692 (both folded in; review those first).
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: the change rides a change set, and a data steward reviews it in the importer's own editor before anything is stored.
What it adds
ImportConfigSubjectHandlerapplyMerged=updateConfiguration(). Publish-only; gated onplugin_datahub_config.ImportConfigDetailHydratorImportConfigReviewSurfaceOptional throughout. The services load only behind
interface_exists(SubjectHandlerInterface::class)— guarded with aClassExistenceResourceso installing or removing the bundle reshapes the container — and the surface resolves the review registry by id, so an installation without Change Control is unaffected. (class_exists()returns false for interfaces; that is why the guard isinterface_exists.)Two decisions worth reviewing
Slot addresses are document paths, not form paths. They are handed back to the merge as exclude paths, so they have to name a place in the stored tree. The editor's form flattens
general.*to its root, and the surface translates for the fields it annotates — that translation belongs on the rendering side.mappingConfigrides one address, whole. A positional address into a list stops being stable the moment a row is inserted, so "the mappings" is a decision a reviewer can make while "mapping 7" is not. Rows still diff individually for display: bymappingIdwhere both sides carry one, falling back to label plus source columns, because the id is minted in the Studio form and a configuration written by the console has none.Verified end to end
Seeded a proposal against the demo's
car-import(17 mappings) and drove it through the Studio UI asadmin: 3 field changes + 4 mapping changes shown, approved, and the merge applied — description, cron0 3 * * *, cleanup strategy, 17 → 18 mappings,additional imagesremoved,co2 emissionandfuel typeadded,mileageretyped to a quantity value in km.Gates
npm run check-typesnpm run lintThe unit suite could not run locally: this checkout has no dev dependencies of its own, and the borrowed Codeception vendor collides with the app's
symfony/contracts.ConfigurationPathMapperTestran green (11 tests, 34 assertions) before the collision appeared;ImportConfigDetailHydratorTest's eight assertions were verified through an equivalent probe against the app's autoloader. Both files need CI.🤖 Generated with Claude Code