HarperFast / HarperFast/harper

checkForUpdatedConfig only inspects YAML parse errors when updateFile is already true

Open Beginner friendly
#2,027 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
89
Forks
10
Avg merge
2d 6h
Merged PRs (30d)
200

Description

## Summary

`config/configUtils.ts`'s `checkForUpdatedConfig` nests its YAML-parse-error check inside the `if (updateFile)` branch, so `configDoc.errors` is only inspected on a boot that *also* happens to be rewriting the config file (e.g. because some other backfilled key was missing). A complete, steady-state config — which is the common case, since Harper writes every param back out — leaves `updateFile` false, and any parse errors on that document are silently discarded.

Note the inconsistency with this file's own sibling: `ensureConfigKeysPresent` (`config/configUtils.ts:329`) checks `configDoc.errors` unconditionally, regardless of whether it's about to rewrite the file.

## Concrete failure scenario

An operator manually edits `harperdb-config.yaml` and accidentally duplicates a top-level key (e.g. a second `replication:` block). `YAML.parseDocument` on such a document returns `errors: ['DUPLICATE_KEY']`, while `.toJSON()` still yields every key with last-one-wins semantics — so nothing is *missing*, `updateFile` stays `false`, and the duplicate-key error is never surfaced. Harper boots successfully with the first `replication:` block silently discarded — the wrong topology, no warning, no log line.

## Suggested fix

Hoist the `configDoc.errors` check in `checkForUpdatedConfig` out of the `if (updateFile)` branch so it runs unconditionally, matching `ensureConfigKeysPresent`'s existing behavior.

## Provenance

Surfaced by an independent multi-model pre-push review (Harper-domain lens) of #1972 (`test(server): promote 3 QA server-layer contract anchors`), as an out-of-diff finding — that PR is test-only and does not touch this file. Filing separately since it's a production-code fix, not a test change.

🤖 Filed by Claude (dispatch pr-fix agent) while addressing review comments on #1972.

Contributor guide

Open the contributing guide

Research direction

Start in config/configUtils.ts at checkForUpdatedConfig and compare its configDoc.errors handling with ensureConfigKeysPresent around line 329. Move the error check so it runs regardless of updateFile, then verify that a duplicate top-level key in harperdb-config.yaml is surfaced instead of being silently discarded.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
82/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.