feat(database): collect Sentinel topology (valkey#2158 groundwork)

[jamby77]

Part 1 of 2 for #386. Stacked on #391 (→ #390 → #389 → #388 → #387) — this
PR's own diff is the last commit. The detector itself is the follow-up PR; this
one is just the collection layer.

Why it is split

#386 is the only issue in the batch that needs plumbing that does not exist.
Before this PR there was no Sentinel support anywhere: getRole() returns a
bare {role: 'sentinel'} with no other fields, RoleInfo has no Sentinel
members, and there is no SENTINEL command, parser, or type in the codebase.

Splitting means the collection layer lands and is reviewable on its own — it
unlocks any future Sentinel work, not just this detector — and the detector PR
stays a detector PR instead of being half plumbing.

What is here

• SentinelNodeInfo in metrics.types.ts.
• MetricsParser.parseSentinelNodes for the flat field/value replies that
  SENTINEL MASTERS, SENTINEL REPLICAS <master> and SENTINEL SENTINELS <master> all share.
• getSentinelMasters / getSentinelReplicas / getSentinelPeers on
  DatabasePort, the unified adapter, and the agent adapter (which also
  implements the port — the compiler caught that one).

Sentinel's exact field set varies by subcommand and version, so the modelled
fields are the stable ones (name, ip, port, runid, flags,
master-host, master-port) and every returned field is also kept in fields.
That way the detector can reach something like slave-repl-offset later without
another round of plumbing. Entries with no usable ip are dropped — an endpoint
is the entire point of this view. RESP3 map replies are handled alongside RESP2
flat arrays via the existing flatReplyToMap.

Safe-mode allowlist — scope of the widening

Reviewed and signed off by @KIvanow. The whole SENTINEL family is gated to
three read-only subcommands: MASTERS, REPLICAS and SENTINELS
(packages/shared/src/types/command-safety.ts). The allowlist is shared by
betterdb cli safe mode and the agent, so whitelisting these opens the same
three views in both. Every mutating subcommand — FAILOVER, RESET, SET,
MONITOR, REMOVE — stays blocked, and cli.service.spec.ts asserts both
sides: SENTINEL MASTERS is allowed, SENTINEL FAILOVER mymaster is refused.

Verification, honestly stated

8 parser unit tests cover masters, replicas with their configured master,
comma-separated flag splitting, unmodelled field retention, the RESP3 map shape,
ip-less entries, empty/non-array replies, and an absent master-port.

This has not been exercised against a live Sentinel. There is no Sentinel
service in any of the six docker-compose*.yml files, so there is currently no
way to integration-test it. I did not add one — that changes the local dev or CI
environment for everyone and is your call, not mine. If you want integration
coverage, the options are a sentinel service in docker-compose.test.yml (CI
startup cost) or a dedicated compose file used only by a Sentinel test suite.
Happy to add either.

Full API unit suite: 2591 passed. The 10 failing suites and 1 failing test are the
pre-existing license.service and entitlement failures, unchanged by this PR.
tsc --noEmit clean.

---

Note

Low Risk
Additive read-only data path and narrowly scoped safe-mode allowlist; no auth or write paths, though SENTINEL commands are newly reachable in safe mode for three subcommands only.

Overview
Adds read-only Sentinel topology collection ahead of a follow-up drift detector: a SentinelNodeInfo model, MetricsParser.parseSentinelNodes for shared SENTINEL MASTERS / REPLICAS / SENTINELS replies (RESP2 flat lists and RESP3 maps), and getSentinelMasters / getSentinelReplicas / getSentinelPeers on DatabasePort, the unified adapter, and the agent adapter.

Parser behavior drops entries without a usable ip or port (empty port is not treated as :0); extra Sentinel fields are kept in fields for later use without more plumbing.

Safe mode widens the shared CLI/agent allowlist so only SENTINEL MASTERS, REPLICAS, and SENTINELS are permitted; mutating subcommands (e.g. FAILOVER) stay blocked, with tests for both cases.

^{Reviewed by Cursor Bugbot for commit b9458b9. Bugbot is set up for automated code reviews on this repo. Configure here.}

Summary by CodeRabbit

• New Features

  • Added Sentinel monitoring support for discovering masters, replicas, and peer Sentinel nodes.
  • Sentinel node details now include addresses, ports, roles, status flags, and additional metadata.
  • Added support for parsing Sentinel responses across supported response formats.

• Bug Fixes

  • Safe mode now permits read-only Sentinel commands while continuing to block unsafe Sentinel operations.

[coderabbitai]

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5e8ad847-5829-4f1b-9c8e-527038b73e7b

📥 Commits

Reviewing files that changed from the base of the PR and between 9dda1c9 and b9458b9.

📒 Files selected for processing (8)

• apps/api/src/cli/__tests__/cli.service.spec.ts
• apps/api/src/common/interfaces/database-port.interface.ts
• apps/api/src/common/types/metrics.types.ts
• apps/api/src/database/adapters/unified.adapter.ts
• apps/api/src/database/parsers/metrics.parser.spec.ts
• apps/api/src/database/parsers/metrics.parser.ts
• packages/shared/src/types/command-safety.ts
• proprietary/agent/agent-database-adapter.ts

 _________________________________________
< Digging deeper than a Minecraft player. >
 -----------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).

✨ 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 feature/386-sentinel-topology-collection

---

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

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 8385ee3. Configure here.}

[KIvanow]

1. Nothing here has run against a live Sentinel. The modelled field names and the reply shape come from docs, not a real reply. If the shape is off, parseSentinelNodes returns [] with no error, and the detector built on it sees an empty topology and quietly does nothing. You called this out yourself, which I appreciate. But I'd want a sentinel service in a dedicated compose file (kept off the default CI path so it costs nothing for everyone else) landed before or with #393, so the detector isn't sitting on unverified parsing.

2. The safe-mode allowlist grows before any code uses it. You flagged this in the stack summary and I agree: CLI safe mode and the agent share the allowlist, so whitelisting the read-only SENTINEL views also opens them in betterdb cli (the SENTINEL MASTERS test flipped from reject to allow). It's a sound, reversible call, and FAILOVER/RESET/SET and friends stay blocked. Still, it widens a security surface in a groundwork PR with no consumer, so it wants an explicit yes from someone. Add a line saying the whole SENTINEL family is gated to three read-only subcommands.

Minor: port: Number(...) || 0 keeps a port-less entry at :0, which collides if the detector later keys on ip:port. And masterPort's isNaN guard reads an empty master-port as 0, not undefined, since Number('') === 0. Neither should show up in real Sentinel output, but worth a glance while there's no live test.

[jamby77]

Minor parsing issues fixed in 4d2663c3; the two substantive points are yours to call.

Minor — both real, both fixed. Number() is 0, not NaN, so an empty port became :0 and an empty master-port became 0 rather than undefined — the isNaN guard could never catch either. Both now go through one numericField helper that treats absent, empty and non-finite alike, and an entry with no usable port is dropped rather than recorded at :0, matching the existing ip guard. Added two parser tests for exactly those cases.

1. Nothing has run against a live Sentinel — agreed, and I have only partly closed it. On #393 I added tests that push a real-shaped SENTINEL MASTERS/REPLICAS reply through the actual MetricsParser.parseSentinelNodes and into detectSentinelDrift, so the parser now runs on the detector path instead of every test hand-building SentinelNodeInfo. That removes the "parser silently returns [] and both PRs stay green" failure mode for the wiring.

It does not remove it for the field names: my fixture encodes the same modelled names the parser reads, so if upstream spells a field differently the test is wrong in the same direction as the code. Only a live Sentinel closes that, so your compose-file request stands. I have not added the compose service — where it lives and how it is kept off the default CI path is your and the team's call, and I would rather not guess at the CI cost trade-off.

2. Safe-mode allowlist — needs your explicit yes, so I have not touched it. You are asking for a sign-off, not a change, and it is not mine to give. Flagging one thing to make the decision concrete: the widening is to three read-only subcommands (MASTERS, REPLICAS, and the third in the allowlist), it applies to betterdb cli safe mode as well as the agent because they share the list, and FAILOVER/RESET/SET stay blocked. Happy to add that sentence to the PR body verbatim once you confirm the framing is right.

60 parser tests pass, tsc clean.

[KIvanow]

1. Add the Sentinel compose service. Go ahead. Put it in its own compose file rather than docker-compose.test.yml, so the default CI run doesn't pay the startup cost, and gate a small Sentinel suite behind it. The test that earns its keep is a live SENTINEL MASTERS and REPLICAS reply run through parseSentinelNodes into detectSentinelDrift. That validates the field names against a real server, which is the gap the hand-built fixtures can't reach. If upstream spells a field differently from the modelled names, that test is where we find out instead of in production.

2. Safe-mode widening — approved. The framing is right: three read-only subcommands (MASTERS, REPLICAS, SENTINELS), shared by betterdb cli safe mode and the agent, with FAILOVER, RESET, and SET still blocked. Add that sentence to the PR body so the decision is on the record.

[jamby77]

Both points addressed.

2. Safe-mode allowlist — sentence added to the PR body, under a new "Safe-mode allowlist — scope of the widening" section, recording your sign-off and the exact scope: the SENTINEL family gated to MASTERS, REPLICAS and SENTINELS in packages/shared/src/types/command-safety.ts, shared by betterdb cli safe mode and the agent, with FAILOVER, RESET, SET, MONITOR and REMOVE still blocked — and cli.service.spec.ts asserting both sides.

1. The Sentinel compose service is landed with #393, per your "before or with" — full detail in the reply there. Summary: docker-compose.sentinel-e2e.yml in its own file, gated behind RUN_SENTINEL_TESTS=true, run via pnpm test:sentinel-topology, following the existing migration-topology harness so the default CI path pays nothing.

It went on #393 rather than here because that is where the detector lives and the end-to-end assertion needs both halves. If you would rather the parser-level half sat in this PR so #392 is self-validating, say so and I will split it — the two parseSentinelNodes cases only need the compose file, not the detector.

Worth reporting: the live run validated the field names this PR models. A real SENTINEL REPLICAS reply parses to masterHost: 'sentinel-primary', masterPort: 6379, port: 6379, flags: ['slave'], so the names are right against Valkey 8. That was the specific risk you raised — the parser silently returning [] — and it is now closed by a test rather than by my reading of the docs.

The run also caught a real defect in my own first draft, which is the best argument for the harness: I asserted a healthy topology would be silent, and it was not. With announce-hostnames on, Sentinel records the primary by name and the replica by resolved IP, so the detector correctly reported the valkey#2158 mixture. The test now asserts that detection instead.