Skip to content

feat(agent-config): three-state field access over reporting instances [4/21] - #325

Merged
gusfcarvalho merged 4 commits into
agent-config/03-types-glob-cronfrom
agent-config/04-field-access
Oct 6, 2026
Merged

gusfcarvalho merged 4 commits into
agent-config/03-types-glob-cronfrom
agent-config/04-field-access

Conversation

@gusfcarvalho

@gusfcarvalho gusfcarvalho commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Part 4 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

Computes three-state field access (editable / locked / unsupported) for each config path across the instances that reported, from the API's locked-key globs and per-instance capabilities.

Tests

field-access.spec.ts.

Size exception

This PR is over the 1k-line limit (about 1,680 lines, package-lock excluded) and carries the size-exception label. About 430 of those lines are the API's agentconfig conformance golden file, vendored byte for byte (with its sync script, README and CI drift check); the rest is field-access.ts and its spec. The field-access rules, their spec and the conformance table that checks them against the API belong together; splitting them would leave a layer whose rules nothing checks.

🤖 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: c5892669-bad1-452d-8ceb-868f2aeebded
  • 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

Source reuse, plugin re-enabling, and registry parsing currently diverge from API classification behavior.

Review effort: Balanced
Findings: 3 Medium severity

Open (3)
What changed in this PR

Adds agent configuration field-access classification across reporting instances.

Changes:

  • Computes editable, restricted, read-only, and forbidden states.
  • Classifies plugin sources and installation eligibility.
  • Adds comprehensive unit tests for access rules and tooltips.
File Description
src/​utils/​agent-config/​field-access.ts Implements field access and source classification.
src/​utils/​agent-config/​__tests__/​field-access.spec.ts Tests field states, source safety, and plugin installation.

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

Comment on lines +179 to +180
case 'source':
return rc.trusted.length ? YES : no(NO_TRUSTED);

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 b25fd2f. On an apply_safe instance without trusted_sources, source is now partial ("only a source this host already uses") when the host file uses any source, because the API classifies that as already-used. It stays read-only only when there is nothing to reuse. usedSources also skips disabled plugins now, matching the API.

Comment on lines +185 to +186
default:
return YES;

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 b25fd2f. When the loaded base disables the plugin, enabled follows reenables-plugin: it applies only when the effective source (the overlay source, else the file source) matches trusted_sources, the kept policy entries classify as safe, and the effective config has no ${env:} references (TestClassifyReenableKeptParts). Otherwise it is no with a re-enable reason. Disabling or a no-op stays data-only. Without a loaded base it stays advisory.

Comment thread src/utils/agent-config/field-access.ts Outdated

@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 Must-fix, 1 Should-fix.

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/field-access.ts
Comment thread src/utils/agent-config/field-access.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 4 commits October 6, 2026 10:55
Layer 4 of 21 in the stacked split of #318.

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

- apply_safe: re-enabling a plugin the host's file disables applies only
  with a trusted effective source, and re-checks the policies and ${env:}
  references it keeps (classify.go reenables-plugin)
- apply_safe without trusted_sources: reusing a source the file already
  uses is Safe, so `source` is partial instead of read-only
- usedSources skips disabled plugins, as the API does
- registry authorities with % are local sources (Go url.Parse)
- agentconfig-conformance.json: one table of expected results from the
  API's pkg/agentconfig tests, asserted against glob, cron5 and
  field-access so the copies cannot drift silently

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
FORBIDDEN_TOOLTIP reads TOOLTIPS['agents.config.field.forbidden'], per
docs/TOOLTIPS.md (UI-COMP-001).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The conformance fixture is now the API's generated golden file
(pkg/agentconfig/testdata/conformance.json @ c0b3792), byte for byte,
instead of a hand-copied table (CORE-DUP-001).

- fixtures/README.md records the API commit and the update flow
- scripts/sync-agentconfig-conformance.sh [ref] copies it (npm run
  sync:agentconfig-conformance); --check compares it, passing with a
  notice while the API has not published the file
- CI job conformance-drift (pull-request.yml) runs the check against
  api@main; make reviewable does not (no network)
- .prettierignore keeps prettier from reformatting the vendored file
- the spec fails when the golden file gains a table it does not run

The golden file adds the case "re-enable keeps a local plugin source",
which field-access.ts already passes. The two case-mismatched time zones
it drops stay as UI-only cases in cron5.spec.ts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ccf-lisa
ccf-lisa Bot force-pushed the agent-config/04-field-access branch from 21b0227 to e23b899 Compare October 6, 2026 14:20
@gusfcarvalho
gusfcarvalho merged commit f85fd51 into main Oct 6, 2026
7 checks passed
@gusfcarvalho
gusfcarvalho deleted the agent-config/04-field-access 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

size-exception PR intentionally exceeds the 1k-line size limit; reasoning in the description

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants