diff --git a/.github/workflows/publish-preview.yml b/.github/workflows/publish-preview.yml
index ad3a049d3..e2828d73e 100644
--- a/.github/workflows/publish-preview.yml
+++ b/.github/workflows/publish-preview.yml
@@ -1,12 +1,15 @@
name: Publish preview packages
# Publishes a throwaway PREVIEW (prerelease) of ALL publishable packages, in
-# lockstep, only when the feature branch has an open PR — so the
-# published-package path can be tested without cutting a real `latest` release.
+# lockstep, on every push to a feature branch — so the published-package path
+# can be tested without cutting a real `latest` release.
#
# See docs/plans/2026-07-14-preview-package-publishing.md (in etendo_schema_forge).
#
-# - Trigger: push to feature/** (auto) + manual workflow_dispatch, gated by PR.
+# - Trigger: push to feature/** (auto) + manual workflow_dispatch. NOT gated
+# by an open PR: the consumer is `make bump-core-version`, which needs only
+# the published version, and a branch is often worth previewing before it is
+# ready for review. An open PR only adds the sticky comment.
# - Version: -preview.... (D3)
# - dist-tag: alpha — never `latest`, so consumers are unaffected. (D2)
# - Scope: the 6 publishable packages, lockstep. (D6)
@@ -31,8 +34,10 @@ concurrency:
cancel-in-progress: true
jobs:
+ # Looks up the branch's open PR, if any. This does NOT gate publishing — its
+ # only consumer is the sticky-comment step, which needs somewhere to post.
find-pr:
- name: Check for an open feature PR
+ name: Look up the branch's PR (optional)
runs-on: ubuntu-latest
outputs:
number: ${{ steps.pr.outputs.number }}
@@ -47,14 +52,15 @@ jobs:
--json number --jq '.[0].number // empty')
echo "number=$PR" >> "$GITHUB_OUTPUT"
if [ -z "$PR" ]; then
- echo "No open PR for $BRANCH — preview publication is skipped."
+ echo "No open PR for $BRANCH — publishing anyway, without a PR comment."
+ echo "Read the version off the 'preview-package' commit status or this run's log."
else
echo "Open PR #$PR found for $BRANCH."
fi
preview:
+ # `needs` only to read the PR number for the comment step — never to gate.
needs: find-pr
- if: needs.find-pr.outputs.number != ''
name: Build, test & publish preview
runs-on: ubuntu-latest
steps:
@@ -132,13 +138,14 @@ jobs:
# --- Make the published version "visible fácil" -----------------------
# A) Commit status: pins "alpha: " to this SHA, so it shows as a
# check on the commit AND in the PR's checks list — no need to open the
- # run to see what was published. Links back to this run.
- # B) PR comment: post a single fresh comment on the PR that gated publishing.
+ # run to see what was published. Links back to this run. This is the
+ # ONLY surface that always exists, so it carries the no-PR case.
+ # B) PR comment: when the branch has an open PR, post a single fresh comment
# with the version and the exact pin snippet. On each push the previous
# bot comment (matched by marker) is deleted and a NEW one is posted, so
# it always lands at the bottom of the PR and notifies subscribers
- # (editing in place would leave it buried and silent). Skipped silently
- # when there is no open PR (the find-pr job skips this entire job).
+ # (editing in place would leave it buried and silent). Skipped when there
+ # is no PR yet — publishing still happened; read the version off (A).
# Both are fail-soft: a comment/status hiccup must never fail the publish.
- name: Pin published version to the commit (status)
continue-on-error: true
@@ -153,6 +160,7 @@ jobs:
-f target_url="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}"
- name: Comment published version on the open PR (sticky)
+ if: needs.find-pr.outputs.number != ''
continue-on-error: true
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
diff --git a/cli/src/lib/entity-methods.js b/cli/src/lib/entity-methods.js
index 1230eea63..9136925f3 100644
--- a/cli/src/lib/entity-methods.js
+++ b/cli/src/lib/entity-methods.js
@@ -269,6 +269,50 @@ export function applyMethodsToCrudPrediction(crud, methods, opts = {}) {
return crud;
}
+/**
+ * Is this AD tab's entity absent from the contract — i.e. did a human write
+ * `entities..exclude: true` in `decisions.json`?
+ *
+ * ETP-4793 — `exclude: true` had exactly one consumer, `resolve-curated.js`
+ * (`if (entityDecision.exclude === true) continue;`), so its whole effect was
+ * "absent from `contract.json`". Neither write path acted on it:
+ * `populateWindowSpec` walks `AD_Tab`, not the contract, so the entity still got
+ * an `ETGO_SF_ENTITY` row plus one `ETGO_SF_FIELD` row per AD column at the
+ * populate-step default `ISINCLUDED = 'Y'`, and nothing downstream closed them —
+ * `stepExcludeNonContractFields` compares column names against a FLAT set
+ * gathered across all contract entities, so an excluded entity's column survives
+ * whenever a contract entity happens to have a column of the same name.
+ *
+ * The measured consequence on the reference instance: 102 entities marked
+ * `exclude: true` across 28 specs, 90 of them present in `ETGO_SF_ENTITY`, 386
+ * of their field rows still `ISINCLUDED = 'Y'` — and one of them (`taxZone`)
+ * served with MORE verbs than the curated `tax` entity beside it, because a tab
+ * with no contract entity falls through to the window-level default while the
+ * curated entity is restricted by its own contract.
+ *
+ * Both write paths now consult this predicate and write `ISINCLUDED = 'N'` on
+ * the entity row, which is the lever every reader already honours: 24 call sites
+ * across the REST and MCP surfaces filter on `SFEntity.PROPERTY_ISINCLUDED`
+ * before touching an entity (`findIncludedEntity`, `listIncludedEntities`,
+ * `NeoServlet`, `NeoDiscoveryHelper`, …), and entity resolution always precedes
+ * field access. No new column, and no AD records, were needed.
+ *
+ * The empty-set guard is load-bearing: a contract with no `backendContract`
+ * entities at all (an older or partially-generated artifact) must answer `false`
+ * for every entity, or a single malformed contract would close an entire window.
+ *
+ * @param {object|null|undefined} contract - parsed contract.json
+ * @param {string} entityName - the NAME written to the ETGO_SF_ENTITY row
+ * @returns {boolean} true when the entity must be closed on both surfaces
+ */
+export function isEntityExcludedFromContract(contract, entityName) {
+ const entities = contract?.backendContract?.entities;
+ if (!entities || !entityName) return false;
+ const names = Object.keys(entities);
+ if (names.length === 0) return false;
+ return !names.includes(entityName);
+}
+
/**
* Read the resolved method list for an entity back off a generated contract.
* This is the ONLY function the two write paths (`push-to-neo.js` and
@@ -278,7 +322,11 @@ export function applyMethodsToCrudPrediction(crud, methods, opts = {}) {
* 1. `apiPrediction.crud..methods` — the resolved allowlist
* 2. `apiPrediction.window.readOnly` — window-level default, which also covers
* AD tabs that have no contract entity at all (excluded entities still get
- * an ETGO_SF_ENTITY row from `populateWindowSpec`)
+ * an ETGO_SF_ENTITY row from `populateWindowSpec`). Those rows now carry
+ * `ISINCLUDED = 'N'` — see `isEntityExcludedFromContract()` — so the method
+ * flags resolved here are unreachable for them, and are deliberately left
+ * at the window default rather than zeroed: an all-`N` set would break the
+ * GET/GETBYID invariant this file enforces.
* 3. all six methods — pre-ETP-4254 behaviour, unchanged
*
* ETP-4745 — `crud..delete === false` (set by `window.hideDelete` /
diff --git a/cli/src/lib/field-visibility.js b/cli/src/lib/field-visibility.js
new file mode 100644
index 000000000..9ad79b7dc
--- /dev/null
+++ b/cli/src/lib/field-visibility.js
@@ -0,0 +1,96 @@
+/**
+ * field-visibility.js — The single source of truth for the curated-visibility →
+ * NEO-flag projection.
+ *
+ * `ETGO_SF_FIELD` stores the same decision twice, on purpose:
+ * - `ISINCLUDED` / `ISREADONLY` — the two booleans NEO's runtime enforces.
+ * - `VISIBILITY` — the curated value verbatim, which `neo_schema`
+ * hands to agents (`system` and `readOnly` both
+ * collapse to Y/Y, so the flags cannot recover it).
+ *
+ * Storing a decision twice means it can disagree with itself. ETP-4793 / IMP-26
+ * found exactly that: `populateSpec` writes the two flags unconditionally but
+ * `VISIBILITY` only on some paths, so rows accumulate open flags with no
+ * curated value. `visibilityMatchesFlags()` is the predicate that detects it,
+ * and validator rule F23 is what runs it.
+ *
+ * Before ETP-4793 this function existed twice — exported from `push-to-neo.js`
+ * and inlined into `lib/neo-delta.js` to dodge a circular import. Both now
+ * import it from here. Do not add a third copy; a validator that re-implements
+ * the projection cannot detect a drift in the projection.
+ */
+
+/**
+ * Map a curated field visibility value to the NEO flag pair.
+ *
+ * Any value outside the curated set — including `null`, `undefined` and the
+ * empty string — is treated as `discarded` (closed). That default is what makes
+ * an absent `VISIBILITY` column coherent with `N`/`N` rather than a violation.
+ *
+ * @param {string|null|undefined} visibility - `editable` | `readOnly` | `system` | `discarded`
+ * @returns {{ isIncluded: 'Y'|'N', isReadOnly: 'Y'|'N' }}
+ */
+export function mapVisibility(visibility) {
+ switch (visibility) {
+ case 'editable':
+ return { isIncluded: 'Y', isReadOnly: 'N' };
+ case 'readOnly':
+ return { isIncluded: 'Y', isReadOnly: 'Y' };
+ case 'system':
+ return { isIncluded: 'Y', isReadOnly: 'Y' };
+ case 'discarded':
+ return { isIncluded: 'N', isReadOnly: 'N' };
+ default:
+ return { isIncluded: 'N', isReadOnly: 'N' };
+ }
+}
+
+/**
+ * The four curated visibility values, in the order the docs present them.
+ */
+export const CURATED_VISIBILITIES = Object.freeze(['editable', 'readOnly', 'system', 'discarded']);
+
+/**
+ * True when `visibility` is one of the four curated values. An absent or empty
+ * value is NOT curated — it is the "never written" state F23 reports separately
+ * from a genuine contradiction.
+ *
+ * @param {string|null|undefined} visibility
+ * @returns {boolean}
+ */
+export function isCuratedVisibility(visibility) {
+ return CURATED_VISIBILITIES.includes(visibility);
+}
+
+/**
+ * Check one stored row against the projection.
+ *
+ * Returns a verdict rather than a boolean because the two failure modes need
+ * different treatment (see F23 in docs/pipeline-validator-reference.md):
+ *
+ * - `contradiction` — `VISIBILITY` holds a curated value whose projection
+ * disagrees with the stored flags. Only a writer bug or a hand-edit can
+ * produce this, so it blocks.
+ * - `unwritten` — `VISIBILITY` is absent/empty while the flags say the field
+ * is included. Coherent for a closed field (`N`/`N` IS the default
+ * projection), a backfill gap for an open one, so it warns.
+ *
+ * @param {{ visibility?: string|null, isIncluded?: string, isReadOnly?: string }} row
+ * @returns {{ ok: boolean, kind: 'ok'|'contradiction'|'unwritten',
+ * expected: { isIncluded: string, isReadOnly: string } }}
+ */
+export function visibilityMatchesFlags(row) {
+ const visibility = row?.visibility ?? null;
+ // Absent flags default to the closed pair, matching mapVisibility's default —
+ // a row that omits both columns is coherent with an omitted VISIBILITY.
+ const isIncluded = row?.isIncluded ?? 'N';
+ const isReadOnly = row?.isReadOnly ?? 'N';
+ const expected = mapVisibility(visibility);
+ const ok = expected.isIncluded === isIncluded && expected.isReadOnly === isReadOnly;
+ if (ok) return { ok: true, kind: 'ok', expected };
+ return {
+ ok: false,
+ kind: isCuratedVisibility(visibility) ? 'contradiction' : 'unwritten',
+ expected,
+ };
+}
diff --git a/cli/src/lib/neo-delta.js b/cli/src/lib/neo-delta.js
index f4ecc1fc5..d420bc1e5 100644
--- a/cli/src/lib/neo-delta.js
+++ b/cli/src/lib/neo-delta.js
@@ -39,23 +39,16 @@
import { newEtendoId } from './etendo-uuid.js';
import { indexByNaturalKey } from './etgo-xml-parser.js';
-import { methodsToXmlFlags, resolveContractEntityMethods } from './entity-methods.js';
-
-/**
- * Local copy of mapVisibility() from push-to-neo.js. Inlined to keep this
- * module free of circular imports (push-to-neo imports computeWindowDelta).
- * If you change one, mirror the change in the other — both are intentionally
- * tiny so divergence is easy to spot.
- */
-function mapVisibility(visibility) {
- switch (visibility) {
- case 'editable': return { isIncluded: 'Y', isReadOnly: 'N' };
- case 'readOnly': return { isIncluded: 'Y', isReadOnly: 'Y' };
- case 'system': return { isIncluded: 'Y', isReadOnly: 'Y' };
- case 'discarded': return { isIncluded: 'N', isReadOnly: 'N' };
- default: return { isIncluded: 'N', isReadOnly: 'N' };
- }
-}
+import {
+ isEntityExcludedFromContract,
+ methodsToXmlFlags,
+ resolveContractEntityMethods,
+} from './entity-methods.js';
+// ETP-4793 — was an inlined copy (to dodge the push-to-neo → neo-delta import
+// cycle). Now a sibling in lib/, so there is no cycle and no second copy: the
+// live push, this offline projection and validator rule F23 all read the same
+// function.
+import { mapVisibility } from './field-visibility.js';
function normalizeAgentPrompt(value) {
if (value == null) return null;
@@ -65,8 +58,10 @@ function normalizeAgentPrompt(value) {
/**
* Local copy of normalizePreconditions() from push-to-neo.js. Inlined for the
- * same no-circular-import reason as mapVisibility/normalizeAgentPrompt — mirror
- * any change in the other. An explicit but empty declaration collapses to null
+ * same no-circular-import reason as normalizeAgentPrompt — mirror any change in
+ * the other. (mapVisibility used to be on this list; ETP-4793 moved it to
+ * lib/field-visibility.js instead, which is the better fix when a helper is
+ * needed by a third caller.) An explicit but empty declaration collapses to null
* so a stale DB value gets cleared (ETP-4275).
*/
function normalizePreconditions(value) {
@@ -249,6 +244,10 @@ export function computeWindowDelta(args) {
// Map from naturalKey → spec/entity/field UUID (for deterministic FKs).
const entityIdByNK = new Map();
+ // ETP-4793 — entity UUIDs whose contract entity is absent (`exclude: true`).
+ // Their field rows are closed too, mirroring `populateWindowSpec`.
+ const closedEntityIds = new Set();
+
let entitySeq = 0;
for (const tab of adTabs) {
entitySeq++;
@@ -273,6 +272,12 @@ export function computeWindowDelta(args) {
|| desiredEntityByTabName.get(tab.name)
|| tab.name;
+ // ETP-4793 — mirror the live push: a tab the contract does not declare
+ // (`exclude: true`) is closed on the entity row. The live counterpart is
+ // `buildEntityMethodFlagsResolver` → `upsertEntity({ isIncluded: 'N' })`.
+ const entityExcluded = isEntityExcludedFromContract(contract, entityName);
+ if (entityExcluded) closedEntityIds.add(entityId);
+
const entityRow = {
_naturalKey: entityNK,
ETGO_SF_ENTITY_ID: entityId,
@@ -281,7 +286,7 @@ export function computeWindowDelta(args) {
AD_MODULE_ID: moduleId,
NAME: entityName,
SEQNO: String(entitySeq * 10),
- ISINCLUDED: 'Y',
+ ISINCLUDED: entityExcluded ? 'N' : 'Y',
// ETP-4254 — the CRUD flags are NO LONGER hardcoded to 'Y'. push-to-neo's
// stepPopulateSpec hands populateWindowSpec a per-tab resolver built from
// the SAME contract value read here (see buildEntityMethodFlagsResolver),
@@ -323,11 +328,20 @@ export function computeWindowDelta(args) {
entityIdByNK,
prevFieldByNatural,
fieldUpserts,
+ closedEntityIds,
});
// ---- Mirror live-push behaviour: never create NEW records with ISINCLUDED=N
// stepExcludeNonContractFields only updates EXISTING rows; it never inserts
// a fresh ETGO_SF_FIELD record for a field that was never in the DB.
+ //
+ // ETP-4793 — this also prunes the field rows of a NEWLY closed entity, while
+ // the live `populateWindowSpec` does insert them with ISINCLUDED='N'. That gap
+ // is pre-existing and unchanged in kind (it already applied to every
+ // non-contract field of a brand-new window); on the reference instance all 90
+ // closed entities already have their rows in the prev snapshot, so they are
+ // kept and flipped. Widening the rule was deliberately left out of this change
+ // rather than folded in unmeasured.
const prunedFieldUpserts = fieldUpserts.filter(
f => prevFieldByNatural.has(f._naturalKey) || f.ISINCLUDED === 'Y',
);
@@ -563,6 +577,7 @@ function applyContractVisibilityToFields({
entityIdByNK,
prevFieldByNatural,
fieldUpserts,
+ closedEntityIds = new Set(),
}) {
const contractFields = extractFieldsFromContract(contract.backendContract);
const fieldDefaultExprs = buildFieldDefaultExprMap(decisions);
@@ -595,6 +610,15 @@ function applyContractVisibilityToFields({
}
for (const row of fieldUpserts) {
+ // ETP-4793 — a field of an entity the contract does not declare is closed
+ // outright, before the flat-column-set rule below gets a say. That rule is
+ // entity-blind on purpose (it mirrors `stepExcludeNonContractFields`
+ // bug-for-bug), which is exactly why an excluded entity's `Name` column used
+ // to survive: some OTHER entity's contract has a column called `Name`.
+ if (closedEntityIds.has(row.ETGO_SF_ENTITY_ID)) {
+ row.ISINCLUDED = 'N';
+ continue;
+ }
const colHit = contractFieldByEntityAndColumn.get(
`${row.ETGO_SF_ENTITY_ID}/${String(row.AD_COLUMN_ID)}`,
);
diff --git a/cli/src/neo-writer.js b/cli/src/neo-writer.js
index 9e53e58f3..fbb91e487 100644
--- a/cli/src/neo-writer.js
+++ b/cli/src/neo-writer.js
@@ -32,6 +32,39 @@ const SYSTEM_COLUMNS = [
'updatedby',
];
+/**
+ * The four field visibility values of the curated schema, as stored in
+ * ETGO_SF_FIELD.VISIBILITY (VARCHAR(20)) and read back by the MCP layer
+ * (McpSchemaFieldBuilder) to emit `visibility` / `userRequired` on neo_schema.
+ *
+ * Kept as data rather than validated inline so the set has exactly one
+ * definition on the writer side. NULL is also legal and means "not classified":
+ * populateSpec creates a row per AD column before any contract is applied, and
+ * columns the contract never mentions legitimately stay NULL.
+ */
+export const FIELD_VISIBILITIES = ['editable', 'readOnly', 'system', 'discarded'];
+
+/**
+ * Validate a visibility value on its way to the DB.
+ *
+ * Fails loudly on an unknown value instead of storing it: the MCP layer serves
+ * this column to agents as authoritative metadata, so a typo silently persisted
+ * here becomes a wrong instruction there.
+ *
+ * @param {string|null|undefined} visibility
+ * @returns {string|null} the value, or null when unclassified
+ */
+export function normalizeVisibility(visibility) {
+ if (visibility == null || visibility === '') return null;
+ if (!FIELD_VISIBILITIES.includes(visibility)) {
+ throw new Error(
+ `upsertField: invalid visibility "${visibility}" `
+ + `(expected one of ${FIELD_VISIBILITIES.join(', ')}, or null)`,
+ );
+ }
+ return visibility;
+}
+
/**
* Generate an Etendo-compatible UUID (32-char uppercase hex, no dashes).
*/
@@ -243,6 +276,10 @@ export async function upsertEntity(client, params) {
* @param {string} [params.fieldId] - If provided, UPDATE instead of INSERT
* @param {string} [params.isIncluded='Y']
* @param {string} [params.isReadOnly='N']
+ * @param {string} [params.visibility] - Curated visibility (see FIELD_VISIBILITIES).
+ * Orthogonal to isIncluded/isReadOnly, which drive NEO runtime behaviour: this
+ * column is agent-facing metadata only, and preserves the distinction those two
+ * booleans collapse (`system` and `readOnly` share the same Y/Y pair).
* @param {string} [params.defaultValue]
* @param {string} [params.agentPrompt] - Per-field agent guidance for neo_schema
* @param {string} [params.javaQualifier]
@@ -300,6 +337,10 @@ export async function upsertField(client, params) {
setClauses.push(`isbusinesscritical = $${paramIndex++}`);
values.push(params.isBusinessCritical ?? 'N');
}
+ if ('visibility' in params) {
+ setClauses.push(`visibility = $${paramIndex++}`);
+ values.push(normalizeVisibility(params.visibility));
+ }
setClauses.push(`updated = $${paramIndex++}`);
values.push(auditVals.updated);
@@ -319,6 +360,7 @@ export async function upsertField(client, params) {
const javaQualifier = params.javaQualifier ?? null;
const seqNo = params.seqNo ?? null;
const agentPrompt = params.agentPrompt ?? null;
+ const visibility = normalizeVisibility(params.visibility);
const fieldId = generateId();
await client.query(
@@ -326,13 +368,13 @@ export async function upsertField(client, params) {
(etgo_sf_field_id, etgo_sf_entity_id, ad_column_id, ad_module_id,
isincluded, isreadonly, isbusinesscritical, defaultvalue, java_qualifier, seqno,
ad_client_id, ad_org_id, isactive, created, createdby, updated, updatedby,
- agent_prompt)
- VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18)`,
+ agent_prompt, visibility)
+ VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19)`,
[fieldId, entityId, columnId, moduleId,
isIncluded, isReadOnly, isBusinessCritical, defaultValue, javaQualifier, seqNo,
auditVals.ad_client_id, auditVals.ad_org_id, auditVals.isactive,
auditVals.created, auditVals.createdby, auditVals.updated, auditVals.updatedby,
- agentPrompt],
+ agentPrompt, visibility],
);
return { fieldId, created: true };
}
@@ -498,12 +540,18 @@ async function populateWindowSpec(client, { specId, windowId, moduleId, excludeS
// flag to 'N' when not supplied, so an entity must ALWAYS receive an explicit
// set: the old `includeAllMethods: false` path left entities with no read
// access, which is never a legitimate outcome.
+ //
+ // ETP-4793 — the resolver may also return `isIncluded: 'N'` for a tab the
+ // contract does not declare (`exclude: true`). The default resolver keeps
+ // `upsertEntity`'s own `'Y'` default, so callers that pass no resolver are
+ // unaffected.
const resolveMethodFlags = (tab) => (
methodFlagsFor ? methodFlagsFor(tab) : { ...ALL_METHOD_FLAGS }
);
let entityCount = 0;
let fieldCount = 0;
+ let closedEntityCount = 0;
const entities = [];
const changes = {
entities: { created: 0, updated: 0, deleted: 0 },
@@ -519,6 +567,17 @@ async function populateWindowSpec(client, { specId, windowId, moduleId, excludeS
|| existingEntityByName.get(tab.name)
|| null;
+ const entityFlags = resolveMethodFlags(tab);
+ // ETP-4793 — an entity the contract does not declare is closed, and so are
+ // its fields. Closing the entity alone would suffice for behaviour (every
+ // reader filters ETGO_SF_ENTITY.ISINCLUDED before it ever reaches a field),
+ // but leaving 15 field rows per closed entity claiming ISINCLUDED='Y' makes
+ // the data lie to anyone counting the agent surface — which is how the gap
+ // went unnoticed. `visibility` is deliberately NOT written here: the offline
+ // XML delta does not model that column at all (IMP-26 §4.2), and NULL next
+ // to 'N'/'N' is already the coherent pair under `mapVisibility`.
+ const entityIsClosed = entityFlags.isIncluded === 'N';
+ if (entityIsClosed) closedEntityCount++;
const { entityId, created: entityCreated } = await upsertEntity(client, {
specId,
tabId: tab.ad_tab_id,
@@ -526,7 +585,7 @@ async function populateWindowSpec(client, { specId, windowId, moduleId, excludeS
name: tab.name,
seqNo: entitySeqNo,
entityId: existingEntityId,
- ...resolveMethodFlags(tab),
+ ...entityFlags,
audit,
});
entityCount++;
@@ -576,6 +635,7 @@ async function populateWindowSpec(client, { specId, windowId, moduleId, excludeS
moduleId,
fieldId: existingFieldId,
seqNo: fieldSeqCounter * 10,
+ ...(entityIsClosed ? { isIncluded: 'N' } : {}),
audit,
});
fieldCount++;
@@ -591,7 +651,7 @@ async function populateWindowSpec(client, { specId, windowId, moduleId, excludeS
// Delete stale entities (exist in DB but tab no longer in AD)
await deleteStaleEntities(existingEntityByTab, visitedEntityIds, client, changes);
- return { entityCount, fieldCount, entities, changes };
+ return { entityCount, fieldCount, closedEntityCount, entities, changes };
}
async function deleteDuplicateEntities(existingEntityResult, entityId, client, changes) {
diff --git a/cli/src/push-to-neo.js b/cli/src/push-to-neo.js
index e583f0d6f..a41e98639 100755
--- a/cli/src/push-to-neo.js
+++ b/cli/src/push-to-neo.js
@@ -24,8 +24,14 @@ import {
import { computeWindowDelta, serializeDelta } from './lib/neo-delta.js';
import { resolveAgentPromptRefs } from './lib/agent-prompt-ref.js';
import { loadEtgoXmlSnapshot } from './lib/etgo-xml-parser.js';
+import { mapVisibility } from './lib/field-visibility.js';
import { GO_MODULE_ID } from './lib/constants.js';
-import { methodsToWriterFlags, NEO_HTTP_METHODS, resolveContractEntityMethods } from './lib/entity-methods.js';
+import {
+ isEntityExcludedFromContract,
+ methodsToWriterFlags,
+ NEO_HTTP_METHODS,
+ resolveContractEntityMethods,
+} from './lib/entity-methods.js';
import { isMainModule } from './utils.js';
const __filename = fileURLToPath(import.meta.url);
@@ -52,21 +58,12 @@ export function toSpecName(windowName) {
/**
* Map a field visibility value to NEO params.
* Returns { isIncluded: "Y"|"N", isReadOnly: "Y"|"N" }.
+ *
+ * ETP-4793 — the implementation moved to `lib/field-visibility.js` so the
+ * offline delta path and validator rule F23 share it instead of re-declaring
+ * it. Re-exported here because this name is part of the module's public API.
*/
-export function mapVisibility(visibility) {
- switch (visibility) {
- case 'editable':
- return { isIncluded: 'Y', isReadOnly: 'N' };
- case 'readOnly':
- return { isIncluded: 'Y', isReadOnly: 'Y' };
- case 'system':
- return { isIncluded: 'Y', isReadOnly: 'Y' };
- case 'discarded':
- return { isIncluded: 'N', isReadOnly: 'N' };
- default:
- return { isIncluded: 'N', isReadOnly: 'N' };
- }
-}
+export { mapVisibility };
/**
* Build the full webhook URL from a base Etendo URL and webhook name.
@@ -286,7 +283,9 @@ export async function pushToNeo(windowName, options = {}) {
const allFields = extractFieldsFromContract(contract.backendContract);
if (options.dryRun === true) {
- return reportDryRunPlan({ allFields, specName, windowId, windowDisplayName, windowName, contract });
+ return reportDryRunPlan({
+ allFields, specName, windowId, windowDisplayName, windowName, contract, schemaRawData,
+ });
}
return executePushTransaction({
@@ -440,6 +439,11 @@ export function buildFieldUpdateParams(f, ctx, fieldId, entityId) {
moduleId: ctx.moduleId,
isIncluded: vis.isIncluded,
isReadOnly: vis.isReadOnly,
+ // Stored alongside — not instead of — the two booleans above. mapVisibility
+ // collapses four curated values into two flags, which is what NEO's runtime
+ // needs but loses the distinction agents are told to act on (`system` and
+ // `readOnly` both map to Y/Y). neo_schema reads this column verbatim.
+ visibility: f.visibility ?? null,
isBusinessCritical: f.businessCritical ? 'Y' : 'N',
audit: ctx.auditOpts,
};
@@ -477,17 +481,47 @@ function summarizeRestrictedEntityMethods(contract) {
return restricted;
}
-function reportDryRunPlan({ allFields, specName, windowId, windowDisplayName, windowName, contract }) {
+/**
+ * ETP-4793 — the entities the push would close (`ISINCLUDED = 'N'`) because the
+ * contract does not declare them. Read off `schemaRawData`, not the contract,
+ * precisely because these entities are the ones the contract is missing.
+ *
+ * @param {object} contract - parsed contract.json
+ * @param {object} schemaRawData - parsed schema-raw.json
+ * @returns {string[]} curated entity names, sorted
+ */
+function summarizeExcludedEntities(contract, schemaRawData) {
+ const names = (schemaRawData?.entities ?? [])
+ .map((ent) => ent.name)
+ .filter((name) => isEntityExcludedFromContract(contract, name));
+ return [...new Set(names)].sort((left, right) => left.localeCompare(right));
+}
+
+function reportDryRunPlan({
+ allFields, specName, windowId, windowDisplayName, windowName, contract, schemaRawData,
+}) {
const entityMethods = summarizeRestrictedEntityMethods(contract);
+ const excludedEntities = summarizeExcludedEntities(contract, schemaRawData);
const plan = {
spec: { action: 'upsertSpec', params: { windowId, name: specName, specType: 'W' } },
- populate: { action: 'populateSpec', params: { specId: '(from step 1)' }, entityMethods },
+ populate: {
+ action: 'populateSpec',
+ params: { specId: '(from step 1)' },
+ entityMethods,
+ excludedEntities,
+ },
fields: allFields.map(f => {
const vis = mapVisibility(f.visibility);
return {
action: 'upsertField',
entityName: f.entityName,
- params: { entityId: '(from populate)', column: f.column, isIncluded: vis.isIncluded, isReadOnly: vis.isReadOnly },
+ params: {
+ entityId: '(from populate)',
+ column: f.column,
+ isIncluded: vis.isIncluded,
+ isReadOnly: vis.isReadOnly,
+ visibility: f.visibility ?? null,
+ },
};
}),
};
@@ -509,6 +543,12 @@ function reportDryRunPlan({ allFields, specName, windowId, windowDisplayName, wi
console.log(` ${name}: ${entityMethods[name].join(', ')}`);
}
}
+ if (excludedEntities.length === 0) {
+ console.log(` Entities closed (exclude: true): none`);
+ } else {
+ console.log(` Entities closed (exclude: true) — ISINCLUDED='N' on entity + fields:`);
+ console.log(` ${excludedEntities.join(', ')}`);
+ }
console.log(`\n Step 3: ${plan.fields.length} field updates via upsertField`);
const included = plan.fields.filter(f => f.params.isIncluded === 'Y');
@@ -526,6 +566,7 @@ function reportDryRunPlan({ allFields, specName, windowId, windowDisplayName, wi
included: included.length,
excluded: excluded.length,
readOnly: readOnly.length,
+ excludedEntities,
},
};
}
@@ -599,12 +640,19 @@ async function stepUpsertSpec(client, ctx) {
* pure function of (contract, NAME), and NAME is already asserted equal between
* the two paths by the XML regeneration check.
*
- * A tab with no contract entity (excluded entity, extra AD tab) falls through to
- * the window-level default inside `resolveContractEntityMethods` — i.e. still
- * read-only for a `window.readOnly` window, all methods otherwise.
+ * ETP-4793 — a tab with no contract entity (`exclude: true` in `decisions.json`,
+ * or an extra AD tab) now also gets `isIncluded: 'N'`, which is what actually
+ * closes it on both the REST and MCP surfaces; see
+ * `isEntityExcludedFromContract()` for why that column and not a new one. Its
+ * method flags still fall through to the window-level default, deliberately —
+ * they are unreachable once the entity is not included, and zeroing GET would
+ * break the invariant `entity-methods.js` enforces.
+ *
+ * `isIncluded` is `'Y'` for every entity the contract does declare, i.e. the
+ * value `upsertEntity` already defaults to, so nothing changes for them.
*
* @param {object} ctx - push context carrying `schemaRawData` + `contract`
- * @returns {(tab: {name: string}) => object} writer method flags
+ * @returns {(tab: {name: string}) => object} writer entity flags, `isIncluded` included
*/
export function buildEntityMethodFlagsResolver({ schemaRawData, contract }) {
const byTabName = new Map();
@@ -613,7 +661,10 @@ export function buildEntityMethodFlagsResolver({ schemaRawData, contract }) {
}
return (tab) => {
const entityName = byTabName.get(tab?.name) ?? tab?.name;
- return methodsToWriterFlags(resolveContractEntityMethods(contract, entityName));
+ return {
+ isIncluded: isEntityExcludedFromContract(contract, entityName) ? 'N' : 'Y',
+ ...methodsToWriterFlags(resolveContractEntityMethods(contract, entityName)),
+ };
};
}
diff --git a/cli/src/validate-pipeline.js b/cli/src/validate-pipeline.js
index 2857cd07f..3f718501c 100644
--- a/cli/src/validate-pipeline.js
+++ b/cli/src/validate-pipeline.js
@@ -30,6 +30,8 @@ import {
resolveEntityHideDelete,
resolveEntityMethods,
} from './lib/entity-methods.js';
+import { parseEtgoXmlFile } from './lib/etgo-xml-parser.js';
+import { visibilityMatchesFlags } from './lib/field-visibility.js';
const execFileAsync = promisify(execFile);
@@ -807,6 +809,170 @@ async function ruleF22(artifactDir, artifactName) {
'Remove window.customTabsAfterBottom, or remove tabOrder from the listed custom tab(s).');
}
+// ─── F23 — ETGO_SF_FIELD visibility vs the flags it projects to ─────────────
+// ETP-4793 / IMP-26 §5.3. `populateSpec` writes ISINCLUDED/ISREADONLY on every
+// push but VISIBILITY only on some paths, so the same decision — stored twice
+// on purpose, because neo_schema needs the curated word and the runtime needs
+// the booleans — can drift apart silently. This rule reads the exported
+// sourcedata and re-runs the projection over it.
+
+/**
+ * Resolve the com.etendoerp.go sourcedata directory holding the exported
+ * ETGO_SF_*.xml files. Mirrors `resolveDefaultPrevXmlDir` in push-to-neo.js.
+ *
+ * @param {string} root - schema_forge repo root
+ * @returns {string}
+ */
+function resolveGoSourcedataDir(root) {
+ if (process.env.SF_GO_SOURCEDATA_DIR) return process.env.SF_GO_SOURCEDATA_DIR;
+ const etendoRoot = process.env.ETENDO_ROOT || join(root, '..');
+ return join(etendoRoot, 'modules', 'com.etendoerp.go', 'src-db', 'database', 'sourcedata');
+}
+
+// One 8 MB XML parse per process, not per artifact. Keyed by directory so a
+// test pointing at a fixture never reads the cached production snapshot.
+const f23SnapshotCache = new Map();
+
+/**
+ * Parse the three sourcedata XMLs and index the field rows by spec name.
+ *
+ * Returns `null` — not an empty index — when the directory is absent, so the
+ * caller can tell "no .go checkout here" apart from "checkout present, spec has
+ * no rows".
+ *
+ * @param {string} sourcedataDir
+ * @returns {Promise>|null>} spec name → field rows
+ */
+async function loadF23FieldsBySpec(sourcedataDir) {
+ if (f23SnapshotCache.has(sourcedataDir)) return f23SnapshotCache.get(sourcedataDir);
+ const pending = (async () => {
+ const paths = {
+ spec: join(sourcedataDir, 'ETGO_SF_SPEC.xml'),
+ entity: join(sourcedataDir, 'ETGO_SF_ENTITY.xml'),
+ field: join(sourcedataDir, 'ETGO_SF_FIELD.xml'),
+ };
+ const present = await Promise.all(Object.values(paths).map(fileExists));
+ if (present.some(ok => !ok)) return null;
+
+ const [specs, entities, fields] = await Promise.all([
+ parseEtgoXmlFile(paths.spec, 'ETGO_SF_SPEC').then(r => r.rows),
+ parseEtgoXmlFile(paths.entity, 'ETGO_SF_ENTITY').then(r => r.rows),
+ parseEtgoXmlFile(paths.field, 'ETGO_SF_FIELD').then(r => r.rows),
+ ]);
+
+ const specNameById = new Map(specs.map(s => [s.ETGO_SF_SPEC_ID, s.NAME]));
+ const entityById = new Map(entities.map(e => [e.ETGO_SF_ENTITY_ID, e]));
+
+ const bySpec = new Map();
+ for (const row of fields) {
+ if (row.ISACTIVE === 'N') continue; // retired config, not a live surface
+ const entity = entityById.get(row.ETGO_SF_ENTITY_ID);
+ const specName = entity && specNameById.get(entity.ETGO_SF_SPEC_ID);
+ if (!specName) continue; // orphan row — F23 is not the rule that owns that
+ if (!bySpec.has(specName)) bySpec.set(specName, []);
+ bySpec.get(specName).push({ row, entityName: entity.NAME });
+ }
+ return bySpec;
+ })();
+ f23SnapshotCache.set(sourcedataDir, pending);
+ return pending;
+}
+
+/**
+ * Human-readable identity for one offending field row.
+ *
+ * `JAVA_QUALIFIER` is only written when the curated key differs from the AD
+ * column name, so `AD_COLUMN_ID` is the usual identity. Some pushed rows carry
+ * neither — ETP-4793 found 105 field rows in the live export with no
+ * AD_COLUMN_ID, JAVA_QUALIFIER or SEQNO at all, just the two flags. Those are
+ * degenerate rows pointing at no AD column; label them by primary key and say
+ * so, because "entity.undefined" tells the reader nothing.
+ */
+function f23FieldLabel({ row, entityName }) {
+ const entity = entityName || '?';
+ const name = row.JAVA_QUALIFIER || row.AD_COLUMN_ID;
+ if (name) return `${entity}.${name}`;
+ return `${entity}.`;
+}
+
+/**
+ * F23: a pushed `ETGO_SF_FIELD` row whose `VISIBILITY` does not project to its
+ * stored `ISINCLUDED`/`ISREADONLY` pair.
+ *
+ * Two failure modes, deliberately scored differently:
+ * - BLOCK `contradiction` — VISIBILITY holds a curated value that projects to
+ * a different pair than the one stored. Only a writer bug or a hand-edit
+ * gets you here, and it means the runtime and `neo_schema` disagree about
+ * the same field.
+ * - WARN `unwritten` — VISIBILITY is absent while the flags say the field is
+ * included. Harmless to the runtime, but `neo_schema` reports no visibility
+ * for that field, so an agent cannot tell `readOnly` from `system`.
+ * Pre-existing backfill debt; a warning so it is counted, not so it blocks.
+ *
+ * Inert (returns null) without a com.etendoerp.go checkout — the exported XML is
+ * the only DB-free view of pushed state, and this repo can be used alone.
+ *
+ * @param {string} artifactDir
+ * @param {string} artifactName - artifact dir name === spec name
+ * @param {string} [sourcedataDir]
+ * @param {string} [root]
+ * @returns {Promise}
+ */
+async function ruleF23(artifactDir, artifactName, sourcedataDir, root = ROOT) {
+ const dir = sourcedataDir ?? resolveGoSourcedataDir(root);
+ const bySpec = await loadF23FieldsBySpec(dir);
+ if (bySpec === null) return null; // no runtime-module checkout — nothing to read
+ const rows = bySpec.get(artifactName);
+ if (!rows || rows.length === 0) return null; // never pushed, or a different spec name
+
+ const contradictions = [];
+ const unwritten = [];
+ for (const entry of rows) {
+ const verdict = visibilityMatchesFlags({
+ visibility: entry.row.VISIBILITY,
+ isIncluded: entry.row.ISINCLUDED,
+ isReadOnly: entry.row.ISREADONLY,
+ });
+ if (verdict.ok) continue;
+ (verdict.kind === 'contradiction' ? contradictions : unwritten).push({ entry, verdict });
+ }
+
+ if (contradictions.length > 0) {
+ const shown = contradictions.slice(0, 3).map(({ entry, verdict }) => (
+ `${f23FieldLabel(entry)} (visibility='${entry.row.VISIBILITY}' projects to `
+ + `ISINCLUDED=${verdict.expected.isIncluded}/ISREADONLY=${verdict.expected.isReadOnly}, `
+ + `stored ${entry.row.ISINCLUDED}/${entry.row.ISREADONLY})`
+ ));
+ const more = contradictions.length > shown.length ? ` (+${contradictions.length - shown.length} more)` : '';
+ return violation(
+ 'F23', artifactName, 'BLOCK',
+ `${contradictions.length} pushed ETGO_SF_FIELD row(s) store a VISIBILITY that contradicts their `
+ + `ISINCLUDED/ISREADONLY flags: ${shown.join('; ')}${more}. The runtime enforces the flags while `
+ + `neo_schema reports the visibility, so the two now describe the field differently.`,
+ `Re-push the window (make regen ONLY=${artifactName} PUSH_TO_NEO=1) and re-run `
+ + `./gradlew export.database in Etendo root. If the drift survives a clean push, the writer is `
+ + `at fault — fix cli/src/neo-writer.js populateSpec, not the XML.`,
+ { f23Contradictions: contradictions.length, f23Unwritten: unwritten.length },
+ );
+ }
+
+ if (unwritten.length > 0) {
+ const shown = unwritten.slice(0, 3).map(({ entry }) => f23FieldLabel(entry));
+ const more = unwritten.length > shown.length ? ` (+${unwritten.length - shown.length} more)` : '';
+ return violation(
+ 'F23', artifactName, 'WARN',
+ `${unwritten.length} pushed ETGO_SF_FIELD row(s) are included (ISINCLUDED=Y) but carry no `
+ + `VISIBILITY value: ${shown.join(', ')}${more}. neo_schema reports these fields with no `
+ + `visibility, so an agent cannot distinguish readOnly from system.`,
+ `Re-push the window (make regen ONLY=${artifactName} PUSH_TO_NEO=1) then `
+ + `./gradlew export.database — populateSpec writes VISIBILITY on the fields it revisits.`,
+ { f23Contradictions: 0, f23Unwritten: unwritten.length },
+ );
+ }
+
+ return null;
+}
+
/**
* Collect the field names declared on the line (non-header / detail) entity of
* the frontend contract. Real generated contracts populate
@@ -1798,7 +1964,7 @@ async function runEnabledChecks(checks, skipSet) {
return (await Promise.all(pendingChecks)).filter(Boolean);
}
-async function runWindowChecks(artifactDir, artifactName, registryContent, root, skipSet, f19Allowlist = []) {
+async function runWindowChecks(artifactDir, artifactName, registryContent, root, skipSet, goSourcedataDir, f19Allowlist = []) {
return runEnabledChecks([
{ rule: 'F1', run: () => ruleF1(artifactDir, artifactName) },
{ rule: 'F2', run: () => ruleF2(artifactDir, artifactName) },
@@ -1820,6 +1986,7 @@ async function runWindowChecks(artifactDir, artifactName, registryContent, root,
{ rule: 'F20', run: () => ruleF20(artifactDir, artifactName, root) },
{ rule: 'F21', run: () => ruleF21(artifactDir, artifactName) },
{ rule: 'F22', run: () => ruleF22(artifactDir, artifactName) },
+ { rule: 'F23', run: () => ruleF23(artifactDir, artifactName, goSourcedataDir, root) },
], skipSet);
}
@@ -1833,16 +2000,19 @@ function tagArtifactKind(results, artifactKind) {
return results.map(result => ({ ...result, artifactKind }));
}
-async function runAggregateSectionChecks(artifactDir, artifactName, skipSet) {
+async function runAggregateSectionChecks(artifactDir, artifactName, skipSet, root, goSourcedataDir) {
const f9Results = await runSingleCheck('F9', () => ruleF9(artifactDir, artifactName), skipSet);
const f4Results = await runSingleCheck('F4', () => ruleF4(artifactDir, artifactName), skipSet);
- return [...f9Results, ...f4Results];
+ const f23Results = await runSingleCheck(
+ 'F23', () => ruleF23(artifactDir, artifactName, goSourcedataDir, root), skipSet,
+ );
+ return [...f9Results, ...f4Results, ...f23Results];
}
-async function runChecksForArtifact({ kind, artifactDir, artifactName, registryContent, root, skipSet, strict, f19Allowlist }) {
+async function runChecksForArtifact({ kind, artifactDir, artifactName, registryContent, root, skipSet, strict, f19Allowlist, goSourcedataDir }) {
if (kind === 'window') {
return tagArtifactKind(
- await runWindowChecks(artifactDir, artifactName, registryContent, root, skipSet, f19Allowlist),
+ await runWindowChecks(artifactDir, artifactName, registryContent, root, skipSet, goSourcedataDir, f19Allowlist),
'window',
);
}
@@ -1853,14 +2023,19 @@ async function runChecksForArtifact({ kind, artifactDir, artifactName, registryC
);
}
if (kind === 'aggregate') {
- return tagArtifactKind(
- await runSingleCheck('F9', () => ruleF9(artifactDir, artifactName), skipSet),
- 'aggregate',
- );
+ // F23 runs here too: an aggregate artifact pushes its own ETGO_SF_SPEC, so
+ // its field rows can drift exactly like a window's. ETP-4793 measured 340 of
+ // the 409 incoherent rows in the live export inside two aggregates
+ // (return-to-vendor, return-from-customer) — registering F23 for windows
+ // only would have left 83 % of the defect invisible.
+ return tagArtifactKind([
+ ...await runSingleCheck('F9', () => ruleF9(artifactDir, artifactName), skipSet),
+ ...await runSingleCheck('F23', () => ruleF23(artifactDir, artifactName, goSourcedataDir, root), skipSet),
+ ], 'aggregate');
}
if (kind === 'aggregate-section') {
return tagArtifactKind(
- await runAggregateSectionChecks(artifactDir, artifactName, skipSet),
+ await runAggregateSectionChecks(artifactDir, artifactName, skipSet, root, goSourcedataDir),
'aggregate-section',
);
}
@@ -1956,6 +2131,10 @@ export async function validatePipeline({
root = ROOT,
registryPath,
f19AllowlistPath,
+ // F23 reads the exported ETGO_SF_*.xml from the com.etendoerp.go checkout.
+ // Injectable so tests point at a fixture instead of whatever module tree
+ // happens to sit beside the developer's repo.
+ goSourcedataDir,
_artifactsRoot,
} = {}) {
const artifactsRoot = _artifactsRoot ?? join(root, 'artifacts');
@@ -1982,6 +2161,7 @@ export async function validatePipeline({
skipSet,
strict,
f19Allowlist,
+ goSourcedataDir,
}));
}
diff --git a/cli/test/entity-methods-pipeline.test.js b/cli/test/entity-methods-pipeline.test.js
index 2f4fded48..7d54f1a5a 100644
--- a/cli/test/entity-methods-pipeline.test.js
+++ b/cli/test/entity-methods-pipeline.test.js
@@ -20,10 +20,13 @@ import { generateContract } from '../src/generate-contract.js';
import { buildEntityMethodFlagsResolver } from '../src/push-to-neo.js';
import { computeWindowDelta } from '../src/lib/neo-delta.js';
-const ALL_Y = { isGet: 'Y', isGetbyid: 'Y', isPost: 'Y', isPut: 'Y', isPatch: 'Y', isDelete: 'Y' };
-const READ_ONLY = { isGet: 'Y', isGetbyid: 'Y', isPost: 'N', isPut: 'N', isPatch: 'N', isDelete: 'N' };
-const XML_ALL_Y = { ISGET: 'Y', ISGETBYID: 'Y', ISPOST: 'Y', ISPUT: 'Y', ISPATCH: 'Y', ISDELETE: 'Y' };
-const XML_READ_ONLY = { ISGET: 'Y', ISGETBYID: 'Y', ISPOST: 'N', ISPUT: 'N', ISPATCH: 'N', ISDELETE: 'N' };
+// ETP-4793 — `isIncluded` / `ISINCLUDED` joined this projection: an entity the
+// contract DOES declare must stay included on both write paths, and the one it
+// does not must be closed on both (see the `exclude: true` test below).
+const ALL_Y = { isIncluded: 'Y', isGet: 'Y', isGetbyid: 'Y', isPost: 'Y', isPut: 'Y', isPatch: 'Y', isDelete: 'Y' };
+const READ_ONLY = { isIncluded: 'Y', isGet: 'Y', isGetbyid: 'Y', isPost: 'N', isPut: 'N', isPatch: 'N', isDelete: 'N' };
+const XML_ALL_Y = { ISINCLUDED: 'Y', ISGET: 'Y', ISGETBYID: 'Y', ISPOST: 'Y', ISPUT: 'Y', ISPATCH: 'Y', ISDELETE: 'Y' };
+const XML_READ_ONLY = { ISINCLUDED: 'Y', ISGET: 'Y', ISGETBYID: 'Y', ISPOST: 'N', ISPUT: 'N', ISPATCH: 'N', ISDELETE: 'N' };
// ---------------------------------------------------------------------------
// Fixture: a two-tab window, mirroring a monitor/log window's shape.
@@ -106,6 +109,7 @@ async function runChain(decisions) {
const xmlFlags = {};
for (const row of delta.tables.ETGO_SF_ENTITY.upserts) {
xmlFlags[row.NAME] = {
+ ISINCLUDED: row.ISINCLUDED,
ISGET: row.ISGET, ISGETBYID: row.ISGETBYID, ISPOST: row.ISPOST,
ISPUT: row.ISPUT, ISPATCH: row.ISPATCH, ISDELETE: row.ISDELETE,
};
@@ -117,6 +121,7 @@ async function runChain(decisions) {
/** Assert the live-DB flags and the predicted XML flags describe the same row. */
function assertPathsAgree(writerFlags, xmlFlags) {
const pairs = [
+ ['isIncluded', 'ISINCLUDED'],
['isGet', 'ISGET'], ['isGetbyid', 'ISGETBYID'], ['isPost', 'ISPOST'],
['isPut', 'ISPUT'], ['isPatch', 'ISPATCH'], ['isDelete', 'ISDELETE'],
];
@@ -199,10 +204,10 @@ describe('ETP-4254 — decisions → contract → both write paths', () => {
assert.deepEqual(crud.log.methods, ['GET', 'GETBYID', 'PUT', 'PATCH']);
assert.deepEqual(writerFlags.log, {
- isGet: 'Y', isGetbyid: 'Y', isPost: 'N', isPut: 'Y', isPatch: 'Y', isDelete: 'N',
+ isIncluded: 'Y', isGet: 'Y', isGetbyid: 'Y', isPost: 'N', isPut: 'Y', isPatch: 'Y', isDelete: 'N',
});
assert.deepEqual(xmlFlags.log, {
- ISGET: 'Y', ISGETBYID: 'Y', ISPOST: 'N', ISPUT: 'Y', ISPATCH: 'Y', ISDELETE: 'N',
+ ISINCLUDED: 'Y', ISGET: 'Y', ISGETBYID: 'Y', ISPOST: 'N', ISPUT: 'Y', ISPATCH: 'Y', ISDELETE: 'N',
});
assertPathsAgree(writerFlags, xmlFlags);
});
@@ -240,10 +245,52 @@ describe('ETP-4254 — decisions → contract → both write paths', () => {
}
});
- it('an AD tab with no contract entity still follows the window default', async () => {
+ // ETP-4793 — an AD tab with no contract entity is now CLOSED (ISINCLUDED='N'),
+ // which is the column every NEO/MCP reader filters on before it resolves an
+ // entity. Its method flags deliberately keep following the window default:
+ // they are unreachable once the entity is not included, and an all-'N' set
+ // would break the GET/GETBYID invariant `entity-methods.js` enforces.
+ it('an AD tab with no contract entity is closed, methods still the window default', async () => {
const { contract } = await runChain({ version: 'v2', window: { readOnly: true }, entities: {} });
const resolver = buildEntityMethodFlagsResolver({ schemaRawData: schemaRaw(), contract });
- assert.deepEqual(resolver({ ad_tab_id: 'T9', name: 'someExtraTab' }), READ_ONLY);
+ const flags = resolver({ ad_tab_id: 'T9', name: 'someExtraTab' });
+
+ assert.equal(flags.isIncluded, 'N', 'a tab absent from the contract must not be served');
+ assert.deepEqual(
+ { ...flags, isIncluded: 'Y' }, READ_ONLY,
+ 'the method flags themselves are untouched — GET/GETBYID stay granted',
+ );
+ });
+
+ it('exclude: true closes the entity on BOTH write paths', async () => {
+ const { contract, writerFlags, xmlFlags } = await runChain({
+ version: 'v2', window: {}, entities: { logLine: { exclude: true } },
+ });
+
+ assert.equal(
+ contract.backendContract.entities.logLine, undefined,
+ 'exclude: true keeps the entity out of the contract — the precondition being tested',
+ );
+ assert.equal(writerFlags.logLine.isIncluded, 'N', 'live push must close it');
+ assert.equal(xmlFlags.logLine.ISINCLUDED, 'N', 'XML delta must close the same row');
+ assert.equal(writerFlags.log.isIncluded, 'Y', 'the sibling the contract DOES declare stays open');
+ assert.equal(xmlFlags.log.ISINCLUDED, 'Y');
+ assertPathsAgree(writerFlags, xmlFlags);
+ });
+
+ // A contract that declares NO entities at all (an older or half-generated
+ // artifact) must not be read as "everything is excluded" — that would close a
+ // whole window on one malformed file.
+ it('an empty contract closes nothing', async () => {
+ const resolver = buildEntityMethodFlagsResolver({
+ schemaRawData: schemaRaw(), contract: { backendContract: { entities: {} } },
+ });
+ assert.equal(resolver({ ad_tab_id: 'T1', name: 'Conversion Rate Log' }).isIncluded, 'Y');
+ assert.equal(
+ buildEntityMethodFlagsResolver({ schemaRawData: schemaRaw(), contract: {} })(
+ { ad_tab_id: 'T1', name: 'Conversion Rate Log' },
+ ).isIncluded, 'Y',
+ );
});
// ETP-4745 — `hideDelete` previously only reached `apiPrediction.crud..delete`
@@ -258,11 +305,11 @@ describe('ETP-4254 — decisions → contract → both write paths', () => {
assert.equal(crud.log.delete, false);
assert.equal(crud.logLine.delete, true);
assert.deepEqual(writerFlags.log, {
- isGet: 'Y', isGetbyid: 'Y', isPost: 'Y', isPut: 'Y', isPatch: 'Y', isDelete: 'N',
+ isIncluded: 'Y', isGet: 'Y', isGetbyid: 'Y', isPost: 'Y', isPut: 'Y', isPatch: 'Y', isDelete: 'N',
});
assert.deepEqual(writerFlags.logLine, ALL_Y, 'sibling entity must keep DELETE');
assert.deepEqual(xmlFlags.log, {
- ISGET: 'Y', ISGETBYID: 'Y', ISPOST: 'Y', ISPUT: 'Y', ISPATCH: 'Y', ISDELETE: 'N',
+ ISINCLUDED: 'Y', ISGET: 'Y', ISGETBYID: 'Y', ISPOST: 'Y', ISPUT: 'Y', ISPATCH: 'Y', ISDELETE: 'N',
});
assert.deepEqual(xmlFlags.logLine, XML_ALL_Y);
assertPathsAgree(writerFlags, xmlFlags);
@@ -297,10 +344,10 @@ describe('ETP-4254 — decisions → contract → both write paths', () => {
'contract.methods still reflects the declared allowlist verbatim');
assert.equal(crud.log.delete, false);
assert.deepEqual(writerFlags.log, {
- isGet: 'Y', isGetbyid: 'Y', isPost: 'N', isPut: 'Y', isPatch: 'N', isDelete: 'N',
+ isIncluded: 'Y', isGet: 'Y', isGetbyid: 'Y', isPost: 'N', isPut: 'Y', isPatch: 'N', isDelete: 'N',
});
assert.deepEqual(xmlFlags.log, {
- ISGET: 'Y', ISGETBYID: 'Y', ISPOST: 'N', ISPUT: 'Y', ISPATCH: 'N', ISDELETE: 'N',
+ ISINCLUDED: 'Y', ISGET: 'Y', ISGETBYID: 'Y', ISPOST: 'N', ISPUT: 'Y', ISPATCH: 'N', ISDELETE: 'N',
});
assertPathsAgree(writerFlags, xmlFlags);
});
@@ -314,3 +361,110 @@ describe('ETP-4254 — decisions → contract → both write paths', () => {
assertPathsAgree(writerFlags, xmlFlags);
});
});
+
+// ---------------------------------------------------------------------------
+// ETP-4793 — the field-level half of `exclude: true`.
+//
+// Closing the entity row alone would be enough for behaviour (every reader
+// filters ETGO_SF_ENTITY.ISINCLUDED before it resolves a field), but the field
+// rows kept claiming ISINCLUDED='Y', which is how the gap stayed invisible: 386
+// such rows on the reference instance. The reason they survived is specific and
+// worth pinning — `stepExcludeNonContractFields` (and the delta that mirrors it
+// bug-for-bug) compares column NAMES against a FLAT set gathered across ALL
+// contract entities, so an excluded entity's column survives whenever a sibling
+// contract entity happens to have a column of the same name.
+//
+// The fixture below is built for exactly that: the excluded `logLine` tab gets a
+// `DocumentNo` column, which the contract's `log` entity also has.
+// ---------------------------------------------------------------------------
+
+const SPEC = 'conversion-rate-downloader-log';
+const SHARED_COL = { ad_column_id: 'C4', ad_table_id: 'TBL2', columnname: 'DocumentNo' };
+
+/**
+ * schemaRaw() with an explicit `editable` visibility on every field, plus a
+ * `DocumentNo` column on the second entity so it collides with the first's.
+ *
+ * The visibility matters: `mapVisibility(undefined)` is `N`/`N`, so the base
+ * fixture — which never asserted on field rows — would show every field closed
+ * and prove nothing.
+ */
+function schemaRawWithSharedColumn() {
+ const raw = schemaRaw();
+ for (const entity of raw.entities) {
+ for (const field of entity.fields) field.visibility = 'editable';
+ }
+ raw.entities[1].fields.push({
+ name: 'documentNo', columnName: 'DocumentNo', type: 'string', reference: 'String',
+ visibility: 'editable',
+ });
+ return raw;
+}
+
+/**
+ * A prev-XML snapshot holding the rows this window already has, so the delta's
+ * "never create NEW records with ISINCLUDED=N" pruning keeps them and the
+ * assertions can see the flip. That mirrors the live instance, where all 90
+ * closed entities already exist in the exported XML.
+ */
+function prevSnapshotFor(columns) {
+ const entityIdByTab = { T1: 'E1', T2: 'E2' };
+ const entityIdByTable = { TBL1: 'E1', TBL2: 'E2' };
+ return {
+ spec: [{ ETGO_SF_SPEC_ID: 'S1', NAME: SPEC }],
+ entity: AD_TABS.map(tab => ({
+ ETGO_SF_ENTITY_ID: entityIdByTab[tab.ad_tab_id],
+ ETGO_SF_SPEC_ID: 'S1',
+ AD_TAB_ID: tab.ad_tab_id,
+ })),
+ field: columns.map((col, i) => ({
+ ETGO_SF_FIELD_ID: `F${i + 1}`,
+ ETGO_SF_ENTITY_ID: entityIdByTable[col.ad_table_id],
+ AD_COLUMN_ID: col.ad_column_id,
+ })),
+ };
+}
+
+/** Compute the XML delta for a window whose second entity is `exclude: true`. */
+async function deltaWithExcludedLogLine() {
+ const raw = schemaRawWithSharedColumn();
+ const columns = [...AD_COLUMNS, SHARED_COL];
+ const decisions = { version: 'v2', window: {}, entities: { logLine: { exclude: true } } };
+ const { schema } = await resolveCurated(raw, { rules: [] }, decisions);
+ const contract = generateContract(schema, [], []);
+ const delta = computeWindowDelta({
+ specName: SPEC,
+ windowId: 'W001',
+ moduleId: 'AABBCCDD11223344',
+ contract,
+ decisions,
+ adTabs: AD_TABS,
+ adColumns: columns,
+ prevSnapshot: prevSnapshotFor(columns),
+ schemaRawData: raw,
+ });
+ const byColumn = {};
+ for (const row of delta.tables.ETGO_SF_FIELD.upserts) byColumn[row.AD_COLUMN_ID] = row;
+ return { contract, delta, byColumn };
+}
+
+describe('ETP-4793 — fields of an entity excluded from the contract', () => {
+ it('are closed even when a declared entity shares the column name', async () => {
+ const { contract, byColumn } = await deltaWithExcludedLogLine();
+
+ assert.equal(contract.backendContract.entities.logLine, undefined);
+ assert.equal(
+ byColumn.C4?.ISINCLUDED, 'N',
+ "logLine.DocumentNo must be closed — the flat column set alone would keep it 'Y' "
+ + 'because log.DocumentNo IS in the contract',
+ );
+ assert.equal(byColumn.C3?.ISINCLUDED, 'N', 'logLine.Rate is closed as well');
+ });
+
+ it('leaves the fields of the declared entity untouched', async () => {
+ const { byColumn } = await deltaWithExcludedLogLine();
+
+ assert.equal(byColumn.C1?.ISINCLUDED, 'Y', 'log.DocumentNo is contract-declared');
+ assert.equal(byColumn.C2?.ISINCLUDED, 'Y', 'log.Status is contract-declared');
+ });
+});
diff --git a/cli/test/field-visibility.test.js b/cli/test/field-visibility.test.js
new file mode 100644
index 000000000..333bf62e8
--- /dev/null
+++ b/cli/test/field-visibility.test.js
@@ -0,0 +1,97 @@
+import { describe, it } from 'node:test';
+import { strict as assert } from 'node:assert';
+import {
+ CURATED_VISIBILITIES,
+ isCuratedVisibility,
+ mapVisibility,
+ visibilityMatchesFlags,
+} from '../src/lib/field-visibility.js';
+import { mapVisibility as mapVisibilityFromPushToNeo } from '../src/push-to-neo.js';
+
+// ETP-4793 — this module is the single source of truth for the curated-visibility
+// → NEO-flag projection. It previously existed twice (exported from push-to-neo.js,
+// inlined in lib/neo-delta.js) and validator rule F23 would have been a third copy.
+
+describe('mapVisibility', () => {
+ it('projects each curated value to its documented flag pair', () => {
+ assert.deepEqual(mapVisibility('editable'), { isIncluded: 'Y', isReadOnly: 'N' });
+ assert.deepEqual(mapVisibility('readOnly'), { isIncluded: 'Y', isReadOnly: 'Y' });
+ assert.deepEqual(mapVisibility('system'), { isIncluded: 'Y', isReadOnly: 'Y' });
+ assert.deepEqual(mapVisibility('discarded'), { isIncluded: 'N', isReadOnly: 'N' });
+ });
+
+ it('collapses readOnly and system to the same pair', () => {
+ // The lossy step that makes the VISIBILITY column necessary: the flags alone
+ // cannot tell an agent which of the two it is looking at.
+ assert.deepEqual(mapVisibility('readOnly'), mapVisibility('system'));
+ });
+
+ it('treats every non-curated value as closed', () => {
+ for (const value of [null, undefined, '', 'EDITABLE', 'hidden', 0, false]) {
+ assert.deepEqual(mapVisibility(value), { isIncluded: 'N', isReadOnly: 'N' },
+ `expected ${JSON.stringify(value)} to project closed`);
+ }
+ });
+
+ it('is the same function push-to-neo.js exports', () => {
+ // push-to-neo re-exports it; the name is public API and callers must not get
+ // a second implementation.
+ assert.equal(mapVisibilityFromPushToNeo, mapVisibility);
+ });
+});
+
+describe('isCuratedVisibility', () => {
+ it('accepts exactly the four curated values', () => {
+ for (const v of CURATED_VISIBILITIES) assert.equal(isCuratedVisibility(v), true, v);
+ for (const v of [null, undefined, '', 'readonly', 'System']) {
+ assert.equal(isCuratedVisibility(v), false, JSON.stringify(v));
+ }
+ });
+});
+
+describe('visibilityMatchesFlags', () => {
+ it('accepts a row whose flags match its curated visibility', () => {
+ const verdict = visibilityMatchesFlags({ visibility: 'readOnly', isIncluded: 'Y', isReadOnly: 'Y' });
+ assert.equal(verdict.ok, true);
+ assert.equal(verdict.kind, 'ok');
+ });
+
+ it('accepts a closed row that carries no visibility', () => {
+ // N/N IS mapVisibility's default, so an absent VISIBILITY is coherent here.
+ // The ETP-4793 exclude fix produces exactly these rows.
+ assert.equal(visibilityMatchesFlags({ isIncluded: 'N', isReadOnly: 'N' }).ok, true);
+ assert.equal(visibilityMatchesFlags({ visibility: null, isIncluded: 'N', isReadOnly: 'N' }).ok, true);
+ });
+
+ it('accepts a row that omits the flags entirely', () => {
+ assert.equal(visibilityMatchesFlags({}).ok, true);
+ });
+
+ it('classifies a curated value disagreeing with the flags as a contradiction', () => {
+ const verdict = visibilityMatchesFlags({ visibility: 'editable', isIncluded: 'N', isReadOnly: 'N' });
+ assert.equal(verdict.ok, false);
+ assert.equal(verdict.kind, 'contradiction');
+ assert.deepEqual(verdict.expected, { isIncluded: 'Y', isReadOnly: 'N' });
+ });
+
+ it('classifies an included row with no visibility as unwritten, not a contradiction', () => {
+ const verdict = visibilityMatchesFlags({ isIncluded: 'Y', isReadOnly: 'N' });
+ assert.equal(verdict.ok, false);
+ assert.equal(verdict.kind, 'unwritten');
+ });
+
+ it('classifies an unrecognised visibility string as unwritten', () => {
+ // Not a contradiction: nothing in the pipeline can produce it, so blaming
+ // the writer would be wrong. It is still incoherent and worth reporting.
+ assert.equal(visibilityMatchesFlags({ visibility: 'bogus', isIncluded: 'Y', isReadOnly: 'N' }).kind,
+ 'unwritten');
+ });
+
+ it('catches the readOnly/system pair being stored as editable flags', () => {
+ // The concrete IMP-26 shape: curation says readOnly, the runtime lets writes
+ // through because ISREADONLY was never flipped.
+ const verdict = visibilityMatchesFlags({ visibility: 'readOnly', isIncluded: 'Y', isReadOnly: 'N' });
+ assert.equal(verdict.kind, 'contradiction');
+ assert.deepEqual(verdict.expected, { isIncluded: 'Y', isReadOnly: 'Y' });
+ });
+});
diff --git a/cli/test/fixtures/f23-sourcedata-aggregate/ETGO_SF_ENTITY.xml b/cli/test/fixtures/f23-sourcedata-aggregate/ETGO_SF_ENTITY.xml
new file mode 100644
index 000000000..a81cf0495
--- /dev/null
+++ b/cli/test/fixtures/f23-sourcedata-aggregate/ETGO_SF_ENTITY.xml
@@ -0,0 +1,10 @@
+
+
+
+
+
+
+
+
+
+
diff --git a/cli/test/fixtures/f23-sourcedata-aggregate/ETGO_SF_FIELD.xml b/cli/test/fixtures/f23-sourcedata-aggregate/ETGO_SF_FIELD.xml
new file mode 100644
index 000000000..7acffe425
--- /dev/null
+++ b/cli/test/fixtures/f23-sourcedata-aggregate/ETGO_SF_FIELD.xml
@@ -0,0 +1,13 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/cli/test/fixtures/f23-sourcedata-aggregate/ETGO_SF_SPEC.xml b/cli/test/fixtures/f23-sourcedata-aggregate/ETGO_SF_SPEC.xml
new file mode 100644
index 000000000..0bef5f2d2
--- /dev/null
+++ b/cli/test/fixtures/f23-sourcedata-aggregate/ETGO_SF_SPEC.xml
@@ -0,0 +1,9 @@
+
+
+
+
+
+
+
+
+
diff --git a/cli/test/fixtures/f23-sourcedata-blank/ETGO_SF_ENTITY.xml b/cli/test/fixtures/f23-sourcedata-blank/ETGO_SF_ENTITY.xml
new file mode 100644
index 000000000..8e2c29ebb
--- /dev/null
+++ b/cli/test/fixtures/f23-sourcedata-blank/ETGO_SF_ENTITY.xml
@@ -0,0 +1,10 @@
+
+
+
+
+
+
+
+
+
+
diff --git a/cli/test/fixtures/f23-sourcedata-blank/ETGO_SF_FIELD.xml b/cli/test/fixtures/f23-sourcedata-blank/ETGO_SF_FIELD.xml
new file mode 100644
index 000000000..2364da9c4
--- /dev/null
+++ b/cli/test/fixtures/f23-sourcedata-blank/ETGO_SF_FIELD.xml
@@ -0,0 +1,10 @@
+
+
+
+
+
+
+
+
+
+
diff --git a/cli/test/fixtures/f23-sourcedata-blank/ETGO_SF_SPEC.xml b/cli/test/fixtures/f23-sourcedata-blank/ETGO_SF_SPEC.xml
new file mode 100644
index 000000000..c9f66f92f
--- /dev/null
+++ b/cli/test/fixtures/f23-sourcedata-blank/ETGO_SF_SPEC.xml
@@ -0,0 +1,9 @@
+
+
+
+
+
+
+
+
+
diff --git a/cli/test/fixtures/f23-sourcedata-clean/ETGO_SF_ENTITY.xml b/cli/test/fixtures/f23-sourcedata-clean/ETGO_SF_ENTITY.xml
new file mode 100644
index 000000000..8e2c29ebb
--- /dev/null
+++ b/cli/test/fixtures/f23-sourcedata-clean/ETGO_SF_ENTITY.xml
@@ -0,0 +1,10 @@
+
+
+
+
+
+
+
+
+
+
diff --git a/cli/test/fixtures/f23-sourcedata-clean/ETGO_SF_FIELD.xml b/cli/test/fixtures/f23-sourcedata-clean/ETGO_SF_FIELD.xml
new file mode 100644
index 000000000..76265771e
--- /dev/null
+++ b/cli/test/fixtures/f23-sourcedata-clean/ETGO_SF_FIELD.xml
@@ -0,0 +1,57 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/cli/test/fixtures/f23-sourcedata-clean/ETGO_SF_SPEC.xml b/cli/test/fixtures/f23-sourcedata-clean/ETGO_SF_SPEC.xml
new file mode 100644
index 000000000..c9f66f92f
--- /dev/null
+++ b/cli/test/fixtures/f23-sourcedata-clean/ETGO_SF_SPEC.xml
@@ -0,0 +1,9 @@
+
+
+
+
+
+
+
+
+
diff --git a/cli/test/fixtures/f23-sourcedata-contradiction/ETGO_SF_ENTITY.xml b/cli/test/fixtures/f23-sourcedata-contradiction/ETGO_SF_ENTITY.xml
new file mode 100644
index 000000000..8e2c29ebb
--- /dev/null
+++ b/cli/test/fixtures/f23-sourcedata-contradiction/ETGO_SF_ENTITY.xml
@@ -0,0 +1,10 @@
+
+
+
+
+
+
+
+
+
+
diff --git a/cli/test/fixtures/f23-sourcedata-contradiction/ETGO_SF_FIELD.xml b/cli/test/fixtures/f23-sourcedata-contradiction/ETGO_SF_FIELD.xml
new file mode 100644
index 000000000..1a94fd9cb
--- /dev/null
+++ b/cli/test/fixtures/f23-sourcedata-contradiction/ETGO_SF_FIELD.xml
@@ -0,0 +1,24 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/cli/test/fixtures/f23-sourcedata-contradiction/ETGO_SF_SPEC.xml b/cli/test/fixtures/f23-sourcedata-contradiction/ETGO_SF_SPEC.xml
new file mode 100644
index 000000000..c9f66f92f
--- /dev/null
+++ b/cli/test/fixtures/f23-sourcedata-contradiction/ETGO_SF_SPEC.xml
@@ -0,0 +1,9 @@
+
+
+
+
+
+
+
+
+
diff --git a/cli/test/fixtures/f23-sourcedata-empty/ETGO_SF_ENTITY.xml b/cli/test/fixtures/f23-sourcedata-empty/ETGO_SF_ENTITY.xml
new file mode 100644
index 000000000..c528be2bd
--- /dev/null
+++ b/cli/test/fixtures/f23-sourcedata-empty/ETGO_SF_ENTITY.xml
@@ -0,0 +1,3 @@
+
+
+
diff --git a/cli/test/fixtures/f23-sourcedata-empty/ETGO_SF_FIELD.xml b/cli/test/fixtures/f23-sourcedata-empty/ETGO_SF_FIELD.xml
new file mode 100644
index 000000000..c528be2bd
--- /dev/null
+++ b/cli/test/fixtures/f23-sourcedata-empty/ETGO_SF_FIELD.xml
@@ -0,0 +1,3 @@
+
+
+
diff --git a/cli/test/fixtures/f23-sourcedata-empty/ETGO_SF_SPEC.xml b/cli/test/fixtures/f23-sourcedata-empty/ETGO_SF_SPEC.xml
new file mode 100644
index 000000000..c528be2bd
--- /dev/null
+++ b/cli/test/fixtures/f23-sourcedata-empty/ETGO_SF_SPEC.xml
@@ -0,0 +1,3 @@
+
+
+
diff --git a/cli/test/fixtures/f23-sourcedata-inactive/ETGO_SF_ENTITY.xml b/cli/test/fixtures/f23-sourcedata-inactive/ETGO_SF_ENTITY.xml
new file mode 100644
index 000000000..8e2c29ebb
--- /dev/null
+++ b/cli/test/fixtures/f23-sourcedata-inactive/ETGO_SF_ENTITY.xml
@@ -0,0 +1,10 @@
+
+
+
+
+
+
+
+
+
+
diff --git a/cli/test/fixtures/f23-sourcedata-inactive/ETGO_SF_FIELD.xml b/cli/test/fixtures/f23-sourcedata-inactive/ETGO_SF_FIELD.xml
new file mode 100644
index 000000000..6c6fb222d
--- /dev/null
+++ b/cli/test/fixtures/f23-sourcedata-inactive/ETGO_SF_FIELD.xml
@@ -0,0 +1,25 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/cli/test/fixtures/f23-sourcedata-inactive/ETGO_SF_SPEC.xml b/cli/test/fixtures/f23-sourcedata-inactive/ETGO_SF_SPEC.xml
new file mode 100644
index 000000000..c9f66f92f
--- /dev/null
+++ b/cli/test/fixtures/f23-sourcedata-inactive/ETGO_SF_SPEC.xml
@@ -0,0 +1,9 @@
+
+
+
+
+
+
+
+
+
diff --git a/cli/test/fixtures/f23-sourcedata-unwritten/ETGO_SF_ENTITY.xml b/cli/test/fixtures/f23-sourcedata-unwritten/ETGO_SF_ENTITY.xml
new file mode 100644
index 000000000..8e2c29ebb
--- /dev/null
+++ b/cli/test/fixtures/f23-sourcedata-unwritten/ETGO_SF_ENTITY.xml
@@ -0,0 +1,10 @@
+
+
+
+
+
+
+
+
+
+
diff --git a/cli/test/fixtures/f23-sourcedata-unwritten/ETGO_SF_FIELD.xml b/cli/test/fixtures/f23-sourcedata-unwritten/ETGO_SF_FIELD.xml
new file mode 100644
index 000000000..7a8224326
--- /dev/null
+++ b/cli/test/fixtures/f23-sourcedata-unwritten/ETGO_SF_FIELD.xml
@@ -0,0 +1,34 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/cli/test/fixtures/f23-sourcedata-unwritten/ETGO_SF_SPEC.xml b/cli/test/fixtures/f23-sourcedata-unwritten/ETGO_SF_SPEC.xml
new file mode 100644
index 000000000..c9f66f92f
--- /dev/null
+++ b/cli/test/fixtures/f23-sourcedata-unwritten/ETGO_SF_SPEC.xml
@@ -0,0 +1,9 @@
+
+
+
+
+
+
+
+
+
diff --git a/cli/test/neo-writer-populate.test.js b/cli/test/neo-writer-populate.test.js
index 10e27a927..012d0e114 100644
--- a/cli/test/neo-writer-populate.test.js
+++ b/cli/test/neo-writer-populate.test.js
@@ -749,3 +749,122 @@ describe('populateSpec (report)', () => {
);
});
});
+
+// ---------------------------------------------------------------------------
+// ETP-4793 — `exclude: true` on the live write path
+// ---------------------------------------------------------------------------
+
+/**
+ * `methodFlagsFor` now carries an `isIncluded` axis alongside the six HTTP verbs
+ * (see `buildEntityMethodFlagsResolver` in push-to-neo.js). When it answers 'N'
+ * for a tab, `populateWindowSpec` must close BOTH the ETGO_SF_ENTITY row and
+ * every ETGO_SF_FIELD row underneath it — closing the entity alone would be
+ * enough for behaviour, since every reader filters ISINCLUDED before it resolves
+ * a field, but field rows still claiming 'Y' misreport the agent surface.
+ */
+describe('populateSpec (window) — entities closed by exclude: true', () => {
+ const SPEC_ID = 'SPEC001';
+ const MODULE_ID = 'MOD001';
+ const WINDOW_ID = 'WIN001';
+ const OPEN_TAB = 'TAB001';
+ const CLOSED_TAB = 'TAB002';
+ const OPEN_TABLE = 'TBL001';
+ const CLOSED_TABLE = 'TBL002';
+
+ const ALL_VERBS_Y = {
+ isGet: 'Y', isGetbyid: 'Y', isPost: 'Y', isPut: 'Y', isPatch: 'Y', isDelete: 'Y',
+ };
+
+ function clientWithOneClosedTab() {
+ return createMockClient({
+ specs: [[SPEC_ID, { spec_type: 'W', ad_window_id: WINDOW_ID, ad_process_id: null }]],
+ tabs: [
+ { ad_tab_id: OPEN_TAB, name: 'Header', ad_table_id: OPEN_TABLE, seqno: 10, ad_window_id: WINDOW_ID },
+ { ad_tab_id: CLOSED_TAB, name: 'Audit Log', ad_table_id: CLOSED_TABLE, seqno: 20, ad_window_id: WINDOW_ID },
+ ],
+ columns: [
+ { ad_column_id: 'COL001', columnname: 'DocumentNo', position: 10, ad_table_id: OPEN_TABLE },
+ // Same column NAME on both tables — the entity-blind flat-column rule in
+ // push-to-neo would keep this one open, so the entity-level close has to
+ // be what shuts it.
+ { ad_column_id: 'COL002', columnname: 'DocumentNo', position: 10, ad_table_id: CLOSED_TABLE },
+ { ad_column_id: 'COL003', columnname: 'Rate', position: 20, ad_table_id: CLOSED_TABLE },
+ ],
+ });
+ }
+
+ /** The tab whose contract entity is missing answers isIncluded: 'N'. */
+ const methodFlagsFor = (tab) => ({
+ isIncluded: tab.ad_tab_id === CLOSED_TAB ? 'N' : 'Y',
+ ...ALL_VERBS_Y,
+ });
+
+ /** Entity INSERT param order: [id, specId, tabId, ?, name, isincluded, …]. */
+ function entityInsertsByName(client) {
+ const byName = {};
+ for (const q of client.queryLog) {
+ if (q.sql.includes('INSERT INTO etgo_sf_entity')) {
+ byName[q.params[4]] = { entityId: q.params[0], isIncluded: q.params[5] };
+ }
+ }
+ return byName;
+ }
+
+ /** Field INSERT param order: [id, entityId, columnId, moduleId, isincluded, …]. */
+ function fieldInsertsByColumn(client) {
+ const byColumn = {};
+ for (const q of client.queryLog) {
+ if (q.sql.includes('INSERT INTO etgo_sf_field')) {
+ byColumn[q.params[2]] = { entityId: q.params[1], isIncluded: q.params[4] };
+ }
+ }
+ return byColumn;
+ }
+
+ it("writes ISINCLUDED='N' on the closed entity and 'Y' on its sibling", async () => {
+ const client = clientWithOneClosedTab();
+
+ await populateSpec(client, { specId: SPEC_ID, moduleId: MODULE_ID, methodFlagsFor });
+
+ const entities = entityInsertsByName(client);
+ assert.equal(entities.Header.isIncluded, 'Y');
+ assert.equal(entities['Audit Log'].isIncluded, 'N');
+ });
+
+ it("closes every field of the closed entity, sharing a column name or not", async () => {
+ const client = clientWithOneClosedTab();
+
+ await populateSpec(client, { specId: SPEC_ID, moduleId: MODULE_ID, methodFlagsFor });
+
+ const fields = fieldInsertsByColumn(client);
+ assert.equal(fields.COL001.isIncluded, 'Y', 'Header.DocumentNo stays open');
+ assert.equal(
+ fields.COL002.isIncluded, 'N',
+ 'Audit Log.DocumentNo is closed despite Header having a DocumentNo too',
+ );
+ assert.equal(fields.COL003.isIncluded, 'N', 'Audit Log.Rate is closed');
+ });
+
+ it('reports the closed entities in closedEntityCount', async () => {
+ const client = clientWithOneClosedTab();
+
+ const result = await populateSpec(client, { specId: SPEC_ID, moduleId: MODULE_ID, methodFlagsFor });
+
+ assert.equal(result.entityCount, 2, 'a closed entity is still written, not skipped');
+ assert.equal(result.fieldCount, 3);
+ assert.equal(result.closedEntityCount, 1);
+ });
+
+ it('counts nothing closed when every tab is included', async () => {
+ const client = clientWithOneClosedTab();
+
+ const result = await populateSpec(client, {
+ specId: SPEC_ID,
+ moduleId: MODULE_ID,
+ methodFlagsFor: () => ({ isIncluded: 'Y', ...ALL_VERBS_Y }),
+ });
+
+ assert.equal(result.closedEntityCount, 0);
+ for (const f of Object.values(fieldInsertsByColumn(client))) assert.equal(f.isIncluded, 'Y');
+ });
+});
diff --git a/cli/test/neo-writer-upsert-field.test.js b/cli/test/neo-writer-upsert-field.test.js
index 32c724ac9..1aef02f02 100644
--- a/cli/test/neo-writer-upsert-field.test.js
+++ b/cli/test/neo-writer-upsert-field.test.js
@@ -1,13 +1,14 @@
/**
- * Tests for upsertField in neo-writer.js, focused on the agentPrompt column
- * and the partial-update contract (only explicitly provided columns are SET).
+ * Tests for upsertField in neo-writer.js, focused on the agentPrompt and
+ * visibility columns and the partial-update contract (only explicitly provided
+ * columns are SET).
*
* Uses a lightweight mock pg client that records the SQL and params of the
* UPDATE/INSERT it receives, so no real database is needed.
*/
import { describe, it } from 'node:test';
import { strict as assert } from 'node:assert';
-import { upsertField } from '../src/neo-writer.js';
+import { upsertField, normalizeVisibility, FIELD_VISIBILITIES } from '../src/neo-writer.js';
function createMockClient() {
const updates = [];
@@ -27,6 +28,23 @@ function createMockClient() {
};
}
+/**
+ * Resolve the value bound to a named column of an INSERT, by reading the
+ * column list out of the SQL instead of assuming a position. Positional
+ * assertions ("the last param") silently pass onto the wrong column the next
+ * time a column is appended — which is exactly what happened when `visibility`
+ * was added after `agent_prompt`.
+ */
+function insertValue(insert, column) {
+ const columns = insert.sql
+ .slice(insert.sql.indexOf('(') + 1, insert.sql.indexOf(')'))
+ .split(',')
+ .map(c => c.trim());
+ const index = columns.indexOf(column);
+ assert.notEqual(index, -1, `column ${column} is present in the INSERT`);
+ return insert.params[index];
+}
+
describe('upsertField (agentPrompt)', () => {
it('SETs agent_prompt on UPDATE when provided', async () => {
const client = createMockClient();
@@ -61,7 +79,7 @@ describe('upsertField (agentPrompt)', () => {
assert.doesNotMatch(sql, /agent_prompt/);
});
- it('persists agent_prompt as the last INSERT param', async () => {
+ it('persists agent_prompt on INSERT', async () => {
const client = createMockClient();
await upsertField(client, {
@@ -71,9 +89,7 @@ describe('upsertField (agentPrompt)', () => {
});
assert.equal(client.inserts.length, 1);
- const { sql, params } = client.inserts[0];
- assert.match(sql, /agent_prompt/);
- assert.equal(params[params.length - 1], 'Hint for a new field.');
+ assert.equal(insertValue(client.inserts[0], 'agent_prompt'), 'Hint for a new field.');
});
it('defaults agent_prompt to null on INSERT when omitted', async () => {
@@ -81,7 +97,97 @@ describe('upsertField (agentPrompt)', () => {
await upsertField(client, { entityId: 'ENT1', moduleId: 'MOD1' });
- const { params } = client.inserts[0];
- assert.equal(params[params.length - 1], null);
+ assert.equal(insertValue(client.inserts[0], 'agent_prompt'), null);
+ });
+});
+
+describe('upsertField (visibility)', () => {
+ it('persists visibility on INSERT', async () => {
+ const client = createMockClient();
+
+ await upsertField(client, { entityId: 'ENT1', moduleId: 'MOD1', visibility: 'system' });
+
+ assert.equal(insertValue(client.inserts[0], 'visibility'), 'system');
+ });
+
+ it('defaults visibility to null on INSERT when omitted', async () => {
+ const client = createMockClient();
+
+ await upsertField(client, { entityId: 'ENT1', moduleId: 'MOD1' });
+
+ assert.equal(insertValue(client.inserts[0], 'visibility'), null);
+ });
+
+ it('SETs visibility on UPDATE when provided', async () => {
+ const client = createMockClient();
+
+ await upsertField(client, {
+ entityId: 'ENT1',
+ moduleId: 'MOD1',
+ fieldId: 'FLD1',
+ visibility: 'readOnly',
+ });
+
+ const { sql, params } = client.updates[0];
+ assert.match(sql, /visibility = \$\d+/);
+ assert.ok(params.includes('readOnly'), 'visibility value is bound in the UPDATE params');
+ });
+
+ it('omits visibility from UPDATE when not provided (partial-update contract)', async () => {
+ const client = createMockClient();
+
+ await upsertField(client, {
+ entityId: 'ENT1',
+ moduleId: 'MOD1',
+ fieldId: 'FLD1',
+ isReadOnly: 'Y',
+ });
+
+ assert.doesNotMatch(client.updates[0].sql, /visibility/);
+ });
+
+ it('stores an explicit null when visibility is passed as null', async () => {
+ const client = createMockClient();
+
+ await upsertField(client, {
+ entityId: 'ENT1',
+ moduleId: 'MOD1',
+ fieldId: 'FLD1',
+ visibility: null,
+ });
+
+ const { sql, params } = client.updates[0];
+ assert.match(sql, /visibility = \$\d+/);
+ assert.ok(params.includes(null), 'null is bound, clearing a stale classification');
+ });
+
+ it('rejects a value outside the curated vocabulary', async () => {
+ const client = createMockClient();
+
+ await assert.rejects(
+ () => upsertField(client, { entityId: 'ENT1', moduleId: 'MOD1', visibility: 'hidden' }),
+ /invalid visibility "hidden"/,
+ );
+ assert.equal(client.inserts.length, 0, 'nothing is written when validation fails');
+ });
+
+ it('accepts every curated visibility value', async () => {
+ for (const visibility of FIELD_VISIBILITIES) {
+ const client = createMockClient();
+ await upsertField(client, { entityId: 'ENT1', moduleId: 'MOD1', visibility });
+ assert.equal(insertValue(client.inserts[0], 'visibility'), visibility);
+ }
+ });
+});
+
+describe('normalizeVisibility', () => {
+ it('treats null and empty string as unclassified', () => {
+ assert.equal(normalizeVisibility(null), null);
+ assert.equal(normalizeVisibility(undefined), null);
+ assert.equal(normalizeVisibility(''), null);
+ });
+
+ it('is case-sensitive — the vocabulary is camelCase', () => {
+ assert.throws(() => normalizeVisibility('readonly'), /invalid visibility/);
});
});
diff --git a/cli/test/push-to-neo-helpers.test.js b/cli/test/push-to-neo-helpers.test.js
index bfc88772d..07197a397 100644
--- a/cli/test/push-to-neo-helpers.test.js
+++ b/cli/test/push-to-neo-helpers.test.js
@@ -11,8 +11,55 @@ import {
formatDuplicateFieldsError,
loadConfig,
buildDesiredEntitiesMap,
+ buildFieldUpdateParams,
} from '../src/push-to-neo.js';
+function updateParams(visibility) {
+ return buildFieldUpdateParams(
+ { entityName: 'header', fieldName: 'docStatus', column: 'DocStatus', visibility },
+ { moduleId: 'MOD1', fieldDefaultExprs: {}, auditOpts: {} },
+ 'FLD1',
+ 'ENT1',
+ );
+}
+
+describe('buildFieldUpdateParams (visibility passthrough)', () => {
+ it('forwards the curated visibility verbatim', () => {
+ assert.equal(updateParams('system').visibility, 'system');
+ });
+
+ it('distinguishes system from readOnly even though both map to Y/Y', () => {
+ const system = updateParams('system');
+ const readOnly = updateParams('readOnly');
+
+ // The booleans are deliberately identical — that collapse is what NEO's
+ // runtime wants, and it is also why the curated value must travel
+ // separately: neo_schema tells agents to skip system fields and display
+ // readOnly ones, a distinction Y/Y cannot express.
+ assert.equal(system.isIncluded, readOnly.isIncluded);
+ assert.equal(system.isReadOnly, readOnly.isReadOnly);
+ assert.notEqual(system.visibility, readOnly.visibility);
+ });
+
+ it('distinguishes discarded from an unknown value even though both map to N/N', () => {
+ const discarded = updateParams('discarded');
+ const unknown = updateParams(undefined);
+
+ assert.equal(discarded.isIncluded, unknown.isIncluded);
+ assert.equal(discarded.visibility, 'discarded');
+ assert.equal(unknown.visibility, null, 'unclassified stays null, never a made-up value');
+ });
+
+ it('keeps the mapVisibility pair untouched for every curated value', () => {
+ for (const visibility of ['editable', 'readOnly', 'system', 'discarded']) {
+ const params = updateParams(visibility);
+ const expected = mapVisibility(visibility);
+ assert.equal(params.isIncluded, expected.isIncluded);
+ assert.equal(params.isReadOnly, expected.isReadOnly);
+ }
+ });
+});
+
describe('toSpecName', () => {
it('converts simple display name to kebab-case', () => {
assert.equal(toSpecName('Sales Order'), 'sales-order');
diff --git a/cli/test/validate-pipeline.test.js b/cli/test/validate-pipeline.test.js
index b62834399..36b2e241e 100644
--- a/cli/test/validate-pipeline.test.js
+++ b/cli/test/validate-pipeline.test.js
@@ -31,6 +31,12 @@ async function runOnFixtures(windowNames, opts = {}) {
// opts.f19AllowlistPath; otherwise this falls through to the real
// (production) allowlist file, which never matches fixture artifact names.
f19AllowlistPath: opts.f19AllowlistPath,
+ // F23 reads the exported ETGO_SF_*.xml from a com.etendoerp.go checkout.
+ // Default every test at a fixture holding valid-but-empty XMLs: without
+ // this, F23 would resolve the conventional `/../modules/...` path and
+ // its verdict would depend on whether the developer happens to have the
+ // runtime module checked out next to this repo. F23's own tests override it.
+ goSourcedataDir: opts.goSourcedataDir ?? join(FIXTURES, '..', 'f23-sourcedata-empty'),
// Override the artifacts root to point at our fixture directory
_artifactsRoot: FIXTURES,
});
@@ -1110,6 +1116,103 @@ describe('Rule F22 — customTabsAfterBottom + tabOrder', () => {
});
});
+// ─── F23 — ETGO_SF_FIELD.VISIBILITY vs the flags it projects to ────────────
+// ETP-4793 / IMP-26 §5.3. Every fixture below declares its spec NAME as
+// 'window-ok' so it lines up with an existing window artifact — F23 keys the
+// XML rows by spec name, and the artifact directory name IS the spec name.
+
+const F23_FIXTURES = join(FIXTURES, '..');
+
+function runF23(sourcedataFixture) {
+ return runOnFixtures(['window-ok'], {
+ goSourcedataDir: sourcedataFixture === null ? join(F23_FIXTURES, 'f23-no-such-dir') : join(F23_FIXTURES, sourcedataFixture),
+ });
+}
+
+describe('Rule F23 — pushed visibility vs ISINCLUDED/ISREADONLY', () => {
+ it('BLOCK when a stored VISIBILITY contradicts the flags it projects to', async () => {
+ const result = await runF23('f23-sourcedata-contradiction');
+ const f23 = result.violations.find(v => v.rule === 'F23');
+ assert.ok(f23, 'F23 should fire on a curated visibility that disagrees with the flags');
+ assert.equal(f23.severity, 'BLOCK');
+ assert.match(f23.message, /header\.documentNo/);
+ assert.match(f23.message, /visibility='editable'/);
+ assert.equal(f23.f23Contradictions, 1);
+ });
+
+ it('reports the contradiction rather than the unwritten row when both exist', async () => {
+ // The same fixture also holds an included row with no VISIBILITY. A rule
+ // returns one entry, so the blocking class must win — otherwise a real
+ // contradiction hides behind pre-existing backfill debt.
+ const result = await runF23('f23-sourcedata-contradiction');
+ const f23 = result.violations.find(v => v.rule === 'F23');
+ assert.equal(f23.severity, 'BLOCK');
+ assert.equal(f23.f23Unwritten, 1, 'the unwritten row is still counted, just not the headline');
+ assert.ok(!/orderDate/.test(f23.message), 'the WARN-class row must not be listed as a contradiction');
+ });
+
+ it('WARN when included rows carry no VISIBILITY at all', async () => {
+ const result = await runF23('f23-sourcedata-unwritten');
+ const f23 = result.violations.find(v => v.rule === 'F23');
+ assert.ok(f23, 'F23 should report the backfill gap');
+ assert.equal(f23.severity, 'WARN');
+ assert.equal(f23.f23Unwritten, 2);
+ assert.match(f23.message, /header\.orderDate/);
+ assert.match(f23.message, /header\.businessPartner/);
+ assert.ok(!/documentNo/.test(f23.message), 'the coherent editable row must not be reported');
+ });
+
+ it('passes when every row projects correctly, including a closed row with no VISIBILITY', async () => {
+ // N/N with no VISIBILITY is exactly mapVisibility's default, so it is
+ // coherent — the ETP-4793 exclude fix relies on that and must stay green.
+ const result = await runF23('f23-sourcedata-clean');
+ assert.ok(!result.violations.find(v => v.rule === 'F23'), 'F23 must not fire on coherent rows');
+ });
+
+ it('ignores rows flagged ISACTIVE=N', async () => {
+ const result = await runF23('f23-sourcedata-inactive');
+ assert.ok(!result.violations.find(v => v.rule === 'F23'), 'retired config is not a live surface');
+ });
+
+ it('is inert — not skipped — without a com.etendoerp.go checkout', async () => {
+ const result = await runF23(null);
+ assert.ok(!result.violations.find(v => v.rule === 'F23'), 'no violation without the XML');
+ assert.ok(!result.skipped.find(s => s.rule === 'F23'),
+ 'and no skipped entry either: one per artifact would drown the report on a functional-only checkout');
+ });
+
+ it('also runs on aggregate artifacts, which push their own spec', async () => {
+ // The regression this pins: F23 was first registered for kind === 'window'
+ // only and reported 69 of the 409 incoherent rows in the live export — the
+ // other 340 sat in two aggregate artifacts (return-to-vendor,
+ // return-from-customer) that the rule never visited.
+ const result = await runOnFixtures(['aggregate-ok'], {
+ goSourcedataDir: join(F23_FIXTURES, 'f23-sourcedata-aggregate'),
+ });
+ const f23 = result.violations.find(v => v.rule === 'F23');
+ assert.ok(f23, 'F23 must visit aggregate artifacts');
+ assert.equal(f23.severity, 'BLOCK');
+ assert.equal(f23.artifactKind, 'aggregate');
+ assert.match(f23.message, /lines\.returnQty/);
+ });
+
+ it('labels a row with no AD_COLUMN_ID by primary key, not "undefined"', async () => {
+ const result = await runF23('f23-sourcedata-blank');
+ const f23 = result.violations.find(v => v.rule === 'F23');
+ assert.ok(f23);
+ assert.ok(!/\.undefined/.test(f23.message), 'an unidentifiable field makes the warning useless');
+ assert.match(f23.message, /no AD_COLUMN_ID, field F0{30}1/);
+ });
+
+ it('honours --skip=F23', async () => {
+ const result = await runOnFixtures(['window-ok'], {
+ goSourcedataDir: join(F23_FIXTURES, 'f23-sourcedata-contradiction'),
+ skip: ['F23'],
+ });
+ assert.ok(!result.violations.find(v => v.rule === 'F23'));
+ });
+});
+
// ─── F19 — custom table `required` flag drift (ETP-4609 follow-up) ─────────
// These fixtures need a real tools/app-shell/src/windows/custom// tree
// to resolve against, so `root` is overridden to the fixtures dir itself (the
diff --git a/docs/decisions-reference.md b/docs/decisions-reference.md
index 39bc9e0c8..29906820f 100644
--- a/docs/decisions-reference.md
+++ b/docs/decisions-reference.md
@@ -526,7 +526,7 @@ Entity keys use **camelCase from tabName** (e.g., `"header"`, `"lines"`, `"basic
| Property | Type | Default | Purpose |
|----------|------|---------|---------|
| `name` | string | Entity key | Override display name. |
-| `exclude` | boolean | `false` | Omit entire entity from schema. |
+| `exclude` | boolean | `false` | Omit entire entity from schema **and close it in NEO** (`ETGO_SF_ENTITY.ISINCLUDED = 'N'` on the entity plus every one of its `ETGO_SF_FIELD` rows). See below. |
| `fields` | object | `{}` | Field-level decisions. |
| `draftMode` | object | `null` | Draft/Processed workflow config. |
| `javaQualifier` | string | `null` | CDI qualifier for custom NeoHandler. |
@@ -1084,6 +1084,34 @@ Rule keys use **extended names** (including trigger column suffix for multi-trig
}
```
+**What it does (ETP-4793).** The entity is dropped from the curated schema, and so
+from `contract.json`. Both write paths then close it in NEO:
+
+| Row | Column | Value |
+|---|---|---|
+| `ETGO_SF_ENTITY` (the excluded entity) | `ISINCLUDED` | `N` |
+| `ETGO_SF_FIELD` (every field of it) | `ISINCLUDED` | `N` |
+
+`ISINCLUDED = 'N'` on the entity is what actually removes it from the served
+surface — every REST and MCP reader filters on it before resolving an entity, so
+neither `GET /{entity}` nor an MCP `neo_list` can reach it. The field rows are
+closed too: it is redundant for behaviour, but a closed entity whose 15 field rows
+still claim `ISINCLUDED = 'Y'` misreports the size of the agent surface, which is
+exactly how the gap went unnoticed for 90 entities / 386 fields.
+
+Two deliberate non-changes:
+
+- **The six HTTP method flags are left at the window default, not zeroed.** They
+ are unreachable once the entity is closed, and an all-`N` set would break the
+ "GET and GETBYID are always granted" invariant that `cli/src/lib/entity-methods.js`
+ enforces. Leaving them alone also keeps the XML diff to one column per entity.
+- **`VISIBILITY` is not written on the closed field rows.** The offline XML delta
+ does not model that column, and NULL beside `ISINCLUDED='N'` / `ISREADONLY='N'`
+ is already the pair `mapVisibility('discarded')` produces.
+
+Excluding an entity changes `ETGO_SF_*`, so it needs a re-push and an export:
+`make regen ONLY= PUSH_TO_NEO=1` then `./gradlew export.database`.
+
### Custom NeoHandler for an entity
```json
{
diff --git a/docs/pipeline-validator-reference.md b/docs/pipeline-validator-reference.md
index 270ed06a2..f13791564 100644
--- a/docs/pipeline-validator-reference.md
+++ b/docs/pipeline-validator-reference.md
@@ -63,6 +63,7 @@ Rules are grouped by the artifact kind they apply to (see [Artifact Classificati
| F17 | BLOCK | `decisions.json` `window.balanceFooter` is missing `debitField`/`creditField`, or references a field that does not exist on the lines entity (validated against `frontendContract.entities..fields[].name`). | Set `window.balanceFooter` to `{ debitField, creditField }` using amount-typed line-entity field names that exist in the contract. |
| F21 | BLOCK | The declared read-only intent in `decisions.json` (`window.readOnly`, `entities..readOnly`, `entities..methods`, and — since ETP-4745 — `window.hideDelete` / `entities..hideDelete`) does not match the HTTP methods the contract would push to `ETGO_SF_ENTITY` — i.e. `apiPrediction.crud..methods` / `apiPrediction.window.readOnly` / `apiPrediction.crud..delete` is stale. Also fires when an emitted `crud..methods` array omits `GET`/`GETBYID` (an entity with no read access is never valid), or when a restricting `entities.` declaration (including a `hideDelete`-only one) matches no contract entity (mistyped key → silent no-op). | Regenerate the contract (`make regen ONLY=`). Both write paths — the live DB push and the offline XML delta — read the method flags off the contract, so a stale contract silently re-opens a read-only window (or re-enables DELETE on a hideDelete entity) for writes on the next push (ETP-4254 / ETP-4745). Never hand-edit `crud.methods`. |
| F22 | BLOCK | `decisions.json` sets `window.customTabsAfterBottom: true` (custom tabs render in a strip below `bottomSection`, outside the sorted tab list) while a custom tab in `customPanelTabs`/`extraTabs`/`attachments` still declares a `tabOrder` — that `tabOrder` is a silent no-op. | Remove `window.customTabsAfterBottom`, or remove `tabOrder` from the listed custom tab(s) (ETP-4415). |
+| F23 | BLOCK / WARN | A pushed `ETGO_SF_FIELD` row whose `VISIBILITY` does not project to its own `ISINCLUDED`/`ISREADONLY` pair under `mapVisibility()` (`lib/field-visibility.js`). The curated decision is stored twice on purpose — the runtime enforces the booleans, `neo_schema` reports the word, and the booleans cannot recover it (`readOnly` and `system` both collapse to `Y`/`Y`) — so the copies can disagree. **BLOCK** when `VISIBILITY` holds a curated value projecting to a different pair (writer bug or hand-edit); **WARN** when `VISIBILITY` is absent while `ISINCLUDED='Y'` (backfill debt: agents cannot tell `readOnly` from `system`). An absent `VISIBILITY` on a closed row is coherent, not a violation — `N`/`N` *is* `mapVisibility(null)`. Ignores `ISACTIVE='N'`. Reads the exported sourcedata XML from `${SF_GO_SOURCEDATA_DIR}` → `${ETENDO_ROOT}/modules/com.etendoerp.go/src-db/database/sourcedata` → the sibling `../modules/...`; **inert (no violation, no `skipped` entry) when that checkout is absent**, so a green F23 is not by itself evidence the rule ran. Registered for `window` and `aggregate` artifacts. | Re-push (`make regen ONLY= PUSH_TO_NEO=1`) then `./gradlew export.database`. A BLOCK surviving a clean push is a writer bug — fix `neo-writer.js → populateSpec`, never the XML (ETP-4793 / IMP-26 §5.3). |
### Report rules
@@ -75,6 +76,7 @@ Rules are grouped by the artifact kind they apply to (see [Artifact Classificati
| Code | Severity | What it detects | How to fix |
|------|----------|-----------------|------------|
| F9 | BLOCK | Aggregate folder has `generated/` but no `aggregate-contract.json`. | Re-run the pipeline for the aggregate, or add the missing `aggregate-contract.json`. |
+| F23 | BLOCK / WARN | Same rule as in the window table — an aggregate pushes its own `ETGO_SF_SPEC`, so its field rows drift identically. At ETP-4793, 340 of the 409 incoherent rows in the live export were inside two aggregates, so registering F23 for windows alone would have hidden 83 % of the defect. | See F23 in the window table. |
### General rules