Severity: Info
File: src/Servy/Mappers/ServiceConfigurationMapper.cs lines 34, 37–38, 40, 55–56, 61, 68
Description:
The UI mapper mixes two conventions for default values — some fields pull their default from AppConfig.*, others hardcode the literal:
// Uses AppConfig (good)
RotationSize = ParseInt(config.RotationSize, AppConfig.DefaultRotationSize),
// Hardcoded (bad, silently drifts if AppConfig.DefaultHeartbeatInterval changes)
HeartbeatInterval = ParseInt(config.HeartbeatInterval, 30),
MaxFailedChecks = ParseInt(config.MaxFailedChecks, 3),
// Hardcoded pre-launch defaults
PreLaunchTimeoutSeconds = ParseInt(config.PreLaunchTimeoutSeconds, 30),
PreLaunchRetryAttempts = ParseInt(config.PreLaunchRetryAttempts, 0),
This is a narrower instance of #733 (Servy mapper vs Core mapper divergence) but specifically about intra-file inconsistency: the mapper itself cannot decide whether AppConfig is the source of truth. If AppConfig.DefaultHeartbeatInterval is ever changed from 30, the value in ServiceConfigurationMapper line 37 silently keeps using the stale 30 — the two will diverge until a user reports weird default behaviour.
Suggested fix:
Route every default through AppConfig. In the same pass, add AppConfig.DefaultHeartbeatInterval, AppConfig.DefaultMaxFailedChecks, AppConfig.DefaultPreLaunchTimeoutSeconds, and AppConfig.DefaultPreLaunchRetryAttempts if they do not already exist, and replace the literals:
HeartbeatInterval = ParseInt(config.HeartbeatInterval, AppConfig.DefaultHeartbeatInterval),
MaxFailedChecks = ParseInt(config.MaxFailedChecks, AppConfig.DefaultMaxFailedChecks),
PreLaunchTimeoutSeconds = ParseInt(config.PreLaunchTimeoutSeconds, AppConfig.DefaultPreLaunchTimeoutSeconds),
PreLaunchRetryAttempts = ParseInt(config.PreLaunchRetryAttempts, AppConfig.DefaultPreLaunchRetryAttempts),
Add a tiny test asserting that for each ParseInt(raw, default) call, default equals the corresponding AppConfig value, so future reviewers can't reintroduce drift without failing CI.
Severity: Info
File:
src/Servy/Mappers/ServiceConfigurationMapper.cslines 34, 37–38, 40, 55–56, 61, 68Description:
The UI mapper mixes two conventions for default values — some fields pull their default from
AppConfig.*, others hardcode the literal:This is a narrower instance of #733 (Servy mapper vs Core mapper divergence) but specifically about intra-file inconsistency: the mapper itself cannot decide whether
AppConfigis the source of truth. IfAppConfig.DefaultHeartbeatIntervalis ever changed from 30, the value inServiceConfigurationMapperline 37 silently keeps using the stale 30 — the two will diverge until a user reports weird default behaviour.Suggested fix:
Route every default through
AppConfig. In the same pass, addAppConfig.DefaultHeartbeatInterval,AppConfig.DefaultMaxFailedChecks,AppConfig.DefaultPreLaunchTimeoutSeconds, andAppConfig.DefaultPreLaunchRetryAttemptsif they do not already exist, and replace the literals:Add a tiny test asserting that for each
ParseInt(raw, default)call,defaultequals the correspondingAppConfigvalue, so future reviewers can't reintroduce drift without failing CI.