Skip to content

docs(agent): complete DeclaredCaps exclusion list - #2028

Open
MuskanPaliwal wants to merge 10 commits into
entireio:mainfrom
MuskanPaliwal:fix-declared-caps-builtins
Open

MuskanPaliwal wants to merge 10 commits into
entireio:mainfrom
MuskanPaliwal:fix-declared-caps-builtins

Conversation

@MuskanPaliwal

@MuskanPaliwal MuskanPaliwal commented Aug 17, 2026 •

Copy link
Copy Markdown
Contributor

DeclaredCaps maintains the inventory of capabilities that built-in agents can expose without advertising them through DeclaredCapabilities. That inventory had already drifted more than once: the original documentation missed several helpers, and ToolInvocationScanner was added after this PR opened without updating the list.

This makes the inventory an enforced architectural invariant instead of relying on reviewers to maintain it manually.

What changed

Every package-level As* capability helper now resolves directly through one of the two standard paths:

  • builtinCapability for capabilities that do not require declaration
  • declaredCapability for capabilities gated by DeclaredCaps

The existing SidecarImageProvider, StreamingTextGenerator, ContextInjector, and ModelLister helpers were normalized to that shape without changing their signatures or behavior.

A package-wide AST test now scans production files in the top-level agent package and verifies that:

  • every As* helper uses one of the standard resolvers;
  • the capabilities using builtinCapability exactly match the list documented above DeclaredCaps;
  • the documented list remains alphabetically sorted.

This covers helpers outside capabilities.go, including AsContextInjector and AsModelLister, and catches future additions such as ToolInvocationScanner automatically.

Compatibility

There are no public API or protocol-schema changes. The normalized helpers preserve their existing return values and declaration-gating behavior.

Verification

  • mise run check
  • 56/56 Vogon canaries passed
  • 4/4 external-agent canaries passed
  • A controlled mutation removing ToolInvocationScanner from the documentation caused the architecture test to fail with the expected inventory mismatch

@MuskanPaliwal
MuskanPaliwal marked this pull request as ready for review August 17, 2026 20:46
@MuskanPaliwal
MuskanPaliwal requested a review from a team as a code owner August 17, 2026 20:46
@MuskanPaliwal
MuskanPaliwal marked this pull request as draft August 18, 2026 06:43
@MuskanPaliwal
MuskanPaliwal marked this pull request as ready for review August 18, 2026 06:54
@MuskanPaliwal MuskanPaliwal changed the title docs(agent): complete DeclaredCaps exclusion list docs(agent): complete DeclaredCaps exclusion list Aug 20, 2026
@MuskanPaliwal

Copy link
Copy Markdown
Contributor Author

Hi @Soph , this pr addresses #1877 out-of-scope documentation fix. Would appreciate your review on this. Thanks!

@Soph

Soph commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Verified every entry against the source - an As* helper in capabilities.go is ungated exactly when it resolves by bare type assertion rather than declaredCapability[T], which is 10 helpers. The list goes from 5/10 to 9/10 with this change, and all four additions are correct. SessionEndBudgeter wasn't in the review note you cited, so that one's a genuine find on top of the ask (the PR body says "three interfaces" but the diff adds four - worth reconciling).

One still missing: ToolInvocationScanner (capabilities.go:230). It belongs between SkillEventExtractor and SessionEndBudgeter, which is exactly where the gap is in declaration order.

Not really your miss, though: AsToolInvocationScanner landed on 2026-08-24 in a1dd3efc3, after this PR was opened but before it merged main on the 26th. The list went stale again inside the lifetime of the fix for staleness.

Which is the more useful thing here. This list is hand-maintained with no sync mechanism and has now rotted twice, and all 10 ungated helpers already carry their own, more detailed rationale next to the code - AsHookFreshness explains the template-based drift, AsSessionEndBudgeter explains the parse-hook split. The aggregate list duplicates information that already lives at the definition, which is why it rots.

Two ways to make this the last fix needed:

  1. Stop enumerating. Replace the parenthetical with a pointer: "...are intentionally excluded - their As* helpers resolve by type assertion alone (builtinCapability), each documenting its own reason at the helper." Nothing lost, drift surface gone.
  2. Make it testable. A go/ast test over capabilities.go classifying each func As* by whether it calls declaredCapability, asserting the ungated set matches the names in the doc comment. ~40 lines, fails the moment capability Add MIT License #11 lands.

Happy to take either, or just the one-word addition if you'd rather keep it simple.

@MuskanPaliwal

Copy link
Copy Markdown
Contributor Author

Thanks @Soph, good catch. I went with the AST-backed option.

The follow-up commit makes every package-level As* helper use either builtinCapability or declaredCapability. The test scans production files across the top-level agent package, so it catches helpers outside capabilities.go, including AsContextInjector and AsModelLister. It derives the built-in set from those helpers and checks that the DeclaredCaps list matches it exactly and stays alphabetized.

I also added ToolInvocationScanner and characterization tests for the four normalized helpers. Their signatures, return values, and declaration behavior are unchanged.

The guard currently covers all 23 As* helpers, 13 built-in and 10 declaration-gated, without hard-coding those counts. mise run check passes. I also removed ToolInvocationScanner from the documentation temporarily and confirmed that the test failed with the expected mismatch.

I’ve updated the PR description to match the new scope.

Entire-Checkpoint: 01M2SQ87FWMD6R4JSCHWBXQE3H
@MuskanPaliwal

Copy link
Copy Markdown
Contributor Author

hi @Soph , i have this open for a long, would love to get your feedback here and get it merged . thanks :))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants