[Starosdev]

Fixes #663.

Summary

QNAP TR-004 enclosures in Individual mode address each bay through the smartmontools jmb39x-q,N backend. Two independent collector defects made those disks unmonitorable. Both are fixed here.

Bug A: comma-containing device types were split at config parse time

The argument construction was never at fault. All three --device sites already append the type as a single element, and the shell layer uses exec.CommandContext with no re-tokenization. The damage happened earlier: viper installs its own stringToWeakSliceHookFunc(",") decode hook, which splits a scalar string when the destination is a slice. type: 'jmb39x-q,0' therefore decoded to the two bogus types jmb39x-q and 0.

A comma belongs to the smartctl device-type grammar (megaraid,14, sat,auto, jmb39x-q,0), it is not a separator between types. GetDeviceOverrides now overrides DecodeHook for the devices key so WeaklyTypedInput lifts a scalar into a one-element slice verbatim. The list form is unchanged.

Two edge cases that removing the hook opens up are handled by a new normalizeDeviceOverrides: configured types are trimmed, and an explicitly empty type stays an empty but non-nil slice so buildOverrideDeviceGroup keeps dropping the device rather than falling back to the scanned type.

Bug B: any non-zero smartctl exit aborted device identification

SmartCtlInfo masked the exit code with 0xBF, but the tolerant branch logged "Continuing." and then fell straight into the pre-existing if err != nil { return err }. It was dead code: no non-zero smartctl exit had ever survived detection, including the bit-6 case the mask was added for.

These enclosures report Read Device Statistics page 0x00 failed and exit 4 on every run while printing a complete JSON document. The disk was therefore registered with no model, serial or WWN. The empty WWN is the actual cause of the reported "No Data": every InfluxDB query filters on the device_wwn tag. This means users on the documented list-form workaround were still broken, because their Collect calls were publishing fine under the existing 0x03 rule while detection threw the identity away.

Detection now applies the same rule the collection path and the server's validateSmartExitStatus already used: only bits 0x01 and 0x02 mean the payload cannot be trusted, since every other bit describes a condition of the disk rather than of the output. The three copies of that rule are replaced by a shared webapp/backend/pkg/smartctl package. Detection additionally rejects empty output, so genuinely unusable results are still refused.

Behavior changes reviewers should weigh

Scalar comma semantics. type: 'sat,auto' previously yielded the two types sat and auto. It now yields one. Both old outputs were nonsense as smartctl device types and neither has ever worked, so no realistic configuration can depend on the old behavior, and no compatibility path is included. The list form is unaffected; TestConfiguration_GetScanOverrides_Raid is the unchanged proof.

Affected devices re-register under a new id. deviceid is untouched and DeviceType is not an identity input, so the parsing change cannot re-id anything by itself. But a drive that is identity-less today resolves through the device_name+host_id fallback; once its model, serial and WWN can be read it hashes differently and appears as a new device, orphaning the old "No Data" row. This is unavoidable, and is the desired outcome. The troubleshooting doc tells affected users to delete the stale entry, whose history is empty-WWN and cannot be merged usefully.

This supersedes a documented asymmetry. The knowledge base recorded the two exit-code masks as deliberately different and instructed against unifying them. git log -S 0xBF shows the mask arrived as a narrow bit-6 spot-fix whose lines were inserted above the existing return err, so the documented tolerance never executed. The invariant described an intent the code never implemented. ADR 0008 records the decision.

Tests

13 new tests, all passing.

• Config: comma preservation in scalar and list form, the four-bay jmb39x-q layout, whitespace trimming, and the empty-vs-nil device type distinction.
• Detect: valid JSON accepted on a non-fatal exit 4 with identity and a lowercase WWN populated; output rejected on fatal exit 2 with no fields copied; empty output rejected; jmb39x-q,0 asserted as one --device argument. The existing SmartCtlInfo test used an empty device type, so that branch had no coverage at all.
• Metrics collector: --device argv asserted on both the --xall and FARM calls (the two sites besides detection that must stay identical), publish on non-fatal exit, and error report without publish on fatal exit. Collect previously had no coverage.

Exit statuses come from a real *exec.ExitError, produced by re-executing the test binary as a helper process rather than shelling out to sh -c, so the tests stay portable.

Verified: go test ./collector/..., backend handler exit-status tests, go vet, gofmt, go mod vendor clean, and grep 0xBF returns nothing.

Not covered here

No TR-004 hardware was available, so the end-to-end acceptance criterion is unverified on real hardware. The detect fixture is modeled on a standard SAT --info --json response rather than captured from a real jmb39x-q device; replacing it with sanitized real output would make the regression test stronger.

[Starosdev]

Merged (2026-07-28)

Changes Made

1. Preserved comma-containing smartctl device types through config parsing

   • Overrode viper's decode hook for the devices key so a scalar type: 'jmb39x-q,0' is no longer split into the two bogus types jmb39x-q and 0
   • A comma belongs to the smartctl device-type grammar, not between types; both the scalar and list forms now round-trip intact
   • Added trimming of configured types, and kept an explicitly empty type as an empty but non-nil slice so the device is still dropped rather than falling back to the scanned type

2. Accepted usable SMART JSON on a non-fatal smartctl exit status

   • Removed a dead fall-through in SmartCtlInfo that returned an error on every non-zero exit, making the existing 0xBF tolerance unreachable
   • Detection now applies the same rule as collection and server-side upload validation: only bits 0x01 and 0x02 invalidate the payload
   • Empty output and output with no usable identity are still rejected

3. Consolidated the smartctl exit-status rule

   • New webapp/backend/pkg/smartctl package replaces four copies of the bitmask logic across detection, collection, exit-code logging and upload validation

4. Documentation

   • New JMicron JMB39x (QNAP TR-004/TR-002) section in docs/TROUBLESHOOTING_DEVICE_COLLECTOR.md, covering Individual mode, bay indexing, docker passthrough, the expected exit code 4 warning, and the stale-entry upgrade note
   • QNAP example and corrected comma guidance in example.collector.yaml

5. Tests

   • 13 new tests covering config parsing of comma, whitespace and empty device types; detection under non-fatal, fatal and empty-output exit statuses; and --device argv on all three smartctl call sites
   • Collect had no prior coverage of argument construction or exit-code handling

PR	#677
Issue	#663
Merge commit	967bbaaf
Docker	ghcr.io/starosdev/scrutiny

Note: end-to-end validation on real TR-004 hardware is still outstanding, and the detect fixture is modeled rather than captured. Both are tracked in the issue comment.