Repository navigation
feat(flags): serve the typed record only for /flags?v=4 and name it v4 - #123366
Conversation
Clients already send v=3 and arbitrary v values (cache busters, typos) to /flags, so a v >= 3 selector would hand them a new record shape once the setting is on. Only an exact v=4 now selects the typed record; every other parsable version keeps the v2 record and an unparsable one keeps the legacy record, as before. /decide is unchanged. The record, its types and its setting are renamed after the request version: FlagsResponseV4, FlagDetailsV4, serves_flags_v4, the FlagsV4 response_format log value, and FLAGS_V4_RESPONSE_ENABLED / flags_v4_response_enabled (no alias; the old variable is not set anywhere). The vendored harness contract still names its schemas flags_response_v3* until the next harness release is vendored.
|
😎 Merged successfully - details. |
🤖 CI report✅ Trunk lane — non-backend lane (
|
| File | Comment lines | Added lines |
|---|---|---|
rust/feature-flags/src/api/endpoint.rs |
6 | 54 |
rust/feature-flags/src/api/types.rs |
3 | 15 |
rust/feature-flags/src/handler/body_logger.rs |
3 | 19 |
rust/feature-flags/src/flags/flag_matching.rs |
2 | 2 |
rust/feature-flags/tests/test_rules_v2_evaluation/wire.rs |
2 | 3 |
rust/feature-flags/src/config.rs |
1 | 5 |
rust/feature-flags/src/flags/evaluate_v2.rs |
1 | 1 |
rust/feature-flags/src/handler/canonical_log.rs |
1 | 2 |
This check does not block merging. It updates on every push and clears when the share drops.
|
Risk: No findings This change narrows when the feature-flags service serves the typed flag record — only an exact Sentinel reviewed |
There was a problem hiding this comment.
Approved.
The public API change has unit and HTTP regression coverage, and the author's STRONG familiarity supplies the policy's assurance path despite cross-team ownership. No unresolved review concerns or other refusal grounds were found.
- Author wrote 95% of the modified lines and has 15 merged PRs in these paths (familiarity STRONG).
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 166L, 7F substantive, 315L/12F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (315L, 12F, two-areas, feat) |
| stamphog 2.6.1 | .stamphog/policy.yml @ ce6a528 · reviewed head ce6a528 |
Problem
The flags service serves the typed flag record (
key,value,reason,metadata,failed; noenabled/variant) for any non-/decide/flagsrequest withv >= 3whileFLAGS_V3_RESPONSE_ENABLEDis on. Clients already sendv=3and arbitraryvvalues to/flags(cache busters, typos, large numbers), so turning the setting on would hand those clients a record shape they cannot read. Only an exactv=4should select the new record.Changes
vparses to exactly4(serves_flags_v4). On the wire:v=4→ typed record (setting on) or v2 record (setting off).v=3,v>=5, cache-buster numbers that fit in ani32→ v2 record, setting on or off.i32(4.0,4.5,abc, numbers pasti32) → legacy record, as today./decide(X-Original-Endpoint: decide) → unchanged;/decide?v=4still gets the v2 record.FlagsResponseV3/FlagDetailsV3/FlagDetailsMetadataV3→…V4,ServiceResponse::V3→V4,serves_flags_v3→serves_flags_v4, the canonical-logresponse_formatvalueFlagsV3→FlagsV4, andtests/test_flags_v3.rs→tests/test_flags_v4.rs. Wire shape is unchanged.FLAGS_V3_RESPONSE_ENABLED→FLAGS_V4_RESPONSE_ENABLED(config fieldflags_v4_response_enabled), default stillfalse. No alias: the old variable is not set in any deployment. The setting still gates the record exactly as before.v.parse::<i32>().unwrap_or(1)is now oneparse_versionhelper, so the unit test covers the real parse.docs/internal/feature-flags/api-writes.mdnow say/flags?v=4.The vendored harness contract under
rust/feature-flags/tests/fixtures/rules_v2_contract(and the Python copy) is untouched. Its schemas are still namedflags_response_v3*. A harness release renames them, and the next re-vendor picks that up.wire.rsnotes the mismatch.How did you test this code?
test_only_an_exact_v4_selects_the_v4_record(unit, no DB):v=4on/off,v=3on/off,v=5, an 8-digit cache buster,v=2,v=4.0, a 44-digit number,abc,/decidewithv=4andv=3.only_an_exact_v4_selects_the_v4_recordcovers the same cases over HTTP (POST and the GET minimal response). The existing v3 integration tests now requestv=4. The setting-off test also coversv=3,v=4andv=99.v >= 3fails the new unit cases (v3,v5, cache buster) and the new integration test.cargo test -p feature-flags. Lib: 1959 passed. All integration binaries pass, except 3test_flag_definitionsdedicated-Redis tests that need a second Redis at the hard-codedlocalhost:6379, which was not running locally.cargo clippy -p feature-flags --all-targets -- -D warningsandcargo fmt --checkare clean.Rollback
Revert this PR. The setting defaults to off and is not set anywhere, so nothing on the wire changes until it is turned on.
🤖 Generated with Claude Code