Skip to content

feat(configuration): strip redundant config.yaml blocks once persisted#1907

Merged
andersonleal merged 4 commits into
mainfrom
feat/configuration-strip-redundant-blocks
Jun 23, 2026
Merged

feat(configuration): strip redundant config.yaml blocks once persisted#1907
andersonleal merged 4 commits into
mainfrom
feat/configuration-strip-redundant-blocks

Conversation

@andersonleal

@andersonleal andersonleal commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

What

After a worker's value is persisted in the configuration store, its config: block in config.yaml is dead weight — the store is the runtime source of truth (the block is seed-only) and a stale block silently drifts from reality.

On boot, before the file watcher starts, serve() now strips that block (keeping the - name: entry) for every worker entry whose value is already in the store, replacing it with a breadcrumb comment pointing at where the value lives:

  - name: iii-stream
    # 'iii-stream': value now lives in the configuration worker at ./data/configuration/iii-stream.yaml. Edit at runtime via 'iii config set iii-stream' (or 'configuration::set'); this block is no longer read.

The location is resolved from the configuration worker's own fs-adapter directory (default ./data/configuration); a non-fs adapter (e.g. bridge) falls back to "the configuration worker store".

Why store-driven

The strip queries configuration::get rather than relying on each worker to report that it seeded. That makes it uniform across:

  • built-in workers (stream, queue, cron, state, pubsub, observability, http) — they seed during the serial boot loop, so their value is in the store by the time the strip pass runs → stripped on first boot.
  • external workers (e.g. shell, coder) — they register over the SDK bus under their config.yaml name and persist a value; their block is stripped on the first boot after they persisted.

Entries with no persisted value yet (e.g. a worker that hasn't run) are left untouched. No per-worker hooks needed — the entire feature is one new module plus one block in serve().

Safety

  • The strip runs before the watcher exists, so self-writes never trigger a reload.
  • The in-memory worker entry's config is nulled only on a confirmed file write, so a later reload diff sees no phantom change and doesn't needlessly restart the worker.
  • Line-based edit (not a serde_yaml round-trip): comments, formatting, and ${VAR} placeholders in other blocks are byte-preserved. Idempotent — a second pass finds no block and no-ops.

Tradeoff (by design)

External workers connect after boot, so their block is stripped one boot late (never on the boot they first register). Same-boot stripping would need file-watcher self-write suppression (reload-storm risk); this keeps the simpler model that matches the existing seed-only semantics.

Test plan

  • engine/src/workers/config_rewrite.rs unit tests (8): strip/keep-entry, env-placeholder preservation, nested config:, idempotency, absent entry/block, after-image: field, first-of-duplicates.
  • engine/tests/config_reload_e2e.rs (3 new, boot through serve()):
    • first-seed: block stripped + value moved to store + comment points at the store path.
    • already-persisted: pre-seeded store → block stripped on the not-seeding path, stored value not clobbered.
    • external worker: a non-builtin worker that registers nothing in-process → its block is stripped from the store value alone.
  • Regression: 14 seed unit tests + 61 per-worker configuration e2e tests green; cargo check clean.

Summary by CodeRabbit

  • New Features
    • Configuration files now automatically strip redundant per-worker config seed blocks after the worker’s value has been persisted, replacing removed sections with a breadcrumb pointing to the persisted config source to reduce unnecessary reload triggers.
    • Rewrites are performed early (before watching) and are best-effort to avoid blocking startup.
  • Tests
    • Added end-to-end coverage for first-boot stripping, already-persisted values, and external/bus-style workers.
    • Added polling-based assertions for rewrite completion and increased an observability test timeout to reduce flakiness.
  • Documentation
    • Updated the changelog with the new configuration-store and config.yaml behavior details.

@vercel

vercel Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
iii-website Ready Ready Preview, Comment Jun 23, 2026 10:37pm

Request Review

@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 762099bc-f011-40d2-8c5a-3807ceaa6ced

📥 Commits

Reviewing files that changed from the base of the PR and between 0964fb3 and df781a9.

📒 Files selected for processing (6)
  • docs/changelog/index.mdx
  • engine/src/lib.rs
  • engine/src/workers/config.rs
  • engine/src/workers/config_rewrite.rs
  • engine/tests/config_reload_e2e.rs
  • engine/tests/observability_configuration_e2e.rs
✅ Files skipped from review due to trivial changes (1)
  • engine/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (4)
  • engine/tests/observability_configuration_e2e.rs
  • engine/tests/config_reload_e2e.rs
  • engine/src/workers/config.rs
  • engine/src/workers/config_rewrite.rs

📝 Walkthrough

Walkthrough

A new config_rewrite module is added to the engine that performs line-based YAML rewriting to strip a worker's seeded config: block from config.yaml after the value has been persisted to the configuration store. The strip is wired into EngineBuilder::serve as a pre-watcher step, validated with unit and e2e tests, and documented in the changelog.

Changes

Worker config block strip on engine startup

Layer / File(s) Summary
config_rewrite module: helpers, strip logic, atomic write, and unit tests
engine/src/lib.rs, engine/src/workers/config_rewrite.rs
New module declared via pub mod config_rewrite;. Implements seed_comment, leading_spaces/entry_indent parsing helpers, strip_worker_config_block (idempotent line-based YAML rewrite returning None when no block exists), apply_strip (best-effort file rewrite with tracing logging, returns true only on successful rewrite), atomic_write (temp-file + rename with permission preservation), and comprehensive unit tests covering stripping, nesting, idempotency, placeholders, and duplicate entries.
Pre-watcher strip integration in EngineBuilder::serve
engine/src/workers/config.rs
Before the filesystem watcher is created, iterates running worker entries that still carry an in-memory config block, calls engine.call("configuration::get", ...) to check whether a value is already persisted, and calls apply_strip to remove the redundant config: block from disk; clears entry.config in memory only when the strip succeeds.
E2E tests for config strip scenarios and test timing
engine/tests/config_reload_e2e.rs, engine/tests/observability_configuration_e2e.rs
Adds wait_for_config_rewrite polling helper to detect breadcrumb strings in config files, then three new #[tokio::test] cases: (1) first-boot seeding strips the worker config block and creates the store file, (2) already-persisted value strips the stale block without overwriting the existing stored value, (3) external-worker-like path strips the stale block leaving only a breadcrumb comment. Also increases observability_configuration_e2e wait_for timeout from 5 to 20 seconds to reduce flakiness under llvm-cov instrumentation.
Changelog documentation for 0.19.7 release
docs/changelog/index.mdx
Documents the worker config.yaml behavior change: config: blocks are seed-only and are stripped on next boot once the value is persisted to the configuration store, replaced with a breadcrumb pointing to the persisted value file at ./data/configuration/<id>.yaml. Specifies the strip implementation (line-based, byte-preserving, atomic, best-effort, runs before watcher, idempotent on subsequent boots) and notes the persisted store format change to value-only output.

Sequence Diagram

sequenceDiagram
  participant Client as Engine Boot
  participant Serve as EngineBuilder::serve
  participant ConfigRewrite as config_rewrite::apply_strip
  participant ConfigStore as Configuration Store
  participant FileSystem as config.yaml
  participant Watcher as File Watcher

  Client->>Serve: start engine
  Serve->>ConfigStore: configuration::get(worker_id)
  ConfigStore-->>Serve: persisted value (if exists)
  alt Value already persisted
    Serve->>ConfigRewrite: apply_strip(path, id, location)
    ConfigRewrite->>FileSystem: read config.yaml
    ConfigRewrite->>ConfigRewrite: strip_worker_config_block(content)
    ConfigRewrite->>FileSystem: atomic_write (temp + rename)
    ConfigRewrite-->>Serve: true (rewritten)
    Serve->>Serve: clear entry.config
  else No persisted value
    Serve-->>Serve: skip strip
  end
  Serve->>Watcher: create filesystem watcher
  Watcher-->>Client: ready (no self-triggered reload)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

  • iii-hq/iii#1878: Both PRs target the iii-pubsub "config.yaml is seed-only, persisted configuration is authoritative" flow—main PR adds the YAML seed-block stripping and rewrite before reloads, while retrieved PR implements the iii-pubsub configuration-worker seeding and live reads using that persisted value.
  • iii-hq/iii#1901: Main PR's config_rewrite logic rewrites config.yaml to point at the per-<id>.yaml configuration-store persisted file, while the retrieved PR refactors that persisted <id>.yaml format to be value-only (no schema)—the rewrite behavior and e2e expectations are directly coupled to the same on-disk artifact.
  • iii-hq/iii#1464: Both PRs modify the config reload path in EngineBuilder::serve around the file-watcher trigger—retrieved PR adds notify-based reload via ReloadManager, main PR adds a pre-watcher config_rewrite strip step that rewrites config.yaml to avoid self-triggering reloads.

Suggested reviewers

  • sergiofilhowz

🐇 A config block once seeded with care,
now lives in the store — no longer there.
A comment breadcrumb marks the trail,
idempotent strips that never fail.
The watcher stays calm, no phantom reload,
the rabbit has lightened the YAML load!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: stripping redundant config blocks once persisted in the configuration store.
Description check ✅ Passed The description comprehensively covers all template sections with clear explanations of what, why, design decisions, safety considerations, and test coverage.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/configuration-strip-redundant-blocks

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@engine/tests/config_reload_e2e.rs`:
- Line 321: Replace the fixed sleep calls at lines 321, 415, and 498 in the
config_reload_e2e test with a polling loop that checks for the expected
condition (the rewritten content) within a bounded timeout. Instead of using
tokio::time::sleep(Duration::from_millis(1000)).await, implement a loop that
periodically checks the actual state/content and breaks when the expected
condition is met or the timeout expires, eliminating the nondeterministic timing
dependency.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0b5c61c3-40aa-44d9-bdc2-c54497ca453b

📥 Commits

Reviewing files that changed from the base of the PR and between 4283417 and b9f4f3e.

📒 Files selected for processing (4)
  • engine/src/lib.rs
  • engine/src/workers/config.rs
  • engine/src/workers/config_rewrite.rs
  • engine/tests/config_reload_e2e.rs

Comment thread engine/tests/config_reload_e2e.rs Outdated
andersonleal added a commit that referenced this pull request Jun 23, 2026
Documents the unreleased configuration work landing after 0.19.6: redundant
worker config: blocks are stripped from config.yaml once the value is
persisted (#1907), plus the value-only persisted store files (#1901).

Claude-Session: https://claude.ai/code/session_012BHDssDNdbRrcgvCwH4cYU
After a worker's value is persisted in the configuration store, its
`config:` block in config.yaml is dead weight: the store is the runtime
source of truth and the block can silently drift. On boot, before the
file watcher starts, serve() now strips the block (keeping the `- name:`
entry) for every worker entry whose value is already in the store, and
replaces it with a breadcrumb comment pointing at the value's location
(e.g. ./data/configuration/<id>.yaml for the fs adapter).

The strip is store-driven (queries configuration::get), so it covers
built-in workers and external workers like `shell` uniformly with no
per-worker hooks. Built-in workers seed during the boot loop, so their
blocks strip on first boot; an external worker registers over the bus
after boot, so its block strips on the first boot after it persisted.
Entries with no persisted value (e.g. a worker that hasn't run yet) are
left untouched.

The strip runs before the watcher exists, so self-writes never trigger a
reload; the matching in-memory entry is nulled only on a confirmed write
so a later reload diff sees no phantom change.
…ge e2e

cargo fmt --check failed on config_rewrite.rs (the `let Some(..) =` binding
needed wrapping). Separately, the Engine Coverage job timed out in
observability_configuration_e2e::alert_rules_hot_swap_and_prune: the shared
wait_for() poll capped at 5s, which is borderline under llvm-cov
instrumentation. Widen the failure-case deadline to 20s (happy path is
unaffected — it returns as soon as the predicate holds).

Claude-Session: https://claude.ai/code/session_012BHDssDNdbRrcgvCwH4cYU
CodeRabbit flagged the three fixed 1s sleeps in config_reload_e2e before
reading the rewritten config.yaml: they race on slow CI runners (the same
flake class that just timed out the coverage job). Replace them with
wait_for_config_rewrite(), which polls the file for the seed-strip breadcrumb
under a bounded 10s deadline. The breadcrumb assertion is now the poll's break
condition, so the redundant follow-up asserts are dropped. The happy path
returns as soon as the rewrite lands.

Claude-Session: https://claude.ai/code/session_012BHDssDNdbRrcgvCwH4cYU
Documents the unreleased configuration work landing after 0.19.6: redundant
worker config: blocks are stripped from config.yaml once the value is
persisted (#1907), plus the value-only persisted store files (#1901).

Claude-Session: https://claude.ai/code/session_012BHDssDNdbRrcgvCwH4cYU
@mintlify

mintlify Bot commented Jun 23, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
iii 🟢 Ready View Preview Jun 23, 2026, 10:47 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@andersonleal andersonleal merged commit 32477c9 into main Jun 23, 2026
38 checks passed
@andersonleal andersonleal deleted the feat/configuration-strip-redundant-blocks branch June 23, 2026 23:09
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