Skip to content

Commit 05dd11b

Browse files
authored
feat: complete SDK feature flag registry and add drift guard (#316) (#420)
1 parent 58d5d16 commit 05dd11b

5 files changed

Lines changed: 199 additions & 5 deletions

File tree

.github/checklists/issue-316.md

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# Acceptance Criteria Checklist — Issue #316
2+
3+
> Generated for pre-PR verification. Confirm each item, then run:
4+
>
5+
> ```bash
6+
> npm run verify:pr -- --checklist .github/checklists/issue-316.md
7+
> ```
8+
9+
## Issue
10+
11+
- **Number:** #316
12+
- **Title:** Implement SDK feature flag framework
13+
14+
## Acceptance Criteria
15+
16+
- [x] Feature flag framework is implemented
17+
- [x] Configuration source metadata is represented safely
18+
- [x] Experimental features can be disabled by default
19+
- [x] Unsupported disabled states return typed errors
20+
- [x] Diagnostics include non-sensitive config source information
21+
- [x] Tests cover enabled and disabled feature paths
22+
- [x] Documentation explains feature stability
23+
24+
## Contributor confirmations
25+
26+
- [x] Automated checks passed (`npm run verify:pr`)
27+
- [x] Tests added or updated for behaviour changes
28+
- [x] Documentation updated when public behaviour changed
29+
- [x] PR description maps each acceptance criterion to the change
30+
- [x] No secrets or `.env` values committed

docs/feature-flags.md

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,30 @@ This SDK uses a **Feature Flag Framework** to gate experimental, in-flight, or o
1919

2020
## Registered Experimental Feature Flags
2121

22-
| Feature Flag Key | Module | Purpose | Default State |
22+
Every registered flag defaults to `false`. A flag that gates a code path is
23+
listed as **Active**; one kept for a planned capability that no code consults
24+
yet is listed as **Reserved**, so the table never implies a capability the SDK
25+
does not have.
26+
27+
| Feature Flag Key | Module | Status | Purpose |
2328
|---|---|---|---|
24-
| `experimentalVault` | `vault` | Batch operations and experimental Soroban savings vault helpers (`executeExperimentalVaultBatch`). | Disabled (`false`) |
25-
| `experimentalSorobanEvents` | `soroban` | Soroban contract event polling (`querySorobanEvents`). | Disabled (`false`) |
26-
| `experimentalMultiAssetVault` | `vault` | Experimental multi-asset vault deposit/withdraw support. | Disabled (`false`) |
27-
| `experimentalAsyncSigner` | `account` | Experimental remote / async signer interface. | Disabled (`false`) |
29+
| `experimentalVault` | `vault` | Active | Batch operations and experimental Soroban savings vault helpers (`executeExperimentalVaultBatch`). |
30+
| `experimentalSorobanEvents` | `soroban` | Active | Soroban contract event polling (`querySorobanEvents`). |
31+
| `experimentalVaultLocks` | `vault` | Active | Vault lock intents — lock creation, lock listing and matured-lock withdrawal. Enabling it does **not** make them work: the capability is registered as `planned` and the actions return `UnsupportedFeatureError`. See [Vault capabilities](./vault-capabilities.md). |
32+
| `experimentalMultiAssetVault` | `vault` | **Reserved** | Multi-asset vault deposit/withdraw support. No code path consults this flag today. |
33+
| `experimentalAsyncSigner` | `account` | **Reserved** | Remote / async signer interface. No code path consults this flag today. |
34+
35+
### Registry completeness
36+
37+
A flag that gates code but is missing from `DEFAULT_FEATURE_FLAGS` still
38+
resolves to `false`, so nothing breaks — but no consumer can discover it and
39+
`config.resolved` diagnostics will not report its state. That drift is silent by
40+
construction, and it happened: `experimentalVaultLocks` is passed as a variable
41+
rather than a literal, so it never appeared in a search of call sites.
42+
43+
`tests/feature-flag-registry.test.ts` now scans `src/` for `experimental*` flag
44+
keys and fails when one is unregistered or undocumented. Adding a flag without
45+
registering it is a test failure rather than a silent omission.
2846

2947
---
3048

src/config/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,7 @@ export const DEFAULT_FEATURE_FLAGS: Record<string, boolean> = {
208208
experimentalSorobanEvents: false,
209209
experimentalMultiAssetVault: false,
210210
experimentalAsyncSigner: false,
211+
experimentalVaultLocks: false,
211212
};
212213

213214
/**

src/types/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ export type FeatureFlagKey =
3434
| 'experimentalSorobanEvents'
3535
| 'experimentalMultiAssetVault'
3636
| 'experimentalAsyncSigner'
37+
| 'experimentalVaultLocks'
3738
| (string & {});
3839

3940
/** Map of feature flag keys to boolean enablement status. */
Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
/**
2+
* Feature flag registry completeness (issue #316).
3+
*
4+
* The framework itself — resolution, precedence, source metadata, typed
5+
* disabled-feature errors, diagnostics integration — landed in #355. What it
6+
* had no defence against is **drift**: a flag can gate a code path without ever
7+
* being registered, and nothing notices.
8+
*
9+
* That is not hypothetical. `experimentalVaultLocks` gates the vault lock
10+
* actions through `readiness.featureFlag` in `src/vault/intents.ts`, a variable
11+
* rather than a literal, so it never appeared in a grep of call sites and was
12+
* absent from `DEFAULT_FEATURE_FLAGS`, `FeatureFlagKey` and the docs table.
13+
*
14+
* These tests are the canary. Adding a flag without registering it now fails
15+
* here instead of silently shipping a flag no consumer can discover.
16+
*/
17+
18+
import { describe, it, expect } from 'vitest';
19+
import fs from 'node:fs';
20+
import path from 'node:path';
21+
import { DEFAULT_FEATURE_FLAGS, isFeatureEnabled } from '../src/config';
22+
23+
const SRC = path.resolve(__dirname, '..', 'src');
24+
25+
/** Every `.ts` file under `src/`. */
26+
function sourceFiles(dir: string, acc: string[] = []): string[] {
27+
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
28+
const full = path.join(dir, entry.name);
29+
if (entry.isDirectory()) sourceFiles(full, acc);
30+
else if (entry.name.endsWith('.ts')) acc.push(full);
31+
}
32+
return acc;
33+
}
34+
35+
/**
36+
* Flag keys referenced anywhere in `src/`.
37+
*
38+
* Matches the `'experimental…'` string literal itself rather than the call
39+
* site, because a flag can be passed as a variable — which is exactly how the
40+
* one drifting flag escaped detection.
41+
*/
42+
function referencedFlagKeys(): Map<string, string[]> {
43+
const found = new Map<string, string[]>();
44+
for (const file of sourceFiles(SRC)) {
45+
const contents = fs.readFileSync(file, 'utf8');
46+
for (const match of contents.matchAll(/['"](experimental[A-Za-z0-9]+)['"]/g)) {
47+
const key = match[1]!;
48+
const rel = path.relative(SRC, file).replace(/\\/g, '/');
49+
const files = found.get(key) ?? [];
50+
if (!files.includes(rel)) files.push(rel);
51+
found.set(key, files);
52+
}
53+
}
54+
return found;
55+
}
56+
57+
describe('registry completeness', () => {
58+
it('registers every experimental flag the source code references', () => {
59+
const referenced = referencedFlagKeys();
60+
const registered = new Set(Object.keys(DEFAULT_FEATURE_FLAGS));
61+
62+
const unregistered = [...referenced.entries()]
63+
.filter(([key]) => !registered.has(key))
64+
.map(([key, files]) => `${key} (referenced in ${files.join(', ')})`);
65+
66+
expect(
67+
unregistered,
68+
'Flags gate code but are missing from DEFAULT_FEATURE_FLAGS. ' +
69+
'An unregistered flag still defaults to false, but no consumer can ' +
70+
'discover it and diagnostics will not report its state.',
71+
).toEqual([]);
72+
});
73+
74+
it('includes the vault lock flag that previously drifted', () => {
75+
// Regression guard for the specific gap this issue closes.
76+
expect(DEFAULT_FEATURE_FLAGS).toHaveProperty('experimentalVaultLocks');
77+
expect(referencedFlagKeys().has('experimentalVaultLocks')).toBe(true);
78+
});
79+
});
80+
81+
describe('safe defaults', () => {
82+
it('defaults every registered flag to false', () => {
83+
for (const [key, value] of Object.entries(DEFAULT_FEATURE_FLAGS)) {
84+
expect(value, `${key} must be disabled by default`).toBe(false);
85+
}
86+
});
87+
88+
it('reports an unregistered flag as disabled rather than throwing', () => {
89+
// An unknown key resolves to false, which is why the drift was silent.
90+
expect(isFeatureEnabled('experimentalSomethingNobodyRegistered')).toBe(false);
91+
});
92+
93+
it('resolves a registered flag as disabled unless enabled explicitly', () => {
94+
expect(isFeatureEnabled('experimentalVaultLocks')).toBe(false);
95+
expect(
96+
isFeatureEnabled('experimentalVaultLocks', {
97+
featureFlags: { experimentalVaultLocks: true },
98+
}),
99+
).toBe(true);
100+
});
101+
});
102+
103+
describe('documentation matches the registry', () => {
104+
it('documents every registered flag in docs/feature-flags.md', () => {
105+
const docs = fs.readFileSync(
106+
path.resolve(__dirname, '..', 'docs', 'feature-flags.md'),
107+
'utf8',
108+
);
109+
110+
const undocumented = Object.keys(DEFAULT_FEATURE_FLAGS).filter(
111+
(key) => !docs.includes(`\`${key}\``),
112+
);
113+
114+
expect(
115+
undocumented,
116+
'Registered flags missing from the documented table.',
117+
).toEqual([]);
118+
});
119+
});
120+
121+
describe('registered flags that gate nothing are declared as reserved', () => {
122+
it('lists which registered flags have no reference in src/', () => {
123+
// Not a failure: a reserved key is a legitimate placeholder, and removing
124+
// one would be a breaking change to a published union. It is documented
125+
// rather than silently implying a capability that does not exist — the same
126+
// reasoning the capability registry applies to `planned` entries.
127+
const referenced = referencedFlagKeys();
128+
const reserved = Object.keys(DEFAULT_FEATURE_FLAGS).filter(
129+
(key) => !referenced.has(key),
130+
);
131+
132+
const docs = fs.readFileSync(
133+
path.resolve(__dirname, '..', 'docs', 'feature-flags.md'),
134+
'utf8',
135+
);
136+
137+
for (const key of reserved) {
138+
expect(
139+
docs.includes(`\`${key}\``) && /Reserved/i.test(docs),
140+
`${key} gates no code path and must be marked Reserved in the docs`,
141+
).toBe(true);
142+
}
143+
});
144+
});

0 commit comments

Comments
 (0)