fix(dynamodb): treat a block-level unresolved intrinsic as declared, and skip a non-string IndexName explicitly

Third review pass on PR #1443. It was right that deferring the block-level
case to #1444 did not hold up: it needed nothing from the other deferred
item, and routes straight into the warning this PR already emits.

Presence was tested on the MEMBER key, so a block that is ITSELF an
unresolved intrinsic - WriteOnDemandThroughputSettings: {"Fn::If": [...]} -
is a record with no MaxWriteRequestUnits, reported "not declared", and fired
the destructive -1 on a ceiling the template was trying to set. That is the
same bug one level up from where the guard looked.
isUnresolvedIntrinsicBlock (every key Ref or Fn::*) routes it into the same
suppression, while a genuine empty block still counts as a removal.

Also makes the non-string IndexName skip explicit in the modified loop. Such
an entry was safe only because previousSdkByName.get(<object>) missed on
identity - an accident, not either mechanism the doc comment names.

Refs #1440

Author: go-to-k

docs/changelog-cdkd.md

@@ -17,7 +17,7 @@ The CLAUDE.md `## Known Limitations` section retains the load-bearing summary
 ---
 
 **Recently Implemented** (2026-08-10):
-- ✅ **`AWS::DynamoDB::GlobalTable`: the per-GSI on-demand reset stops firing on a present-but-unresolvable value (issue [#1440](https://github.com/go-to-k/cdkd/issues/1440))** — `src/provisioning/providers/dynamodb-globaltable-provider.ts` + `tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts`. **The class:** the #1423 per-GSI reset decided "the template DROPPED this member" from the TRANSLATED side, and `toSdkGlobalSecondaryIndexes` runs every value through `toFiniteNumber` — which returns `undefined` for a present-but-unparseable value (an unresolved `{Ref: …}`, `''`, an object). Such a GSI was therefore indistinguishable from a removal and was answered with the `-1` sentinel, **silently CLEARING the ceiling the template was trying to SET**. That is the same silent-wrong-action class the reset exists to remove, and the identical hazard was caught in self-review on the table-level path during #1434's PR (the gate there tests RAW key presence); this closes the per-GSI half, which #1434 deliberately left alone because it needed more than a one-line gate change. **The fix** adds `collectRawOnDemandDeclarations(properties, region)` beside `toSdkGlobalSecondaryIndexes` — a per-`IndexName` map of RAW CFn key presence walking the SAME two sources the translation does (top-level GSI for the write half; the LOCAL replica's index entry for the read half, with the GSI-level spelling as the documented hand-authored fallback). Keeping the two functions adjacent is deliberate: they must agree on where each member lives, and a divergence between them is exactly what would re-open the bug. A missing entry means "not declared", which lets the reset FIRE — the DESTRUCTIVE direction, NOT a conservative one (the first draft's comment claimed the opposite and review caught it). Two different mechanisms keep that safe: a non-array `GlobalSecondaryIndexes` is refused loudly by `toSdkGlobalSecondaryIndexes`, while a malformed per-ENTRY shape is deliberately passed through untouched but carries no usable `IndexName`, so the reset loop skips it before consulting the map. **Be precise about what the guard buys** — review probed the PR tip and disproved the obvious claim: it does NOT put the value back on a path that reaches AWS. The coercion happens upstream in `toSdkGlobalSecondaryIndexes`, so the unparseable value is already gone before the reset loop and NOTHING is sent for that member either way. What the guard removes is the DESTRUCTIVE half; what remains is a no-op — and a silent no-op is its own trap, since the only output was the immutable-field warning further down, which tells the user to RECREATE THE INDEX (useless and alarming advice for an unresolved intrinsic). The suppression therefore now WARNS, naming the member and saying the limit was left unchanged. Review also caught a **source divergence that reintroduced the very bug**: an already-SDK-shaped `gsi['OnDemandThroughput']` WINS over the derived members in the translation, so reading the derived spellings reported a member DECLARED that the translation never sends — an explicit `{MaxReadRequestUnits: 50}` beside a leftover `WriteOnDemandThroughputSettings` suppressed the write reset and left the old ceiling live, the #1160 silent drop re-created by the guard itself. The collector now derives both flags from the explicit block alone when present. **Tests:** 6 units (unresolved intrinsic on the write half; on the read half via the replica entry; the GSI-level read fallback spelling counted as a declaration; the accurate warning; the explicit-block precedence; and a fence that a GENUINE removal still resets, so the guard narrows the reset rather than disabling it). Revert-proofed — neutering the raw-presence guard fails three, and neutering the explicit-block precedence fails its own test, while the #1423 fence stays green throughout.
+- ✅ **`AWS::DynamoDB::GlobalTable`: the per-GSI on-demand reset stops firing on a present-but-unresolvable value (issue [#1440](https://github.com/go-to-k/cdkd/issues/1440))** — `src/provisioning/providers/dynamodb-globaltable-provider.ts` + `tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts`. **The class:** the #1423 per-GSI reset decided "the template DROPPED this member" from the TRANSLATED side, and `toSdkGlobalSecondaryIndexes` runs every value through `toFiniteNumber` — which returns `undefined` for a present-but-unparseable value (an unresolved `{Ref: …}`, `''`, an object). Such a GSI was therefore indistinguishable from a removal and was answered with the `-1` sentinel, **silently CLEARING the ceiling the template was trying to SET**. That is the same silent-wrong-action class the reset exists to remove, and the identical hazard was caught in self-review on the table-level path during #1434's PR (the gate there tests RAW key presence); this closes the per-GSI half, which #1434 deliberately left alone because it needed more than a one-line gate change. **The fix** adds `collectRawOnDemandDeclarations(properties, region)` beside `toSdkGlobalSecondaryIndexes` — a per-`IndexName` map of RAW CFn key presence walking the SAME two sources the translation does (top-level GSI for the write half; the LOCAL replica's index entry for the read half, with the GSI-level spelling as the documented hand-authored fallback). Keeping the two functions adjacent is deliberate: they must agree on where each member lives, and a divergence between them is exactly what would re-open the bug. A missing entry means "not declared", which lets the reset FIRE — the DESTRUCTIVE direction, NOT a conservative one (the first draft's comment claimed the opposite and review caught it). Two different mechanisms keep that safe: a non-array `GlobalSecondaryIndexes` is refused loudly by `toSdkGlobalSecondaryIndexes`, while a malformed per-ENTRY shape is deliberately passed through untouched but carries no usable `IndexName`, so the reset loop skips it before consulting the map. **Be precise about what the guard buys** — review probed the PR tip and disproved the obvious claim: it does NOT put the value back on a path that reaches AWS. The coercion happens upstream in `toSdkGlobalSecondaryIndexes`, so the unparseable value is already gone before the reset loop and NOTHING is sent for that member either way. What the guard removes is the DESTRUCTIVE half; what remains is a no-op — and a silent no-op is its own trap, since the only output was the immutable-field warning further down, which tells the user to RECREATE THE INDEX (useless and alarming advice for an unresolved intrinsic). The suppression therefore now WARNS, naming the member and saying the limit was left unchanged. Review also caught a **source divergence that reintroduced the very bug**: an already-SDK-shaped `gsi['OnDemandThroughput']` WINS over the derived members in the translation, so reading the derived spellings reported a member DECLARED that the translation never sends — an explicit `{MaxReadRequestUnits: 50}` beside a leftover `WriteOnDemandThroughputSettings` suppressed the write reset and left the old ceiling live, the #1160 silent drop re-created by the guard itself. The collector now derives both flags from the explicit block alone when present. A third review pass then folded in the BLOCK-level case: a block that is ITSELF an unresolved intrinsic (`{"Fn::If": […]}`) is a record with no member, so the member test reported "not declared" and fired the destructive reset — #1440 one level up from where the guard looked. `isUnresolvedIntrinsicBlock` (all keys `Ref` / `Fn::*`) routes it into the same suppression + warning, while a genuine `{}` still counts as a removal. The same pass made the non-string `IndexName` skip EXPLICIT in the modified loop — such an entry had been safe only by a Map identity miss downstream, an accident rather than a guarantee. **Tests:** 9 units (unresolved intrinsic on the write half; on the read half via the replica entry; the GSI-level read fallback spelling counted as a declaration; the accurate warning; no misleading recreate-the-index advice alongside it; the explicit-block precedence; a block-level intrinsic treated as declared; a genuine empty block still treated as a removal; and a fence that a GENUINE removal still resets, so the guard narrows the reset rather than disabling it). Revert-proofed — neutering the raw-presence guard fails three, and neutering the explicit-block precedence fails its own test, while the #1423 fence stays green throughout.
 - ✅ **`AWS::DynamoDB::GlobalTable`: the TABLE-level on-demand write ceiling resets when the template drops it; the two REPLICA overrides are proven un-resettable and deliberately left alone (issue [#1434](https://github.com/go-to-k/cdkd/issues/1434))** — `src/provisioning/providers/dynamodb-globaltable-provider.ts` + `tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts`. **The class:** removing `WriteOnDemandThroughputSettings` from a template never set `flatChanged`, so no `UpdateTable` went out and the old ceiling stayed live in AWS while cdkd reported success — the absent-field-reset silent drop (#1160), the table-level sibling of the per-GSI case #1423 closed. **The fix** merges per member rather than branching on "the whole block disappeared" (the shape the #1433 review caught: a block can survive with its member dropped, which is the likelier user edit) and is SUPPRESSED while the billing mode is flipping, since dropping the on-demand block on the way to PROVISIONED is the natural template edit rather than a clear request, and step 4's flip owns that call. The `oldBilling` / `newBilling` pair is now resolved ONCE above step 3 and reused by step 4, so the two sites cannot disagree about what "flipping" means. **The issue asked for three fields; only ONE turned out to be implementable, and the live probes are what settled it** (us-east-1 source + us-west-2 replica, one global table per probe, both torn down — transcripts on the issue). Table-level `-1` behaves exactly as the issue assumed: `{MaxWriteRequestUnits: -1}` was accepted and `DescribeTable` then returned `{MaxReadRequestUnits: 100}`, i.e. the dropped member cleared and the untouched sibling preserved, with the reset reading back as ABSENCE never as -1 — verified independently rather than inherited from #1423, since reset semantics are field-specific. The two REPLICA overrides have NO reset mechanism at all: `OnDemandThroughputOverride: {MaxReadRequestUnits: -1}` is accepted but **stored literally as -1** on readback (writing it would be strictly worse than the current no-op — a nonsense value instead of a stale-but-valid one); the documented "empty override means inherit the source table" form `{}` is accepted and then **hangs the table in `UPDATING` for over an hour** with the override unchanged and every later call returning `ResourceInUseException` (it left a table AWS refused to delete for having "acted as a source region for new replica(s) ... in the last 24 hours" — treat `{}` as hazardous in any future probe); an entry carrying only `IndexName` is rejected outright (`ValidationException: There are no actions specified in the Replica Update Action`). `ProvisionedThroughputOverride` is the same story by construction — the registry schema declares `ReadCapacityUnits` / `MaxReadRequestUnits` `"minimum": 1`, and live `-1` / `0` both fail validation while `{}` returns `InternalServerError`. So no in-band sentinel exists, and shipping any of them would trade a silent no-op for a live defect. **Tests:** 6 units covering the full removal, the PARTIAL removal, value-changed (never the sentinel), stringly-typed coercion, billing-flip suppression, and the no-change redeploy; revert-proofed — neutering the reset fails exactly the first two while the other four stay green, since they pin behavior the change does not alter. **Related finding filed, not fixed here:** the probes surfaced that the table-level on-demand READ ceiling (`Replicas[local].ReadOnDemandThroughputSettings`, i.e. the canonical `Billing.onDemand({maxReadRequestUnits})`) is never wired AT ALL — dropped on the way in rather than merely un-reset, and the repo's own verbatim-`cdk synth` fixture already carried the value with no assertion over it (issue [#1436](https://github.com/go-to-k/cdkd/issues/1436)).
 
 **Recently Implemented** (2026-08-09):

src/provisioning/providers/dynamodb-globaltable-provider.ts

@@ -1338,7 +1338,12 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider {
         await this.waitForTableActiveAfterUpdate(physicalId, logicalId);
       }
       for (const gsi of gsiDiff.modified) {
-        if (!gsi.IndexName) continue;
+        // Explicit `typeof` rather than truthiness: an unresolved-intrinsic
+        // `IndexName` is a truthy OBJECT, and everything downstream (the
+        // declaration map, `previousSdkByName`) is keyed by string. Such an
+        // entry used to fall through and be safe only by a Map identity miss —
+        // an accident, not a guarantee.
+        if (typeof gsi.IndexName !== 'string' || gsi.IndexName.length === 0) continue;
         // Already applied atomically with the BillingMode flip above.
         if (gsiHandledByBillingFlip.has(gsi.IndexName)) continue;
         // A BillingMode flip re-shapes EVERY index by construction: the two
@@ -3130,6 +3135,24 @@ export function toSdkGlobalSecondaryIndexes(
   return result;
 }
 
+/**
+ * True when a CFn sub-object is nothing but an unresolved intrinsic
+ * (`{"Fn::If": […]}` / `{"Ref": …}`), i.e. the template DID declare the block
+ * but its value could not be resolved to a shape with members.
+ *
+ * Needed because presence is otherwise tested on the MEMBER key: an
+ * intrinsic-valued block IS a record, so `asRecord` succeeds while
+ * `['MaxReadRequestUnits']` is undefined — reporting "not declared" and firing
+ * the destructive `-1` reset on a ceiling the template was trying to SET. That
+ * is issue #1440 one level up from where the member test looks.
+ */
+function isUnresolvedIntrinsicBlock(value: unknown): boolean {
+  const record = asRecord(value);
+  if (!record) return false;
+  const keys = Object.keys(record);
+  return keys.length > 0 && keys.every((k) => k === 'Ref' || k.startsWith('Fn::'));
+}
+
 /** Which on-demand members a GSI's CFn side actually DECLARES (issue #1440). */
 export interface RawOnDemandDeclaration {
   readonly readDeclared: boolean;
@@ -3210,18 +3233,28 @@ export function collectRawOnDemandDeclarations(
       });
       continue;
     }
+    // A block that is ITSELF an unresolved intrinsic counts as declared: the
+    // member test cannot see into it, and reporting "not declared" would fire
+    // the destructive reset on a ceiling the template was trying to set.
+    const readBlocks = [
+      localEntry?.['ReadOnDemandThroughputSettings'],
+      gsi['ReadOnDemandThroughputSettings'],
+    ];
+    const writeBlock = gsi['WriteOnDemandThroughputSettings'];
     out.set(name, {
       // Same sources as the SDK translation: replica entry first, GSI-level
       // spelling as the hand-authored fallback. Declared in EITHER place
       // counts — the translation resolves "first PARSEABLE", so an OR here is
       // a safe superset for suppression (it can never report not-declared for
       // a member the translation did resolve).
-      readDeclared:
-        asRecord(localEntry?.['ReadOnDemandThroughputSettings'])?.['MaxReadRequestUnits'] !==
-          undefined ||
-        asRecord(gsi['ReadOnDemandThroughputSettings'])?.['MaxReadRequestUnits'] !== undefined,
+      readDeclared: readBlocks.some(
+        (block) =>
+          asRecord(block)?.['MaxReadRequestUnits'] !== undefined ||
+          isUnresolvedIntrinsicBlock(block)
+      ),
       writeDeclared:
-        asRecord(gsi['WriteOnDemandThroughputSettings'])?.['MaxWriteRequestUnits'] !== undefined,
+        asRecord(writeBlock)?.['MaxWriteRequestUnits'] !== undefined ||
+        isUnresolvedIntrinsicBlock(writeBlock),
     });
   }
   return out;

tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts

@@ -1368,6 +1368,42 @@ describe('DynamoDBGlobalTable GSI throughput translation (issue #1387)', () => {
       expect(onDemand?.['MaxWriteRequestUnits']).toBe(-1);
     });
 
+    it('treats a BLOCK-level unresolved intrinsic as declared, not as a removal', async () => {
+      // Third-review catch (#1444 item D, folded in): presence was tested on
+      // the MEMBER key, so `WriteOnDemandThroughputSettings: {"Fn::If": [...]}`
+      // — a record with no MaxWriteRequestUnits — reported "not declared" and
+      // fired the destructive -1 on a ceiling the template was trying to set.
+      const previous = structuredClone(ON_DEMAND_TABLE_PROPS) as Record<string, unknown>;
+      const next = structuredClone(ON_DEMAND_TABLE_PROPS) as Record<string, unknown>;
+      (next['GlobalSecondaryIndexes'] as Record<string, unknown>[])[0]![
+        'WriteOnDemandThroughputSettings'
+      ] = { 'Fn::If': ['SomeCondition', { MaxWriteRequestUnits: 60 }, { Ref: 'AWS::NoValue' }] };
+
+      await provider.update('OnDemand', 'od-table', RESOURCE_TYPE, next, previous);
+
+      const onDemand = gsiUpdateAction()?.['OnDemandThroughput'] as
+        | Record<string, unknown>
+        | undefined;
+      expect(onDemand?.['MaxWriteRequestUnits']).not.toBe(-1);
+    });
+
+    it('does not treat a REAL block that merely lacks the member as an intrinsic', async () => {
+      // The intrinsic test must not become a blanket "any block counts" —
+      // a genuine `{}` / sibling-only block IS a removal of the member.
+      const previous = structuredClone(ON_DEMAND_TABLE_PROPS) as Record<string, unknown>;
+      const next = structuredClone(ON_DEMAND_TABLE_PROPS) as Record<string, unknown>;
+      (next['GlobalSecondaryIndexes'] as Record<string, unknown>[])[0]![
+        'WriteOnDemandThroughputSettings'
+      ] = {};
+
+      await provider.update('OnDemand', 'od-table', RESOURCE_TYPE, next, previous);
+
+      const onDemand = gsiUpdateAction()?.['OnDemandThroughput'] as
+        | Record<string, unknown>
+        | undefined;
+      expect(onDemand?.['MaxWriteRequestUnits']).toBe(-1);
+    });
+
     it('STILL resets a genuinely removed member (the #1423 behavior is intact)', async () => {
       // The guard must narrow the reset to real removals only — not disable it.
       const previous = structuredClone(ON_DEMAND_TABLE_PROPS) as Record<string, unknown>;