chore: open-source readiness (docs, attribution, hardening) - #3
Merged
Conversation
Prepares the repo for OSS publication per the Fluent Health policy: - Add CONTRIBUTING.md, SECURITY.md, NOTICE (Apache-2.0 derivative attribution to FeatBit) - Add catalog-info.yaml with fluentinhealth.com/oss: pending annotation - README: add Contributing section + Fluent Health attribution link - Harden .gitignore with secret/credential patterns - Scrub internal repo reference (gravitee-policy-token-exchange) from CI comments Security audit (secrets / internal refs / PII / dependencies): PASS. LICENSE remains Apache-2.0 (required for a FeatBit derivative). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
4 tasks
Devild007
added a commit
that referenced
this pull request
Jun 25, 2026
Pre-push adversarial audit (superpowers:code-reviewer) returned 6 findings against the perf-optimization commits 316d195/901b955/bc546fb. This commit addresses them; remaining items are documented push-backs. MEDIUM — GetUserFlags silently empty on `{"data": null}`: The single-pass typed envelope changed `data` to nullable with `= null` default. Under kotlinx-serialization 1.6.3 + the project's `explicitNulls = false` config, JSON null on a nullable field coerces to Kotlin null WITHOUT throwing — re-introducing the "indistinguishable from no flags, serve defaults forever" failure the old AST code guarded against. Empirically verified with a probe test against the pinned kotlinx version. Fix: make `data` non-nullable with a `LatestAllData()` default. Absent field still defaults to empty (matches old behavior); explicit JSON null now throws SerializationException, caught + logged by `safePoll`. Regression test added. MEDIUM — fast-path insight emission untested: The `value and detail getters agree` test ran against `offlineClientWith` where `insightsEnabled == false`, so a mutation dropping `insights.offer(...)` from `evaluateValue`'s Found branch slid silently. Added MockWebServer-based test that drives an online polling client, calls `boolVariation()`, and verifies an insight POST hits `/api/public/insight/`. Mutation-verified — removing the emit kills the test. MEDIUM — fast-path guard not exercised: Added paired negative test that confirms zero insight POSTs reach the server when the client is pre-init without bootstrap. MEDIUM — MemoryStore default-method binary compat: `public interface MemoryStore` gained `upsertAll` as a default method. Under Kotlin 1.9's default `-Xjvm-default=disable`, the body lands in `DefaultImpls` and the interface bytecode has the method as abstract — old Java binary implementers would `AbstractMethodError` on `store.upsertAll`. The SDK does not currently expose a `MemoryStore` injection point but the interface IS public per the clean-arch domain-port carve-out. Fix: enable `-Xjvm-default=all-compatibility` module-wide. Default-method bodies now land in interface bytecode (real Java 8 defaults) with a `DefaultImpls` fallback for legacy binaries. NIT — FBUser endUser defensive immutability: Wrap cached `customizedProperties` in `Collections.unmodifiableList` so SDK-internal code can't `as MutableList` and corrupt the per-FBUser-lifetime cache. NIT — corrected allocation-count comment "~3 → ~4" in FBClientImpl (forgot the `listOf(VariationInsight.of(...))` singleton wrapper). NIT — documented the directional improvement in `DefaultMemoryStore.upsertAll` listener-throw semantics: bulk path commits ALL writes under the lock before any listener fires, so a throwing listener leaves the store in its full post-batch state, not partially written. NIT — removed dead import `kotlinx.serialization.encodeToString` in GetUserFlags (call site uses StringFormat member fn, not the reified inline extension). NIT — MockWebServer race on the fast-path insight test: `takeRequest(1ms)` after `client.close()` raced the in-flight HTTP POST landing on the server. Replaced with a bounded poll loop (8 × 250ms = 2s budget) that breaks on first insight hit. Push-backs (documented in chat, not in code): * FBClientImpl.close insights-timeout nesting (pre-existing, not introduced by perf pass — out of scope). * DefaultMemoryStore `listeners.isEmpty()` short-circuit observability (author's existing Kdoc already concedes code-review -only coverage). Audit loop ended on a zero-finding pass per CLAUDE.md rule #3. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Prepares this repo for open-source publication per the Fluent Health OSS policy. Generated/updated the required files and ran the security audit. The actual make-public toggle and CTO approval are left to a maintainer.
Audit result (LLM-driven, code + history)
.env, keys, service accounts,sm://, or private-key material in tree or historygravitee-policy-token-exchangemention in CI commentstest@featbit.comis FeatBit's public demo account (from their Apache-2.0 schema)Files added / changed
fluentinhealth.com/oss: pending(flip topublishedafter going public)License
Kept Apache-2.0 (this is a derivative of FeatBit's Apache-2.0 SDK — MIT would not be appropriate). Attribution is in
NOTICE. Worth a CTO confirmation given it's a derivative work.Follow-ups for a maintainer (cannot be done in a PR)
GitHub settings are currently: secret scanning disabled, Dependabot security updates disabled, no branch ruleset on
main. Enable after/while publishing:A CTO approval issue accompanies this PR.
🤖 Generated with Claude Code