Skip to content

feat(agent-config): overlay validation, config diff and display helpers [5/21] - #326

Merged
gusfcarvalho merged 2 commits into
agent-config/04-field-accessfrom
agent-config/05-validation-diff
Oct 6, 2026
Merged

gusfcarvalho merged 2 commits into
agent-config/04-field-accessfrom
agent-config/05-validation-diff

Conversation

@gusfcarvalho

Copy link
Copy Markdown
Contributor

Part 5 of 21 of the stacked split of #318 (agent remote configuration). Every layer adds the final version of its files from #318, and only imports from layers below it, so each layer passes make reviewable on its own. Nothing is reachable in the app until layer 20 wires the Configuration tab in.

What

Overlay validation (shape, schedule, policy sources), a structural config diff used by the save preview, and display formatting helpers.

Tests

Unit specs for each.

🤖 Generated with Claude Code

Copilot AI balanced review requested due to automatic review settings October 5, 2026 10:51
@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 3be61e1f-5c0f-4831-a91e-221f4acf0fde
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Large arrays can produce misleading change counts, and the credential-sanitization path lacks regression coverage.

Review effort: Balanced
Findings: 1 Medium severity · 1 Low severity

Open (2)
What changed in this PR

Adds reusable agent-configuration validation, structural diffing, and display utilities for the future Configuration tab.

Changes:

  • Adds client-side overlay validation and scalar coercion.
  • Adds structural and element-level configuration diffing.
  • Adds display helpers and unit tests.
File Description
src/​utils/​agent-config/​validation.ts Validates overlays and detects masked values.
src/​utils/​agent-config/​display.ts Adds sanitization and formatting helpers.
src/​utils/​agent-config/​config-diff.ts Computes structural configuration differences.
src/​utils/​agent-config/​__tests__/​validation.spec.ts Tests validation and coercion.
src/​utils/​agent-config/​__tests__/​display.spec.ts Tests relative-time formatting.
src/​utils/​agent-config/​__tests__/​config-diff.spec.ts Tests configuration and array diffing.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/utils/agent-config/config-diff.ts Outdated
Comment on lines +42 to +45
if (n * m > LCS_LIMIT) {
for (let k = 0; k < Math.max(n, m); k++) {
if (k < n && k < m && deepEqual(before[k], after[k])) {
steps.push({ op: 'eq', i: k, j: k });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fixed in a5066a0. arrayElementChanges matches the common prefix and suffix before the LCS, so a single insertion, removal or edit aligns exactly at any size in linear memory. Only a middle that is still larger than the limit falls back to index by index. Regression test: 600 vs 601 items with an insertion at the front and in the middle, a removal and an edit.

Comment on lines +10 to +16
export function sanitizeForDisplay<T>(doc: T): T {
if (!isPlainObject(doc)) return doc;
const out = clone(doc) as ConfigDoc;
if (isPlainObject(out.api) && isPlainObject(out.api.auth)) {
delete out.api.auth.client_secret;
}
return out as T;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fixed in a5066a0. display.spec.ts now checks that api.auth.client_secret is removed from the returned copy, that the input document is unchanged, that a plugin config key named client_secret is kept, and that documents without api.auth or non-objects pass through.

@ianmiell ianmiell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ccf-review: REQUEST_CHANGES

1 Blocker.

Stack (gh stack 343): #322 → #323 → #324 → #325 → #326 → #327 → #328 → #329 → #330 → #331 → #332 → #333 → #334 → #335 → #336 → #337 → #338 → #339 → #340 → #341 → #342

Comment thread src/utils/agent-config/validation.ts
ianmiell
ianmiell previously approved these changes Oct 6, 2026

@ianmiell ianmiell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ccf-review: APPROVE

no findings.

Stack (gh stack 343): #322 → #323 → #324 → #325 → #326 → #327 → #328 → #329 → #330 → #331 → #332 → #333 → #334 → #335 → #336 → #337 → #338 → #339 → #340 → #341 → #342

gusfcarvalho and others added 2 commits October 6, 2026 10:55
Layer 5 of 21 in the stacked split of #318.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eir common ends

- validateOverlayClientSide blocks Infinity, NaN and integers past 2^53
  anywhere in the overlay: JSON.stringify writes non-finite numbers as
  null, which RFC 7396 reads as deleting the key on every host
- arrayElementChanges matches the common prefix and suffix before the
  LCS, so one insertion in an array past the LCS limit is still one
  addition (regression test at 600 x 601)
- sanitizeForDisplay spec: client_secret removed from the copy only
- plugin-name cases join the agentconfig conformance table

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ccf-lisa
ccf-lisa Bot force-pushed the agent-config/05-validation-diff branch from 0b518a2 to 8641493 Compare October 6, 2026 14:20
@gusfcarvalho
gusfcarvalho merged commit dd3b5f6 into main Oct 6, 2026
7 checks passed
@gusfcarvalho
gusfcarvalho deleted the agent-config/05-validation-diff branch October 6, 2026 15:14
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.

3 participants