Skip to content

feat(OMN-10773): atomic env file writer for interactive onboarding config output - #1550

Merged
jonahgabriel merged 2 commits into
mainfrom
omn-10773-config-writer
May 9, 2026
Merged

jonahgabriel merged 2 commits into
mainfrom
omn-10773-config-writer

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented May 9, 2026 •

Copy link
Copy Markdown
Collaborator

OMN-10773

Implements the config writer for the interactive onboarding executor (Task 6 of the interactive onboarding executor plan). Separated from the executor so file writes are an explicit outer action, never triggered implicitly.

Changes

  • config_writer.py — write_env_file(env_dict, target_path): reads existing file, merges (preserves keys not in env_dict, overwrites keys that are), writes atomically via tmp+rename using Path.replace() (PTH105-clean). Returns merged content as string for dry-run display without writing.
  • 8 unit tests: new file creation, merge preserving existing keys, overwrite, atomic write (no tmp leftover), roundtrip read-back, empty dict, home-dir safety assertion, return type check

Design note

write_env_file never decides whether to write — it only writes when called. The handler/CLI outer layer controls the dry_run flag (Task 7). This function is a pure effect: call it when you mean to write.

dod_evidence

  • 8 unit tests passing (uv run pytest tests/unit/onboarding/test_config_writer.py)
  • uv run mypy src/omnibase_infra/onboarding/config_writer.py --strict — clean
  • pre-commit run --all-files — all hooks passing
  • Tests use only tmp_path; test_no_writes_under_real_home explicitly asserts ~/.omnibase/.env mtime unchanged

Evidence-Source: 43a688815172db3797bae4ae01ff47c4d9ef3b12
Evidence-Ticket: OMN-10773

…nfig output

- write_env_file(env_dict, target_path): reads existing file, merges (preserve
  keys not in env_dict, overwrite keys that are), writes atomically via tmp+rename
- Returns merged content as string for dry-run display without writing
- Uses Path.replace() / Path.unlink() (PTH105/PTH108 compliant)
- 8 unit tests: new file, merge, overwrite, atomic (no tmp leftover), roundtrip,
  empty dict, home-dir safety assertion, return type check
@coderabbitai

coderabbitai Bot commented May 9, 2026 •

Copy link
Copy Markdown

Warning

Rate limit exceeded

@jonahgabriel has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 1 minute before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 057663ae-acbb-446a-92a9-13ce2e3c5f2d

📥 Commits

Reviewing files that changed from the base of the PR and between 73bb1a8 and f23f843.

📒 Files selected for processing (3)
  • src/omnibase_infra/onboarding/config_writer.py
  • tests/integration/test_config_writer_integration.py
  • tests/unit/onboarding/test_config_writer.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch omn-10773-config-writer

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment thread src/omnibase_infra/onboarding/config_writer.py Fixed
# Conflicts:
#	src/omnibase_infra/onboarding/config_writer.py
#	tests/unit/onboarding/test_config_writer.py
@jonahgabriel
jonahgabriel merged commit 3cade3e into main May 9, 2026
51 of 53 checks passed
@jonahgabriel
jonahgabriel deleted the omn-10773-config-writer branch May 9, 2026 21:08
jonahgabriel added a commit that referenced this pull request Aug 9, 2026
… override (#2705)

* feat(OMN-15789): EventBusKafka.subscribe(auto_offset_reset=) per-call override

Adds an optional auto_offset_reset kwarg to EventBusKafka.subscribe(),
threaded through to _start_consumer_for_topic_unlocked's new
auto_offset_reset_override parameter. None (the default, every
pre-existing call site) preserves current behavior exactly -- falls
back to self._config.auto_offset_reset. Each (topic, group_id) pair
already gets its own dedicated AIOKafkaConsumer, so the override is
scoped to exactly the one consumer a given subscribe() call creates.

This is the real_broker-side half of the event_bus_substrate fixture's
fidelity contract (OMN-15789, omnibase_core PR #1550, merged): the
fixture's real_broker leg needs the same per-call auto_offset_reset
surface EventBusSemanticFake (omnibase_core) already exposes, so a
single EventBusKafka instance can be reused across earliest/latest
test legs instead of needing a separate instance per offset policy.

New unit test proves the override threads through correctly (and that
a None override is not silently widened to a truthy value):
test_subscribe_threads_auto_offset_reset_override_to_consumer_start.

Landed as its own PR (split from the fixture/conftest/exemplar-test
half, which imports omnibase_core.event_bus.testing.* -- unreleased
on PyPI as of this commit; omnibase_infra pins omnibase-core from
PyPI only, OMN-14628 pure-PyPI seam, no git-rev override) so this
independently-useful, independently-CI-green production change is not
blocked on that release. The fixture-consuming half is prepared on
this same branch's follow-up commit, held pending the release.

Evidence-Ticket: OMN-15789

* fix(OMN-15789): update fake_start test doubles for auto_offset_reset_override kwarg

The governed impacted-test selector (pre-push) caught a real regression
in 5d55a94: four TestKafkaEventBusSubscribe tests hand-roll a
fake_start(topic, group_id) async double patched over
_start_consumer_for_topic_unlocked, and the new
auto_offset_reset_override keyword-only parameter broke their fixed
signature (TypeError: unexpected keyword argument). Widens every
fake_start/fake_start_unlocked double in this file to accept the new
kwarg. 121/121 passed in tests/unit/event_bus/test_kafka_event_bus.py
after the fix; 2984/2984 passed across
tests/unit/event_bus/+adapters/+backends/+services/.

Evidence-Ticket: OMN-15789
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