Skip to content

Keep daemon status working with newer config files#1865

Open
boudra wants to merge 2 commits into
mainfrom
lenient-config-loading
Open

Keep daemon status working with newer config files#1865
boudra wants to merge 2 commits into
mainfrom
lenient-config-loading

Conversation

@boudra

@boudra boudra commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Linked issue

None found.

Type of change

  • Bug fix
  • New feature (with prior issue + design alignment)
  • Refactor / code improvement
  • Docs

What does this PR do

Persisted config objects now strip unrecognized keys through Zod instead of failing on them. That keeps read paths like paseo daemon status from breaking when a config file contains settings written by a newer checkout, while malformed known fields still fail validation.

This also documents the config parsing behavior and adds a regression test with unknown root, daemon, and nested daemon fields.

How did you verify it

  • Added a red test for a config containing unrecognized persisted fields. Before the fix it failed with Unrecognized key; after the fix it passes.
  • npx vitest run packages/server/src/server/persisted-config.test.ts --bail=1
  • npm run lint -- packages/server/src/server/persisted-config.ts packages/server/src/server/persisted-config.test.ts
  • npm run typecheck:server
  • npm run format:files -- docs/data-model.md packages/server/src/server/persisted-config.ts packages/server/src/server/persisted-config.test.ts
  • Pre-commit hook also ran targeted lint, format check, and full npm run typecheck.

Checklist

  • One focused change. Unrelated cleanups split out.
  • npm run typecheck passes
  • npm run lint passes
  • npm run format ran (Biome)
  • UI changes include screenshots or video for every affected platform
  • Tests added or updated where it made sense

boudra added 2 commits July 2, 2026 18:39
Config files can be shared across checkouts with different schema versions. Strip unrecognized keys on load so read paths like daemon status do not fail before they can report state; known malformed fields still fail validation.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5616c6af6a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

}

const migrated = stripRemovedConfigFields(parsed);
const migrated = stripUnrecognizedConfigFields(stripRemovedConfigFields(parsed));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve future config keys before saving

When a newer config contains unknown fields, this now returns a parsed config with those fields stripped; the same loader feeds write paths such as DaemonConfigStore.persistConfig (loadPersistedConfig → merge → savePersistedConfig) and paseo daemon set-password. In that downgrade/newer-config scenario, saving any unrelated setting rewrites config.json without the future sections, so the compatibility fix can silently destroy settings written by the newer daemon; the tolerant read path should either retain unknown data for saves or be limited to read-only status flows.

Useful? React with 👍 / 👎.

@greptile-apps

greptile-apps Bot commented Jul 2, 2026

Copy link
Copy Markdown

Greptile Summary

This PR makes the config read path forward-compatible by switching every Zod sub-schema from .strict() to .strip(), so a config file written by a newer daemon no longer breaks older read paths like paseo daemon status. Known fields with bad values still fail validation, and saves are unaffected in practice because the in-memory PersistedConfig type has no unknown fields.

  • Schema change: all object schemas in persisted-config.ts changed from .strict() to .strip(); the two .passthrough() schemas (mcp, browserTools) are unchanged.
  • Tests: the "rejects unknown logging config fields" test updated to assert stripping; a new regression test covers unknown fields at the root, daemon, and nested relay levels through the full loadPersistedConfig path.
  • Docs: data-model.md gets a brief note documenting the strip-on-read / validate-known-fields contract.

Confidence Score: 5/5

Safe to merge — the change is narrowly scoped to the config schema and load path, and a regression test covers the exact scenario described in the PR.

Three-file change with a clear behavioral contract: unknown fields are dropped on read, malformed known fields still error. The new test exercises root, daemon, and nested relay unknown keys through the full load path. The only issue is a now-stale inline comment on stripRemovedConfigFields.

No files require special attention; the comment on stripRemovedConfigFields in persisted-config.ts is the sole minor rough edge.

Important Files Changed

Filename Overview
packages/server/src/server/persisted-config.ts All Zod sub-schemas changed from .strict() to .strip(); both mcp and browserTools retain .passthrough() (unchanged, pre-existing). stripRemovedConfigFields pre-processes explicit field removals and now carries a stale comment.
packages/server/src/server/persisted-config.test.ts Existing "rejects unknown logging config fields" test correctly updated to "strips unknown logging config fields"; new regression test covers unknown root, daemon, and nested relay fields through the full loadPersistedConfig path.
docs/data-model.md Two-sentence doc note added accurately describing the new strip-on-read / strict-on-valid-fields behavior; no issues.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Read config.json from disk] --> B[JSON.parse]
    B --> C[stripRemovedConfigFields\nexplicitly remove known-removed fields]
    C --> D[PersistedConfigSchema.safeParse\nwith .strip on every sub-schema]
    D -->|unknown keys silently dropped| E{Valid known fields?}
    E -->|Yes| F[Return PersistedConfig\nunknown keys gone]
    E -->|No - bad value on known field| G[Throw descriptive error]
    H[savePersistedConfig] --> I[PersistedConfigSchema.safeParse]
    I -->|TS-typed input| J{Valid known fields?}
    J -->|Yes| K[writePrivateFileAtomicSync]
    J -->|No| L[Throw error]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Read config.json from disk] --> B[JSON.parse]
    B --> C[stripRemovedConfigFields\nexplicitly remove known-removed fields]
    C --> D[PersistedConfigSchema.safeParse\nwith .strip on every sub-schema]
    D -->|unknown keys silently dropped| E{Valid known fields?}
    E -->|Yes| F[Return PersistedConfig\nunknown keys gone]
    E -->|No - bad value on known field| G[Throw descriptive error]
    H[savePersistedConfig] --> I[PersistedConfigSchema.safeParse]
    I -->|TS-typed input| J{Valid known fields?}
    J -->|Yes| K[writePrivateFileAtomicSync]
    J -->|No| L[Throw error]
Loading

Comments Outside Diff (1)

  1. packages/server/src/server/persisted-config.ts, line 356-359 (link)

    P2 The comment above stripRemovedConfigFields says the function exists "so the strict schema does not reject" unknown keys, but the schema now uses .strip() everywhere, so it would silently discard those fields on its own. The comment no longer explains the actual reason this pre-processing step exists (explicit documentation of intentionally removed fields + COMPAT reference), which could mislead a future maintainer into believing the schema is still strict and this step is the only guard.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Reviews (1): Last reviewed commit: "refactor(config): use schema stripping f..." | Re-trigger Greptile

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.

1 participant