Skip to content

ci: Run the Flagsmith SDK behavioural spec - #99

Merged
Zaimwa9 merged 1 commit into
mainfrom
feat/sdk-spec
Oct 21, 2026
Merged

Zaimwa9 merged 1 commit into
mainfrom
feat/sdk-spec

Conversation

@Zaimwa9

@Zaimwa9 Zaimwa9 commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Runs the Flagsmith SDK behavioural spec against this SDK on every pull request and push to main.

  • sdk-spec/steps.dart: a function creating the SDK client, plus one function per When step in the spec's features, each below a comment holding the step's text. Flag values come from Flag.value (feat: Expose flag values with their JSON type #100), so they keep their JSON types. It doubles as a CI-checked usage example.
  • sdk-spec/Dockerfile: builds the SDK from source and runs the steps file under the spec's Dart harness. The SDK depends on Flutter, so the harness entrypoint is compiled with flutter build bundle and runs on flutter_tester, the headless engine flutter test uses.
  • .github/workflows/sdk-spec.yml: calls the spec's reusable workflow with language: dart. The workflow starts a real Flagsmith instance, seeds it through the Admin API and runs the scenarios through the steps file.
  • renovate.json: lets Renovate bump the uses: ref and spec-ref together, through one regex manager matching both. Only the SDK spec is enabled, so Renovate opens no other PRs.

The spec currently covers getting environment flags and getting identity flags with traits.

Spec ref

The workflow is pinned to spec v0.5.0, the first release with the Dart harness (Flagsmith/sdk-spec#6). Renovate bumps both refs together from here.

@Zaimwa9

Zaimwa9 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

@themis-blindfold review

1 similar comment
@Zaimwa9

Zaimwa9 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

@themis-blindfold review

Comment thread spec-adapter/adapter.dart Outdated
Comment thread spec-adapter/adapter.dart Outdated
@themis-blindfold

Copy link
Copy Markdown

⚖️ Themis review: 🟠 Fix before merge

The new behavioural-spec job is currently failing because the adapter serialises number and boolean flag values as strings. It also cannot submit a protocol-valid identity request containing a null trait value, so the adapter does not yet cover the full advertised value contract.

Area Score
🎯 Correctness 2/5
🧪 Test coverage 2/5
📐 Code quality 3/5
🚀 Product impact 3/5

🟠 Majors

📝 Walkthrough
  • SDK spec workflow - adds the reusable behavioural-spec job for pull requests and main.
  • Adapter service - creates isolated in-memory clients and translates environment and identity evaluations into the spec response shape.
🧪 How to verify
  1. Build the adapter from the repository root with docker build -f spec-adapter/Dockerfile ..
  2. Run the adapter against a mock Flagsmith API returning string, number, boolean, and null flag values; confirm each response preserves its JSON type.
  3. Send POST /clients/{clientId}/identity-flags with {"traits":{"plan":null}}; confirm the Flagsmith request succeeds and carries the null value.
  4. Confirm the sdk-spec / Spec v0.1.1 check completes successfully for this ref.
    Automate: add focused adapter contract tests for typed flag values and null trait values.

Product take: Cross-SDK behavioural coverage is a solid improvement, but it cannot be relied on while its checked-in scenarios fail.

🧭 Assumptions & unverified claims

No unverified assumptions or claims.

The new guardrail will earn its keep once its values keep their shape. · reviewed at bd82c6c

@Zaimwa9

Zaimwa9 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

@themis-blindfold review

@Zaimwa9

Zaimwa9 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

@themis-blindfold review

@Zaimwa9
Zaimwa9 force-pushed the feat/sdk-spec branch 2 times, most recently from 16cedeb to dcef837 Compare October 21, 2026 14:42
@Zaimwa9
Zaimwa9 changed the base branch from main to feat/typed-flag-values October 21, 2026 14:52
Comment thread renovate.json Outdated
@themis-blindfold

Copy link
Copy Markdown

⚖️ Themis review: 🟠 Fix before merge

The new behavioural check exposes real SDK compatibility gaps and is currently failing; the release-update configuration would also let its two required pins drift. The package, unit-test, and analysis checks completed successfully, while the SDK-spec check failed.

Area Score
🎯 Correctness 2/5
🧪 Test coverage 3/5
📐 Code quality 3/5
🚀 Product impact 2/5

🟠 Majors

  • sdk-spec/steps.dart — Flag values are still stringified, so the SDK-spec check remains red.
  • sdk-spec/steps.dart — Null trait values are still rejected before the identity request is made.
  • renovate.json — Renovate updates only spec-ref, leaving the reusable-workflow pin behind.
📝 Walkthrough
  • sdk-spec workflow - runs the Dart behavioural spec on pull requests and pushes to main.
  • sdk-spec/steps.dart - adapts environment and identity flag calls to the spec protocol; its unchanged values expose the SDK's string-only flag values.
  • sdk-spec/Dockerfile - compiles the Flutter harness for the headless Flutter engine.
  • renovate.json - intends to keep the harness workflow and spec refs aligned, but currently tracks only one of them.
🧪 How to verify
  1. Run the SDK-spec workflow and confirm environment and identity scenarios pass with numeric, boolean, and string flag values.
  2. Exercise an identity request with a null trait value and confirm it is forwarded rather than rejected during serialisation.
  3. Run a Renovate dry run against a workflow using matching vX.Y.Z refs and confirm it rewrites both uses: and spec-ref together.
  4. Run flutter test after the SDK compatibility fixes.
    Automate: add a Renovate configuration fixture that asserts a tagged SDK-spec update changes both refs.

Product take: This adds valuable compatibility coverage, but it cannot protect releases while its required check is red.
Fixing the value and null-trait gaps would make it meaningful coverage for the SDK's core flag-fetching paths.

🧭 Assumptions & unverified claims

No unverified assumptions or claims.

The new harness is doing its job; it has found the loose bolts. · reviewed at dcef837

Comment thread sdk-spec/steps.dart
@themis-blindfold

Copy link
Copy Markdown

⚖️ Themis review: 🟠 Fix before merge

The behavioural-spec gate is green and covers the advertised environment and identity flows, but the identity adapter still rejects a nullable value that its own input type accepts, and Renovate cannot maintain the two workflow pins together.

Area Score
🎯 Correctness 3/5
🧪 Test coverage 3/5
📐 Code quality 4/5
🚀 Product impact 4/5

🟠 Majors

  • sdk-spec/steps.dart: Null trait values are rejected before the identity request.
  • renovate.json: Observed: Renovate matches only spec-ref while the GitHub Actions manager that discovers uses: is disabled. Predicted: after moving to a release tag, an update would leave the reusable workflow pinned to the old revision.
📝 Walkthrough
  • Behavioural workflow - runs the pinned Dart spec on pull requests and pushes to main.
  • Dart adapter - maps SDK client creation and environment/identity flag retrieval into the harness steps.
  • Flutter runner - builds the harness entrypoint into a Flutter bundle and executes it with the headless tester.
  • Dependency maintenance - restricts Renovate to the SDK-spec reference.
🧪 How to verify
  1. Invoke the identity step with a trait map containing {"plan": null} and confirm it returns flags rather than raising ArgumentError.
  2. Build and run the spec image with the Dart harness build context, then run the environment and identity scenarios.
  3. Run flutter test after adding the null-trait regression case.
  4. Change both workflow references to a test vX.Y.Z tag and run a Renovate dry run; confirm its update rewrites both fields in one pull request.
    Automate: add a fixture that invokes getIdentityFlags with a null trait and asserts the serialised identity request.

Product take: This is a solid compatibility improvement: it makes the SDK behaviour contract executable on every change. The nullable-trait and pin-maintenance paths need closing so the gate stays representative and maintainable.

🧭 Assumptions & unverified claims
  • The pinned harness could not be fetched in this environment, so whether its current feature set sends a null trait remains unverified.

A useful contract test, once its two loose ends are tied down · reviewed at e556ff1

@Zaimwa9
Zaimwa9 marked this pull request as ready for review October 21, 2026 15:14
@Zaimwa9
Zaimwa9 requested a review from a team as a code owner October 21, 2026 15:14
@Zaimwa9
Zaimwa9 requested review from talissoncosta and removed request for a team October 21, 2026 15:14
@Zaimwa9

Zaimwa9 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

@themis-blindfold review

@themis-blindfold

Copy link
Copy Markdown

⚖️ Themis review: 🟠 Fix before merge

The new SDK behavioural-spec workflow is passing alongside the existing checks, but the null-trait serialization finding remains unresolved. That leaves an important protocol edge case outside the gate’s coverage.

Area Score
🎯 Correctness 3/5
🧪 Test coverage 3/5
📐 Code quality 4/5
🚀 Product impact 3/5

🟠 Majors

  • sdk-spec/steps.dart: null trait values remain rejected by the SDK serializer.
📝 Walkthrough
  • Compatibility workflow - runs the pinned SDK behavioural spec for pull requests and pushes to main.
  • Dart steps - constructs the client and adapts environment and identity flag results to the spec’s response shape.
  • Flutter runner - builds a bundle and launches it with the headless Flutter engine.
  • Dependency updates - configures a grouped Renovate update for the two spec references once they use release tags.
🧪 How to verify
  1. Add and run a regression that calls getFeatureFlags with Trait(key: 'plan', value: null) and asserts the identity request contains "trait_value": null.
  2. Run flutter test to cover the SDK’s trait serialization and flag parsing paths.
  3. Build the runner with docker build -f sdk-spec/Dockerfile --build-context harness=../sdk-spec/harness . and execute the resulting image against a local spec harness.
  4. Confirm the reusable workflow’s SDK-spec job passes for both environment and identity scenarios.
    Automate: add the null-trait case to the behavioural-spec adapter regression suite.

Product take: This adds a valuable compatibility guard for SDK consumers, while the unresolved nullable-trait case leaves an important protocol edge case uncovered. The gate matters most as protection against protocol drift.

🧭 Assumptions & unverified claims

The behavioural spec’s exact contract for null trait values could not be independently checked; the SDK serializer’s rejection of null is directly established.

The spec runner has its Flutter engine; the last nullable trait still needs its ticket stamped · reviewed at cbad7c9

@Zaimwa9

Zaimwa9 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

@themis-blindfold review

Base automatically changed from feat/typed-flag-values to main October 21, 2026 16:13
@themis-blindfold

Copy link
Copy Markdown

⚖️ Themis review: 🟠 Fix before merge

This adds a useful compatibility gate and the recorded SDK-spec, analysis, test, and publish checks all passed. One previously raised compatibility path remains open: null trait values cannot be serialised before an identity evaluation; confirm the pinned spec's trait-value contract and support it if null is valid.

Area Score
🎯 Correctness 3/5
🧪 Test coverage 4/5
📐 Code quality 4/5
🚀 Product impact 3/5

🟠 Majors

  • sdk-spec/steps.dart: Null trait values remain unsupported before identity calls (unverified protocol input).
📝 Walkthrough
  • sdk-spec workflow - runs the Dart behavioural harness for pull requests and pushes to main.
  • Dart steps - maps environment and identity flag evaluations into the spec response shape while preserving typed flag values.
  • Container harness - builds a Flutter bundle and invokes it with the headless Flutter engine.
  • Renovate config - tracks both SDK-spec release references through the custom regex manager.
🧪 How to verify
  1. Run flutter analyze and flutter test from the repository root.
  2. Build the harness image: docker build -t flagsmith-sdk-spec -f sdk-spec/Dockerfile --build-context harness=<sdk-spec>/harness ..
  3. Run the SDK-spec workflow against a PR and verify both environment and identity-with-traits scenarios pass.
  4. Submit a Renovate dry run or update PR and verify it changes both uses: and spec-ref to the same release.
  5. If null is a valid trait value in the pinned protocol, add a null-trait scenario and verify the identity request reaches Flagsmith.

Automate: Keep the SDK-spec workflow as a required check.

Product take: A solid compatibility improvement: regressions in the two covered flag-evaluation paths now surface before merge. Its value depends on keeping the adapter aligned with the spec's full trait contract.

🧭 Assumptions & unverified claims
  • The published v0.5.0 feature definitions could not be retrieved to confirm whether null is a valid input for a trait value; the null-trait major remains unverified on that point.

A new compatibility canary is singing; it just needs to accept every seed it is handed. · reviewed at 43a853e

@Zaimwa9
Zaimwa9 merged commit 62c2442 into main Oct 21, 2026
5 checks passed
@Zaimwa9
Zaimwa9 deleted the feat/sdk-spec branch October 21, 2026 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants