Skip to content

Commit 4c779d4

Browse files
claudeclaude
committed
Merge remote-tracking branch 'origin/main' into claude/issue-5896-now-pin-identity
2 parents 412af0f + e6025e9 commit 4c779d4

6 files changed

Lines changed: 287 additions & 17 deletions

File tree

‎.changeset/revert-commit-write-intent.md‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
'@objectstack/metadata-protocol': patch
3+
---
4+
5+
fix(metadata-protocol): `revertCommit` states its write intent per item, so an `object` overlay can be reverted at all (#6563)
6+
7+
`ObjectStackProtocolImplementation.revertCommit` restored an edited artifact
8+
through `repo.restoreVersion(ref, prevVersion, { actor, source, message })` — with
9+
no `intent`. `SysMetadataRepository.restoreVersion` therefore fell back to its
10+
`?? 'override-artifact'` default, `put` opened with
11+
`assertAllowed(ref.type, opts.intent)`, and that gate refuses every type whose
12+
registry entry is not `allowOrgOverride`. `object` is exactly such a type, so
13+
every `object` item of a reverted commit came back in `failed[]`:
14+
15+
```
16+
[NOT_OVERRIDABLE] 'object' is not allowOrgOverride in the registry.
17+
Overlay-allowed: view, page, dashboard, app, action, report, dataset, ...
18+
```
19+
20+
The package-commit undo (ADR-0067) therefore could not revert the metadata type
21+
Studio and AI-built apps create most, while the same edit reverted fine one
22+
artifact at a time through the version-history revert — the two user-facing
23+
revert paths disagreed about what is revertable. The failure was per item, so
24+
the call still answered `success` overall with a populated `failed[]`, which
25+
reads as a flaky revert rather than a systematic refusal.
26+
`rollbackToPackageCommit` reverts through the same loop and inherited it, and
27+
there the symptom was quieter still: a per-item refusal never throws, so the
28+
rollback recorded the commit as reverted and answered `success: true` while the
29+
object was untouched.
30+
31+
`revertCommit` now derives the intent from the artifact the way its sibling
32+
`rollbackMetaItem` already does — `isArtifactBacked` gives `'override-artifact'`,
33+
otherwise `'runtime-only'` — and does it **per item**, because a commit is a
34+
batch that routinely mixes a runtime-created object with an overlay on a
35+
packaged view.
36+
37+
The repository's default is deliberately unchanged: it is right for callers that
38+
genuinely mean "override a packaged artifact", and the defect was this caller
39+
never saying which of the two cases it is. So the gate is not widened — an
40+
object a code package really ships still resolves to `'override-artifact'` and
41+
is still refused with `NOT_OVERRIDABLE`, which is pinned alongside the fix.

‎packages/lint/src/reference-integrity-suite.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ describe('reference-integrity suite — every member actually runs', () => {
191191
// the two must not diverge across commands again.
192192
runAs: 'user',
193193
nodes: [
194-
{ id: 'start', type: 'start', config: { objectName: 'crm_lead', triggerType: 'record-created' } },
194+
{ id: 'start', type: 'start', config: { objectName: 'crm_lead', triggerType: 'record-after-create' } },
195195
// validateFlowTemplatePaths: `budget` is not a field on crm_lead. In a
196196
// FILTER position an erased condition widens the query rather than
197197
// narrowing it, so the runtime refuses the node — gating, not advisory

‎packages/lint/src/validate-flow-template-paths.test.ts‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ function flowWith(notify: AnyRec, objectName = 'crm_lead'): AnyRec {
3232
name: 'notify_lead',
3333
type: 'record_change',
3434
nodes: [
35-
{ id: 'start', type: 'start', config: { objectName, triggerType: 'record-created' } },
35+
{ id: 'start', type: 'start', config: { objectName, triggerType: 'record-after-create' } },
3636
{ id: 'n1', type: 'notify', notify },
3737
],
3838
},
@@ -117,7 +117,7 @@ describe('validateFlowTemplatePaths', () => {
117117
name: 'external',
118118
type: 'record_change',
119119
nodes: [
120-
{ id: 'start', type: 'start', config: { objectName: 'sys_user', triggerType: 'record-created' } },
120+
{ id: 'start', type: 'start', config: { objectName: 'sys_user', triggerType: 'record-after-create' } },
121121
{ id: 'n1', type: 'notify', notify: { title: '{record.anything.deep}', body: 'x' } },
122122
],
123123
},
@@ -158,7 +158,7 @@ describe('validateFlowTemplatePaths', () => {
158158
name: 'typed_start',
159159
type: 'record_change',
160160
nodes: [
161-
{ id: 'start', type: 'start', start: { objectName: 'crm_lead', triggerType: 'record-created' } },
161+
{ id: 'start', type: 'start', start: { objectName: 'crm_lead', triggerType: 'record-after-create' } },
162162
{ id: 'n1', type: 'notify', notify: { title: '{record.crm_account.name}', body: 'x' } },
163163
],
164164
},
@@ -176,7 +176,7 @@ describe('validateFlowTemplatePaths', () => {
176176
name: 'webhook',
177177
type: 'record_change',
178178
nodes: [
179-
{ id: 'start', type: 'start', config: { objectName: 'crm_lead', triggerType: 'record-created' } },
179+
{ id: 'start', type: 'start', config: { objectName: 'crm_lead', triggerType: 'record-after-create' } },
180180
{ id: 'h1', type: 'http', http: { url: 'https://x.test/{record.full_naem}', method: 'GET' } },
181181
],
182182
},
@@ -197,7 +197,7 @@ describe('validateFlowTemplatePaths', () => {
197197
{
198198
id: 'start',
199199
type: 'start',
200-
config: { objectName: 'crm_lead', triggerType: 'record-created', expand: ['crm_account'] },
200+
config: { objectName: 'crm_lead', triggerType: 'record-after-create', expand: ['crm_account'] },
201201
},
202202
{ id: 'n1', type: 'notify', notify: { title: 'From {record.crm_account.name}', body: 'x' } },
203203
],
@@ -218,7 +218,7 @@ describe('validateFlowTemplatePaths', () => {
218218
{
219219
id: 'start',
220220
type: 'start',
221-
config: { objectName: 'crm_lead', triggerType: 'record-created', expand: ['target_channels'] },
221+
config: { objectName: 'crm_lead', triggerType: 'record-after-create', expand: ['target_channels'] },
222222
},
223223
{ id: 'n1', type: 'notify', notify: { title: 'From {record.crm_account.name}', body: 'x' } },
224224
],
@@ -247,7 +247,7 @@ describe('validateFlowTemplatePaths', () => {
247247
name: 'crud_flow',
248248
type: 'record_change',
249249
nodes: [
250-
{ id: 'start', type: 'start', config: { objectName: 'crm_lead', triggerType: 'record-created' } },
250+
{ id: 'start', type: 'start', config: { objectName: 'crm_lead', triggerType: 'record-after-create' } },
251251
{ id: 'c1', type, config },
252252
],
253253
},
@@ -330,7 +330,7 @@ describe('validateFlowTemplatePaths', () => {
330330
{
331331
id: 'start',
332332
type: 'start',
333-
config: { objectName: 'crm_lead', triggerType: 'record-created', expand: ['crm_account'] },
333+
config: { objectName: 'crm_lead', triggerType: 'record-after-create', expand: ['crm_account'] },
334334
},
335335
{
336336
id: 'c1',
@@ -360,7 +360,7 @@ describe('validateFlowTemplatePaths', () => {
360360
name: 'guarded',
361361
type: 'record_change',
362362
nodes: [
363-
{ id: 'start', type: 'start', config: { objectName: 'crm_lead', triggerType: 'record-created' } },
363+
{ id: 'start', type: 'start', config: { objectName: 'crm_lead', triggerType: 'record-after-create' } },
364364
{ id: 'guard', type: 'try_catch', label: 'Guard', config: container },
365365
],
366366
},
@@ -399,7 +399,7 @@ describe('validateFlowTemplatePaths', () => {
399399
name: 'looped',
400400
type: 'record_change',
401401
nodes: [
402-
{ id: 'start', type: 'start', config: { objectName: 'crm_lead', triggerType: 'record-created' } },
402+
{ id: 'start', type: 'start', config: { objectName: 'crm_lead', triggerType: 'record-after-create' } },
403403
{
404404
id: 'each',
405405
type: 'loop',

‎packages/metadata-protocol/src/protocol.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10163,10 +10163,38 @@ export class ObjectStackProtocolImplementation implements
1016310163
reverted.push({ type: it.type, name: it.name, action: 'removed' });
1016410164
} else if (it.prevVersion !== null && it.prevVersion !== undefined) {
1016510165
// Edited an existing artifact → restore the pre-commit body.
10166+
//
10167+
// [#6563] The write INTENT is derived per item, exactly as the
10168+
// sibling caller {@link rollbackMetaItem} derives it. Left
10169+
// unstated, `SysMetadataRepository.restoreVersion` defaults to
10170+
// `'override-artifact'` and `put`'s `assertAllowed` refuses every
10171+
// type that is not `allowOrgOverride` — `object` among them — so
10172+
// each `object` item of a reverted commit came back in `failed[]`
10173+
// as `NOT_OVERRIDABLE` while the same edit reverted fine one
10174+
// artifact at a time through the version-history revert. The
10175+
// repository's default is right for callers that genuinely mean
10176+
// "override a packaged artifact"; the defect was this caller never
10177+
// saying which of the two cases it is.
10178+
//
10179+
// Per ITEM, not per call: `revertCommit` reverts a batch, and a
10180+
// commit routinely mixes a runtime-created object with an overlay
10181+
// on a packaged view. A genuinely artifact-backed item still
10182+
// resolves to `'override-artifact'` and is still refused — the
10183+
// derivation states the case, it does not widen the gate.
10184+
//
10185+
// Two neighbours are deliberately NOT changed here, each filed
10186+
// with its own measurement: the soft-remove limb above states the
10187+
// same intent as a CONSTANT, so a commit that CREATED an object
10188+
// still cannot be reverted (#6620); and neither limb refreshes the
10189+
// SchemaRegistry the way `rollbackMetaItem` does, so a restored
10190+
// body is persisted but not yet dispatched on (#6621).
10191+
const intent: 'override-artifact' | 'runtime-only' =
10192+
this.isArtifactBacked(it.type, it.name) ? 'override-artifact' : 'runtime-only';
1016610193
await repo.restoreVersion(ref, it.prevVersion, {
1016710194
actor,
1016810195
source: 'protocol.revertCommit',
1016910196
message: `revert commit ${request.commitId}`,
10197+
intent,
1017010198
});
1017110199
reverted.push({ type: it.type, name: it.name, action: 'restored' });
1017210200
}

0 commit comments

Comments
 (0)