Repository navigation
L4: flow clone action — whole-definition copy, mandatory new name, no ancestry (ADR-0126 §7.1) #12156
Description
Activity
Claim: maintainer direct-dispatch (#12150 program — dispatched at creation, anti-preemption per maintainer instruction 2026-08-25)
Session:session_01KWRU3s15AJz7PGW7a7wdCh
Branch:claude/issue-12156-flow-clone-action
Domain: domain:services
File surface: the flow clone row action in the services/automation domain (+tests); ⛔ nopackages/spec, no objectui (the renderer half rides objectstack-ai/objectui#6301), no ledger writes (clone has no ledger involvement — amendment ruling 2)
Container & model:mode:subagent,model: opus(implementation leg, default-judgment tier)
Clause-②: judged no — a new action surface, no change to existing accept/reject behavior; its own same-name refusal is new-surface validation. Flagged for reviewer confirmation on the PR.
Serial constraints: PR #12026 (#11997 name-shadow diagnostics) is in flight on adjacent flow-name territory — read its diff before choosing test names/surfaces; no shared single-writer path expected
Generated by Claude Code
{
"issue": 12156,
"status": "done",
"premise_still_valid": true,
"branch": "claude/issue-12156-flow-clone-action",
"pr": "#12190",
"files_changed": [
"packages/runtime/src/flow-clone.ts (new) — whole-definition copy, mutated-field set, dropped-key set, same-name refusal message, success notice",
"packages/runtime/src/domains/automation.ts — POST /:name/clone route, its arm in isFlowAuthoringWrite, route-table doc entry (+149 lines, no deletions)",
"packages/runtime/src/domains/automation-flow-clone.test.ts (new) — 18 tests",
".changeset/olive-crabs-clone.md — minor, @objectstack/runtime (v17 line)"
],
"gates_run": [
"pnpm --filter @objectstack/runtime typecheck -> TYPECHECK_EXIT=0 (tsc --noEmit)",
"check:route-envelope -> 'Dispatcher domains - 16 audited: 11 helper-only, 5 with declared hand-built responses (0 ratcheted)'",
"check:cross-package-test-inputs -> 'OK: 16 package(s) read outside themselves, all declared'",
"check:published-files -> '69 publishable package(s) ... declare a files whitelist'",
"check:slot-lookup -> 'ratchet holds: 107 unswept site(s) in 25 file(s), none new'",
"check:query-options-erasure -> 'ratchet holds: 67 unswept non-test site(s) in 17 file(s), none new'",
"check:type-check-coverage -> self-test 47+65+29+28+19 case(s) hold",
"check:test-source-alias -> 'OK - 72 packages with tests scanned'",
"check:type-source-resolution -> 'OK - 93 tsc program(s) across 77 packages scanned'",
"check:changeset-gate-self-tests / check-empty-changeset / check-changeset-no-major / check-adr-0087-registration -> all green ('no major bump', '1 declaring changeset added', 'no declared-breaking changeset')",
"check:objectui-changeset, check:ci-filter-parity, check:plugin-teardown-shape, docs-audit/check-affected-docs, pm/release-rehearsal-clone --self-test, check-nul-bytes -> all green",
"ALL 18 derived gates reported EXIT=0 individually (exit captured before any pipe). Gate list derived by 'node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack' from the merge base, run at final commit 2f2e8ec. NOTE: the wrapper script's own exit was 1 - a trailing grep in my reporting loop found no match in the three gates that are quiet on success (nul-bytes, test-source-alias, type-source-resolution); all three logs were read and are clean."
],
"tests": "All at final commit 2f2e8ec. (1) pnpm --filter @objectstack/runtime exec vitest run --maxWorkers=2 src/domains/automation-flow-clone.test.ts -> 'Test Files 1 passed (1) / Tests 18 passed (18)'. (2) With the two neighbour files that share the route and its gate (automation-toggle-unknown-flow.test.ts, automation-write-capability-gate.test.ts) -> 'Test Files 3 passed (3) / Tests 50 passed (50)'. (3) typecheck TYPECHECK_EXIT=0. ABLATION proving the #11703 deep-equality test can fail: cloneFlowDefinition's body was replaced with an enumerated four-facet assembly (type/nodes/edges/version). Mutation confirmed ON DISK by counting the text I meant to change in both directions - injected marker 'ABLATION_ENUMERATED_FACETS' count = 1, deleted text 'structuredClone(source)' count = 0 (the editor's exit code proves nothing; a zero-hit replace also exits 0). Result: RED, 'Tests 2 failed | 16 passed (18)', the two being the deep-equality test and the deep-copy test - the predicted direction. RESTORE leg: cmp against the pre-mutation copy = byte-identical YES, marker count back to 0, deleted text back to 1, re-run GREEN 18/18. NO REBUILD is involved on either leg and none was needed: the test imports '../flow-clone.js', a relative SOURCE specifier vitest resolves inside packages/runtime/src, so no dist sits between the mutation and the measurement. The mutation script carried a 'trap ... EXIT INT TERM' restore. Worth flagging to review: only 2 of 18 tests moved under the ablation - the whole-object deep-equality assertion is the ONLY thing standing between this action and #11703's failure mode.",
"deviations": [
"SURFACE: the card says 'a clone action for flows on the data door', shaped on the permission-set clone's ObjectSchemaactions:entry. There is no sys_flow ObjectSchema - flows are metadata, and their authoring door is the /automation domain (POST / , PUT /:name, DELETE /:name, POST /:name/toggle). The clone was therefore built as POST /automation/:name/clone in packages/runtime/src/domains/automation.ts, mirroring the permission-set clone's IDIOM (a named action with a mandatory new name/label, a dedicated endpoint like discard_permission_set_overlay's) rather than its literal location.",
"SHAPE: the reference clone at sys-permission-set.object.ts:115-152 is itself the enumerated-param-list assembly that #11703 measured failing. The card forbids that shape for flows, so the reference was studied and deliberately NOT copied structurally - only its dialogue idiom (mandatory new name, mandatory new label, an explanatory description sentence) carried over.",
"ADDED SCOPE (small, deliberate, argued on the PR): the source's ADR-0010 protection envelope (_packageId/_provenance/_lock/...) is DROPPED from the clone rather than copied. FlowSchema spreads MetadataProtectionFields, so a packaged flow's parsed definition really carries these. Copying them would record the base's package on the clone (ancestry, banned by amendment ruling 2), make the clone a package artifact that upgrade/uninstall re-seeds or removes, and carry the base's _lock onto the clone - leaving it as uneditable as the flow the admin cloned to get around. The drop is DERIVED from MetadataProtectionFields + METADATA_READ_DECORATIONS, not restated as a literal list. Consequence for the card's acceptance wording: deep equality is asserted against the source MINUS the envelope, with the envelope drop pinned by its own separately-named test. Flagged on the PR for reviewer confirmation.",
"NO NEW CONTRACT METHOD: packages/spec is excluded by the card, so the route is built from the existing IAutomationService getFlow + registerFlow rather than a new cloneFlow method. Upside, not just a workaround: the clone goes through the engine's own registerFlow, so it is canonicalized and validated exactly as a create is. Same reasoning picked the refusal code - the standard-catalog RESOURCE_CONFLICT (HttpStatusErrorCodeMap 409), which needs no error-code-ledger entry and therefore no spec change.",
"GATE WIDENED: POST /:name/clone was added to isFlowAuthoringWrite (manage_metadata). A clone REGISTERS flow metadata at environment scope, so an ungated clone door would have reproduced the #10145 escalation verbatim - with the twist that the caller need not author a definition, it copies a trusted one. Matched exactly as the toggle arm is (no depth bound, parts[0] !== 'trigger' so the legacy execution door for a flow literally named 'clone' is not over-blocked); both properties have tests.",
"SERIAL CONSTRAINT HONOURED: PR #12026 (#11997 name-shadow diagnostics) was read before choosing surfaces. Its files are in packages/objectql (index.ts, registry.ts) and packages/services/service-automation (engine.ts, plugin.ts, index.ts, flow-precedence.ts, flow-name-shadowing.test.ts). This PR touches NEITHER package - zero file overlap. Its mechanism is cited in my refusal message's reasoning only.",
"CI NOT WAITED ON, per the standing dispatch contract: report delivered at draft-PR time. Gate status at report time is local-green; CI convergence is the PM's read."
],
"open_questions": [
{
"question": "Confirm the protection-envelope drop (deviation 3). ADR-0126 §7.1 says 'mutate only name/label/status'; taken byte-literally that would carry _packageId/_provenance/_lock onto the clone.",
"options": [
"A - drop the envelope (implemented): the clone is org-owned and editable, and carries no pointer to the base's package.",
"B - carry the envelope verbatim: literal reading of 'mutate only three fields', but the clone stays locked, package-owned, re-seeded on upgrade, and names its base."
],
"recommendation": "A. B is self-defeating on the ADR's own terms - §7.1 calls the clone 'an ordinary org/install-owned flow' and ruling 2 forbids any record of what it was copied from, and _packageId is exactly such a record. B also leaves the clone as uneditable as the original, which deletes the feature's reason to exist."
},
{
"question": "Not a blocker, recorded so it is not discovered later: a cloned flow is ARMED. The engine disables on status 'obsolete'/'invalid' only, so 'draft' (and 'active') stay enabled AND get the trigger bound - a cloned record-change or schedule flow fires alongside its base from the moment it is created. The action's response states this plainly and points at the toggle door, and ADR-0126 §7.2 rules clone and disable INDEPENDENT primitives ('not halves of an unfinished ceremony'), so this card deliberately did not fold a disable in. Deliberately NOT filed as a separate issue: the condition is pre-existing, already surfaced by the bootstrap draft-and-enabled audit, and §7.2 already charters the disable primitive - a new card would duplicate it. Raising it here for PM to route if that reading is wrong.",
"options": [
"A - leave as-is: clone stays a pure copy, the notice carries the fact, §7.2's disable primitive is the answer.",
"B - file a card to make 'draft' actually unbind the trigger (a platform-wide semantic change, well beyond this issue)."
],
"recommendation": "A for this card. B is a real question about what 'draft' means platform-wide, but it is not clone's to decide and would change behaviour for every existing draft flow."
}
],
"out_of_scope_findings": [],
"summary": "Adds POST /api/v1/automation/:name/clone (ADR-0126 §7.1): a flow clone that deep-copies the parsed definition and mutates exactly name/label/status, never assembling from an enumerated facet list. New machine name and label are both mandatory; a same-name or otherwise-taken name is refused 409 RESOURCE_CONFLICT with a message naming the offending name, the reason (the engine's bare-name flow map makes a second definition a silent, registration-order-dependent shadow) and the remedy. No ancestry is recorded on the definition or the response, and the source's ADR-0010 protection envelope is dropped so the clone is an org-owned editable flow rather than a second copy of the package's locked artifact. References are not re-pointed and the response says so, together with the fact that status 'draft' is a lifecycle label and not an off-switch. The route joins the manage_metadata authoring-write set. 18 new tests including the #11703-shaped whole-object deep-equality test, whose ability to fail was proven by ablation (2 of 18 red under an enumerated-facet mutation, mutation and restore both confirmed on disk)."
}
Generated by Claude Code
Generated by Claude Code
- added a commit that references this issue
on Sep 9, 2026
Part of #12150 (Epic: ADR-0126 implementation, v17 line — ships as a 17.x minor, maintainer-ruled). Cut on maintainer instruction 2026-08-25 by PM session
session_01KWRU3s15AJz7PGW7a7wdCh. Contract: ADR-0126 §7.1 (merged28b47a93). Independent of the ledger — cloning is authoring an ordinary sibling, no ledger involvement (amendment ruling 2).Deliverable
A clone action for flows on the data door, shaped on the landed permission-set clone (
sys-permission-set.object.ts:115-152, #11513):name/label/status. ⛔ Never param-list assembly: Theclone_permission_setaction copies only 2 of the 6 definition facets, so a clone silently drops system permissions, RLS and tab permissions #11703 measured an enumerated-facet clone silently dropping three of six facets, and a flow has far more facets.Acceptance
name/label/status— the Theclone_permission_setaction copies only 2 of the 6 definition facets, so a clone silently drops system permissions, RLS and tab permissions #11703 counter-example as a test).defaultFromRowhas no non-editable carry-over #11753 discipline) — renderer half rides L5, not this card.Refs: ADR-0126 §7.1, §9 · #11513 / #11703 / #11753 · #11997