fix(ci): fix firebase exception wrapping, missing ADR doc, and gitignore

[kirich1409]

Summary

Three CI failures blocking all open PRs (#143, #144, #145):

Tests & Coverage — FirebaseConfigValueProvider.fetch() was catching RuntimeException and re-throwing it directly, so RuntimeException("Network unavailable") never got wrapped in FetchException. Only CancellationException should propagate unwrapped (coroutine cancellation semantics).

Build Docs — docs/adr/001-configvalues-multi-module.md was referenced in mkdocs.yml nav but both missing from repo AND listed in .gitignore. Removed the ignore rule and added the ADR file.

Analyze Kotlin (CodeQL) — secondary failure caused by the build failures above; should resolve automatically.

Test plan

• Tests & Coverage passes
• Build Docs passes
• Analyze Kotlin passes

🤖 Generated with Claude Code

[qodo-code-review]

Review Summary by Qodo

Fix Firebase exception wrapping and add missing ADR documentation

🐞 Bug fix 📝 Documentation

Walkthroughs

Description

• Fix Firebase exception handling to wrap RuntimeException in FetchException
  - Only CancellationException should propagate unwrapped for coroutine semantics
• Add missing ADR 001 documentation file referenced in mkdocs.yml
• Remove /docs/adr/ from .gitignore to enable version control of ADR files

Diagram

flowchart LR
  A["FirebaseConfigValueProvider.fetch()"] -->|"catch CancellationException"| B["Propagate unwrapped"]
  A -->|"catch other Exception"| C["Wrap in FetchException"]
  D[".gitignore"] -->|"remove /docs/adr/"| E["Enable ADR versioning"]
  F["mkdocs.yml reference"] -->|"add missing file"| G["docs/adr/001-configvalues-multi-module.md"]

Loading

File Changes

1. providers/firebase/src/main/kotlin/dev/androidbroadcast/featured/firebase/FirebaseConfigValueProvider.kt 🐞 Bug fix +2/-1

Fix exception handling in Firebase config fetch

• Import CancellationException from kotlinx.coroutines
• Change exception catch from RuntimeException to CancellationException
• Only unwrapped CancellationException propagates; all other exceptions wrapped in FetchException
• Fixes test failures by properly handling Firebase fetch exceptions

providers/firebase/src/main/kotlin/dev/androidbroadcast/featured/firebase/FirebaseConfigValueProvider.kt

---

2. docs/adr/001-configvalues-multi-module.md 📝 Documentation +21/-0

Add ADR 001 for multi-module ConfigValues design

• Create new ADR document describing multi-module ConfigValues architecture
• Document decision to use single shared ConfigValues instance at app level
• Explain Gradle plugin's FlagRegistrar aggregation mechanism
• List consequences including flat flag namespacing and single provider set

docs/adr/001-configvalues-multi-module.md

---

3. .gitignore ⚙️ Configuration changes +0/-0

Enable version control for ADR documentation

• Remove /docs/adr/ entry to enable version control of ADR files
• Allows ADR documentation to be tracked in repository

.gitignore

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

ⓘ The new review experience is currently in Beta. Learn more

[Copilot]

Pull request overview

Fixes CI failures by correcting Firebase Remote Config exception wrapping, restoring missing MkDocs ADR content, and ensuring docs ADRs aren’t ignored by git.

Changes:

• Update FirebaseConfigValueProvider.fetch() to rethrow coroutine cancellation while wrapping other failures in FetchException.
• Add the missing ADR doc referenced by mkdocs.yml.
• Stop ignoring /docs/adr/ so ADR docs are included in the repo and docs build.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.

File	Description
providers/firebase/src/main/kotlin/dev/androidbroadcast/featured/firebase/FirebaseConfigValueProvider.kt	Adjust exception handling so cancellation propagates and all other failures are wrapped consistently.
docs/adr/001-configvalues-multi-module.md	Adds ADR 001 content to satisfy MkDocs nav and document multi-module setup.
.gitignore	Removes rule that was excluding ADR docs from version control.

[Copilot]

The ADR’s “Decision” section describes flags as top-level ConfigParam constants and references an app-level GeneratedFlagRegistry. In this repo, flags are typically declared via the featured { } Gradle DSL and the plugin generates Generated{Local,Remote}Flags plus a per-module GeneratedFlagRegistrar; there doesn’t appear to be any GeneratedFlagRegistry type in the codebase. Please update the ADR to reflect the actual generated artifacts and how multi-module apps should wire things together (and avoid referencing non-existent symbols).

Suggested change

	In multi-module Android/KMP projects, each feature module may declare its own `ConfigParam` flags. The question is how to compose these into a single `ConfigValues` instance that is shared across the app.
	## Decision
	Each module declares its flags as top-level `ConfigParam` constants. A single `ConfigValues` instance is created at the app level (e.g., in the DI graph) by passing all params through the same providers.
	The Gradle plugin generates a `FlagRegistrar` per module. The app module aggregates them via the generated `GeneratedFlagRegistry`, which collects every module's registrar at compile time.
	Modules never create their own `ConfigValues`; they only declare params and read from the shared instance injected from the app layer.
	## Consequences
	- Flag namespacing is flat — param names must be unique across all modules.
	- There is one set of local/remote providers for the whole app; per-module providers are not supported.
	- The Gradle plugin enforces uniqueness and generates the aggregation boilerplate automatically.
	In multi-module Android/KMP projects, each feature module may declare its own flags via the `featured { }` Gradle DSL. The question is how to compose these generated flags into a single `ConfigValues` instance that is shared across the app.
	## Decision
	Each module declares its flags in the `featured { }` Gradle DSL. The Gradle plugin generates module-scoped accessors such as `GeneratedLocalFlags` and `GeneratedRemoteFlags`, along with a per-module `GeneratedFlagRegistrar`.
	A single `ConfigValues` instance is created at the app level (for example, in the DI graph) and is wired using the set of `GeneratedFlagRegistrar` instances from the modules that participate in the app. There is no app-level generated `GeneratedFlagRegistry`; multi-module apps must explicitly include each module's generated registrar in their app wiring.
	Modules never create their own `ConfigValues`; they declare flags through the plugin-generated model and read from the shared instance injected from the app layer.
	## Consequences
	- Flag namespacing is flat — flag names must be unique across all modules.
	- There is one set of local/remote providers for the whole app; per-module providers are not supported.
	- The Gradle plugin generates `GeneratedLocalFlags`, `GeneratedRemoteFlags`, and a `GeneratedFlagRegistrar` for each module, while the app is responsible for wiring the relevant registrars together.

[Copilot]

The consequences list says “The Gradle plugin enforces uniqueness” and “generates the aggregation boilerplate automatically”. From the current plugin implementation, resolveFeatureFlags just serializes each module’s flags and scanAllLocalFlags only depends on per-module tasks; there’s no cross-module duplicate-key validation or generated app-level aggregator. Please reword these bullets to match actual behavior (or add explicit validation/generation if that’s intended).

Suggested change

	- The Gradle plugin enforces uniqueness and generates the aggregation boilerplate automatically.
	- The Gradle plugin currently serializes each module's declared flags, but it does not enforce cross-module uniqueness or generate an app-level aggregation boilerplate automatically.

[Copilot]

fetch() now treats CancellationException specially (rethrowing it), but the firebase provider tests only assert that failures are wrapped in FetchException. Please add a unit test covering the cancellation path (e.g., cancelled Task or coroutine cancellation) to ensure cancellation is not accidentally wrapped in FetchException in the future.