-
Notifications
You must be signed in to change notification settings - Fork 907
feat: add experimental feature-flag system #205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@moonshot-ai/agent-core": minor | ||
| --- | ||
|
|
||
| Add an experimental feature-flag system: a central registry (`flags/registry.ts`) plus an env-driven resolver. Gate a feature with `flags.enabled('id')`, toggled via `KIMI_CODE_EXPERIMENTAL_<NAME>` or the `KIMI_CODE_EXPERIMENTAL_FLAG` master switch. No flags are defined yet. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| import type { ExperimentalFlagMap } from '@moonshot-ai/kimi-code-sdk'; | ||
|
|
||
| // Resolved experimental flags, fetched once from the core over RPC at startup and then read | ||
| // synchronously by the command palette and dispatch. App-local cache, not a source of truth. | ||
| let snapshot: ExperimentalFlagMap = {}; | ||
|
|
||
| /** Replace the cached flag snapshot. Call once after fetching via `harness.getExperimentalFlags()`. */ | ||
| export function setExperimentalFlags(flags: ExperimentalFlagMap): void { | ||
| snapshot = flags; | ||
| } | ||
|
|
||
| /** An `undefined` flag means "not gated" → always enabled, so callers can pass an optional flag id. */ | ||
| export function isExperimentalFlagEnabled(flag: string | undefined): boolean { | ||
| return flag === undefined || snapshot[flag] === true; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| export * from './types'; | ||
| export * from './registry'; | ||
| export * from './resolver'; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| import type { FlagDefinitionInput } from './types'; | ||
|
|
||
| /** | ||
| * Experimental feature flags. Empty by default — there are no experimental features yet. | ||
| * | ||
| * To add one, append an entry and gate the feature with `flags.enabled('my-feature')`: | ||
| * { id: 'my-feature', env: 'KIMI_CODE_EXPERIMENTAL_MY_FEATURE', default: false, surface: 'both' } | ||
| * | ||
| * Keep the `as const satisfies` — it derives the literal `FlagId` union that gives `enabled()` | ||
| * autocomplete and typo-checking. `env` must start with 'KIMI_CODE_EXPERIMENTAL_', be unique, and | ||
| * not equal the master switch 'KIMI_CODE_EXPERIMENTAL_FLAG'; `id` must not be 'flag'. | ||
| */ | ||
| export const FLAG_DEFINITIONS = [] as const satisfies readonly FlagDefinitionInput[]; | ||
|
|
||
| /** Literal union of registered flag ids (currently none → `never`). */ | ||
| export type FlagId = (typeof FLAG_DEFINITIONS)[number]['id']; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| import { parseBooleanEnv } from '#/config/resolve'; | ||
|
|
||
| import { FLAG_DEFINITIONS, type FlagId } from './registry'; | ||
| import type { FlagDefinitionInput } from './types'; | ||
|
|
||
| /** Master switch: when truthy, forces every flag on (highest priority). */ | ||
| export const MASTER_ENV = 'KIMI_CODE_EXPERIMENTAL_FLAG'; | ||
|
|
||
| /** Shared prefix for per-feature variables. */ | ||
| export const EXPERIMENTAL_PREFIX = 'KIMI_CODE_EXPERIMENTAL_'; | ||
|
|
||
| /** | ||
| * Conventional env-name generator: flag id → recommended env variable name. Only used when | ||
| * authoring a new flag (to fill `env`) and for an optional consistency test; NOT used during | ||
| * resolution. | ||
| */ | ||
| export function flagEnvKey(id: string): string { | ||
| return `${EXPERIMENTAL_PREFIX}${id.toUpperCase().replaceAll('-', '_')}`; | ||
| } | ||
|
|
||
| /** | ||
| * Pure, synchronous flag resolver. State comes entirely from (env, registry) and nothing is | ||
| * cached: env is read live on every call, so a single shared instance always reflects the current | ||
| * process env. Defaults to process.env + FLAG_DEFINITIONS; tests can inject a custom env / defs. | ||
| * | ||
| * Precedence (highest wins): | ||
| * L1 master switch KIMI_CODE_EXPERIMENTAL_FLAG → every flag is on | ||
| * L2 per-feature def.env (parseBooleanEnv, may force on or off) | ||
| * L3 registry default | ||
| */ | ||
| export class FlagResolver { | ||
| private readonly env: Readonly<Record<string, string | undefined>>; | ||
| private readonly byId: ReadonlyMap<string, FlagDefinitionInput>; | ||
|
|
||
| constructor( | ||
| env: Readonly<Record<string, string | undefined>> = process.env, | ||
| definitions: readonly FlagDefinitionInput[] = FLAG_DEFINITIONS, | ||
| ) { | ||
| this.env = env; | ||
| this.byId = new Map(definitions.map((def) => [def.id, def])); | ||
| } | ||
|
|
||
| enabled(id: FlagId): boolean { | ||
| const def = this.byId.get(id); | ||
| if (def === undefined) return false; | ||
| if (parseBooleanEnv(this.env[MASTER_ENV]) === true) return true; // L1 master switch | ||
| const override = parseBooleanEnv(this.env[def.env]); // L2 per-feature | ||
| if (override !== undefined) return override; | ||
| return def.default; // L3 default | ||
| } | ||
| } | ||
|
|
||
| export function createFlagResolver( | ||
| env?: Readonly<Record<string, string | undefined>>, | ||
| definitions?: readonly FlagDefinitionInput[], | ||
| ): FlagResolver { | ||
| return new FlagResolver(env, definitions); | ||
| } | ||
|
|
||
| /** | ||
| * Process-global flag accessor. Flags are env-driven and process-global, so a single shared | ||
| * instance (reading live process.env) is the canonical way to consult them — import this directly | ||
| * rather than constructing or injecting a resolver. | ||
| */ | ||
| export const flags = new FlagResolver(); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| import type { FlagId } from './registry'; | ||
|
|
||
| /** Which layer consumes a flag — documentation/grouping only; not used in resolution. */ | ||
| export type FlagSurface = 'core' | 'tui' | 'both'; | ||
|
|
||
| /** Shape of a registry entry (id is a loose string so `as const satisfies` can validate it). */ | ||
| export interface FlagDefinitionInput { | ||
| readonly id: string; | ||
| /** Full environment variable name, e.g. `KIMI_CODE_EXPERIMENTAL_MY_FEATURE`. Read directly by the resolver. */ | ||
| readonly env: string; | ||
| readonly default: boolean; | ||
| readonly surface: FlagSurface; | ||
| } | ||
|
|
||
| /** FlagId-typed view so consumers can fetch a definition by its literal id. */ | ||
| export type FlagDefinition = FlagDefinitionInput & { readonly id: FlagId }; | ||
|
|
||
| /** Resolved enabled-state of every experimental flag (flag id → enabled); used for the SDK snapshot. */ | ||
| export type ExperimentalFlagMap = Record<string, boolean>; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
|
|
||
| import { | ||
| EXPERIMENTAL_PREFIX, | ||
| FLAG_DEFINITIONS, | ||
| MASTER_ENV, | ||
| createFlagResolver, | ||
| flagEnvKey, | ||
| type FlagDefinitionInput, | ||
| type FlagId, | ||
| } from '../../src/flags'; | ||
|
|
||
| // Controlled fake definitions to assert the precedence matrix precisely (independent of the | ||
| // real registry contents). | ||
| const DEFS = [ | ||
| { | ||
| id: 'a-on-default', | ||
| env: 'KIMI_CODE_EXPERIMENTAL_A', | ||
| default: true, | ||
| surface: 'core', | ||
| }, | ||
| { | ||
| id: 'b-off-default', | ||
| env: 'KIMI_CODE_EXPERIMENTAL_B', | ||
| default: false, | ||
| surface: 'tui', | ||
| }, | ||
| ] as const satisfies readonly FlagDefinitionInput[]; | ||
|
|
||
| type Env = Record<string, string | undefined>; | ||
|
|
||
| function make(env: Env) { | ||
| const resolver = createFlagResolver(env, DEFS); | ||
| // The fake ids are not part of the real FlagId union, so cast to FlagId when calling. | ||
| return (id: string) => resolver.enabled(id as FlagId); | ||
| } | ||
|
|
||
| describe('FlagResolver', () => { | ||
| it('L3 default: returns the registry default when env is empty', () => { | ||
| const enabled = make({}); | ||
| expect(enabled('a-on-default')).toBe(true); | ||
| expect(enabled('b-off-default')).toBe(false); | ||
| }); | ||
|
|
||
| it('L2 per-feature on (lenient truthy values)', () => { | ||
| for (const v of ['1', 'true', 'yes', 'on', 'TRUE', ' On ']) { | ||
| expect(make({ KIMI_CODE_EXPERIMENTAL_B: v })('b-off-default')).toBe(true); | ||
| } | ||
| }); | ||
|
|
||
| it('L2 per-feature off (lenient falsy values) overrides default=true', () => { | ||
| for (const v of ['0', 'false', 'no', 'off']) { | ||
| expect(make({ KIMI_CODE_EXPERIMENTAL_A: v })('a-on-default')).toBe(false); | ||
| } | ||
| }); | ||
|
|
||
| it('L2 unparseable value falls back to default', () => { | ||
| expect(make({ KIMI_CODE_EXPERIMENTAL_B: 'maybe' })('b-off-default')).toBe(false); | ||
| expect(make({ KIMI_CODE_EXPERIMENTAL_A: 'maybe' })('a-on-default')).toBe(true); | ||
| }); | ||
|
|
||
| it('L1 master switch: every flag is on when enabled (including default=false)', () => { | ||
| const enabled = make({ [MASTER_ENV]: '1' }); | ||
| expect(enabled('a-on-default')).toBe(true); | ||
| expect(enabled('b-off-default')).toBe(true); | ||
| }); | ||
|
|
||
| it('L1 master switch beats an L2 per-feature off (D2)', () => { | ||
| const enabled = make({ [MASTER_ENV]: '1', KIMI_CODE_EXPERIMENTAL_A: '0' }); | ||
| expect(enabled('a-on-default')).toBe(true); | ||
| }); | ||
|
|
||
| it('master switch is inactive for lenient falsy values', () => { | ||
| const enabled = make({ [MASTER_ENV]: '0' }); | ||
| expect(enabled('b-off-default')).toBe(false); | ||
| }); | ||
|
|
||
| it('reads the env name declared in the registry (the declared name works, others do not)', () => { | ||
| expect(make({ KIMI_CODE_EXPERIMENTAL_B: '1' })('b-off-default')).toBe(true); | ||
| // The name mechanically derived from the id must not take effect (env is explicitly ..._B). | ||
| expect(make({ KIMI_CODE_EXPERIMENTAL_B_OFF_DEFAULT: '1' })('b-off-default')).toBe(false); | ||
| }); | ||
|
|
||
| it('unknown id resolves to false (defensive)', () => { | ||
| expect(make({})('not-a-real-flag')).toBe(false); | ||
| }); | ||
|
|
||
| it('flagEnvKey convention: kebab -> prefix + upper snake', () => { | ||
| expect(flagEnvKey('my-feature')).toBe('KIMI_CODE_EXPERIMENTAL_MY_FEATURE'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('FLAG_DEFINITIONS invariants', () => { | ||
| it('every env satisfies: prefix / unique / not the master switch', () => { | ||
| const seenEnv = new Set<string>(); | ||
| const seenId = new Set<string>(); | ||
| const defs: readonly FlagDefinitionInput[] = FLAG_DEFINITIONS; | ||
| for (const def of defs) { | ||
| expect(def.env.startsWith(EXPERIMENTAL_PREFIX)).toBe(true); | ||
| expect(def.env).not.toBe(MASTER_ENV); | ||
| expect(def.id).not.toBe('flag'); // reserved: would collide with the master switch | ||
| expect(seenEnv.has(def.env)).toBe(false); | ||
| expect(seenId.has(def.id)).toBe(false); | ||
| seenEnv.add(def.env); | ||
| seenId.add(def.id); | ||
| } | ||
| }); | ||
| }); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This changeset only selects
@moonshot-ai/agent-core, but the diff also changesapps/kimi-codestartup/command behavior andpackages/node-sdkpublic API (KimiHarness.getExperimentalFlagsand exported flag types). The repo’sgen-changesetsrules require listing packages that were actually changed and adding@moonshot-ai/kimi-codewhen bundled internal changes enter the CLI; otherwise the release PR can omit the CLI/SDK version and changelog entries for this user-visible/API change.Useful? React with 👍 / 👎.