Skip to content

Commit 9eedd7f

Browse files
edburnsCopilotCopilot
authored
Edburns/1810 java tool ergonomics tool as lambda seeking review (#1895)
* ADR-006 * Plan * GUTDODP * GUTDODP * On branch edburns/1810-java-tool-ergonomics-tool-as-lambda GOTDODP Your branch is up to date with 'upstream/edburns/1810-java-tool-ergonomics-tool-as-lambda'. Changes to be committed: (use "git restore --staged <file>..." to unstage) modified: 1810-java-tool-ergonomics-tool-as-lambda-remove-before-merge/1810-ignorance-reduction-for-implementation-plan.md modified: 1810-java-tool-ergonomics-tool-as-lambda-remove-before-merge/20260628-prompts.md new file: 1810-java-tool-ergonomics-tool-as-lambda-remove-before-merge/20260629-prompts.md Signed-off-by: Ed Burns <edburns@microsoft.com> * On branch edburns/1810-java-tool-ergonomics-tool-as-lambda Completed Phase 03. modified: 1810-java-tool-ergonomics-tool-as-lambda-remove-before-merge/1810-ignorance-reduction-for-implementation-plan.md modified: 1810-java-tool-ergonomics-tool-as-lambda-remove-before-merge/20260629-prompts.md Signed-off-by: Ed Burns <edburns@microsoft.com> * GUTDODP * GUTDODP * On branch edburns/1810-java-tool-ergonomics-tool-as-lambda modified: 1810-java-tool-ergonomics-tool-as-lambda-remove-before-merge/1810-ignorance-reduction-for-implementation-plan.md - Check off the things already done. modified: 1810-java-tool-ergonomics-tool-as-lambda-remove-before-merge/20260629-prompts.md - GUTDODP * Add Phase 4 checklist linked to child issues Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Initial plan * Add Param<T> public API type for lambda-defined tools (#1839) Introduce `com.github.copilot.tool.Param<T>` — an immutable, fluent runtime parameter metadata class for inline/lambda tool definitions. Validation behavior: - Rejects blank name/description - Rejects required=true with non-empty defaultValue - Validates default values against declared Class<T> Includes comprehensive unit tests (ParamTest, 24 cases). Updates Phase 4.1 checkbox in the implementation plan. * Add Float/Short/Byte default validation test coverage for Param<T> * GUTDODP * Add shepherd-task-to-ready skill Automates the lifecycle of a child Task issue from 'assigned to Copilot' through CI approval and review-agent feedback resolution, stopping just before marking the PR as Ready for Review. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Iterate the skill On branch edburns/1810-java-tool-ergonomics-tool-as-lambda modified: .github/skills/shepherd-task-to-ready/SKILL.md modified: 1810-java-tool-ergonomics-tool-as-lambda-remove-before-merge/20260630-prompts.md Signed-off-by: Ed Burns <edburns@microsoft.com> * Iterate skill * Spotless * Mark 4.1 as complete * Update shepherd skill: rerun for approval, ignore expected failure, fix base branch - Use 'gh run rerun' instead of fork-only approve API endpoint - Ignore expected 'Block remove-before-merge paths' workflow failure - Verify and fix PR base branch after creation (Copilot may target main) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Skill: prepend base branch instruction before assigning to Copilot Avoids race condition where Copilot targets main instead of the specified base branch. Instruction is added to issue body before assignment; base branch verification remains as fallback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Iterate the skill * Skill: add guard against editing plan/checklist files The model should not mark checklist items as complete — that is the human DRI's responsibility. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Skill: clarify checklist editing is out of scope, not DRI-specific Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * GUTDODP * Rename skill: shepherd-task-to-ready → shepherd-task-from-assignment-to-ready Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Working to prompt the second skill. On branch edburns/1810-java-tool-ergonomics-tool-as-lambda modified: 1810-java-tool-ergonomics-tool-as-lambda-remove-before-merge/20260630-prompts.md Signed-off-by: Ed Burns <edburns@microsoft.com> * Add shepherd-task-from-ready-to-merged-to-base skill Follow-up skill that takes a PR from Ready for Review through Copilot code review comment resolution (done locally) and merge to the specified base branch. Max 20 iterations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Find the PR * feat(java): implement ToolDefinition.from* lambda overloads (Phase 4.2) (#1857) * Initial plan * feat(java): implement ToolDefinition.from* overloads for lambda-defined tools (Phase 4.2) Co-authored-by: edburns <75821+edburns@users.noreply.github.com> * Fix review comments: typed defaults, array items schema, primitive cast - buildSchemaFromParams: parse default values to declared type before placing in JSON schema (avoids String defaults for numeric/boolean) - schemaForClass: emit items schema for Java array types using getComponentType() for schema fidelity - coerceDefaultValue: use boxed valueOf() instead of type.cast() for primitive types to avoid ClassCastException Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix review round 3: ToolResultObject passthrough, Optional* schema - formatResult: pass ToolResultObject through directly instead of JSON-serializing, preserving structured result semantics - schemaForClass: add OptionalInt/OptionalLong/OptionalDouble support to match compile-time SchemaGenerator behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix review round 4: Optional* coercion in coerceArg - Return OptionalInt.empty()/OptionalLong.empty()/OptionalDouble.empty() for missing non-required params instead of null - Construct Optional*.of(...) from Number when value is present - Avoids NPE and aligns with annotation-processor behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix review round 5: null-future guards, Optional* cast safety, @SInCE tags - All async fromAsync*/fromAsyncWithToolInvocation handlers now check for null future and return failedFuture with clear NPE message - Optional* coercion catches ClassCastException for non-numeric values and throws IllegalArgumentException with diagnostic message - Fixed @SInCE 1.0.2 -> 1.0.6 on all new API entries (15 in ToolDefinition, 1 in Param) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: edburns <75821+edburns@users.noreply.github.com> Co-authored-by: Ed Burns <edburns@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Skill: add GraphQL thread resolution to Step 8 Use resolveReviewThread mutation to programmatically mark review threads as resolved after replying, instead of requiring manual resolution in the UI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * On branch edburns/1810-java-tool-ergonomics-tool-as-lambda modified: .github/skills/shepherd-task-from-ready-to-merged-to-base/SKILL.md Refine skill for resolving comments. modified: 1810-java-tool-ergonomics-tool-as-lambda-remove-before-merge/20260630-prompts.md - GUTDODP Signed-off-by: Ed Burns <edburns@microsoft.com> * On branch edburns/1810-java-tool-ergonomics-tool-as-lambda modified: .github/skills/shepherd-task-from-ready-to-merged-to-base/SKILL.md - Instruct the agent to close the issue after merging. modified: 1810-java-tool-ergonomics-tool-as-lambda-remove-before-merge/20260630-prompts.md - GUTDODP Signed-off-by: Ed Burns <edburns@microsoft.com> * Add shepherd-task skill: end-to-end orchestrator Invokes shepherd-task-from-assignment-to-ready then shepherd-task-from-ready-to-merged-to-base in sequence. Only proceeds to Phase 2 if Phase 1 succeeds. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * On branch edburns/1810-java-tool-ergonomics-tool-as-lambda modified: .github/skills/shepherd-task/SKILL.md - Tell the agent to mark the task as complete. modified: 1810-java-tool-ergonomics-tool-as-lambda-remove-before-merge/20260630-prompts.md - GUTDODP Signed-off-by: Ed Burns <edburns@microsoft.com> * Invoke skill result * fix: use --body-file to preserve markdown formatting when prepending to issue body The previous approach using inline --body with shell variable interpolation stripped newlines from the original issue body, causing the Copilot cloud agent to receive a wall of unformatted text and misinterpret the assignment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: add idempotency guard for base branch prepend in shepherd skill Skip the issue body prepend if it already starts with '**Base branch:**', preventing double-prepending on retries after partial failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat: add /compact between Phase 1 and Phase 2 in shepherd-task skill Reduces context window usage before entering the review-iteration-heavy Phase 2, retaining only essential state (PR number, branch, inputs). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(java): implement ParamSchema + ParamCoercion internals for Param (Phase 4.3) (#1877) * Initial plan * feat(java): implement schema + coercion internals for Param (Phase 4.3) Fixes #1841 Adds two package-private internal helper classes in com.github.copilot.tool: - ParamSchema: runtime JSON Schema generation from Param<?> metadata. buildSchema() validates duplicate names; forType() mirrors the compile-time SchemaGenerator using java.lang.reflect.Class instead of javax.lang.model. - ParamCoercion: runtime argument coercion using existing ObjectMapper policy. coerce() resolves args → default → empty-optional → required-error in order. coerceDefault() parses string defaults into declared Java types. emptyOptionalOrNull() returns empty Optional variants for optional primitives. Both classes are package-private per resolution 3.8 (no public internal helpers). coerce() takes Map<String,Object> to avoid a cyclic rpc dependency. Updates Phase 4.3 checkbox in plan file. Co-authored-by: edburns <75821+edburns@users.noreply.github.com> * fix(java): address Copilot code review comments on ParamSchema/ParamCoercion - Add null guard for varargs array in buildSchema() - Clarify ParamSchema class Javadoc: simplified counterpart, not full parity - Clarify forType() Javadoc: flat type mapping only, no generics resolution - Clarify coerceDefault() Javadoc: ObjectMapper fallback is safety net only Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(java): correct ParamSchema Javadoc - arrays do produce items schema Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: edburns <75821+edburns@users.noreply.github.com> Co-authored-by: Ed Burns <edburns@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(skills): improve PR discovery with multi-strategy polling The shepherd skill's PR polling only matched by title/branch regex, which fails when Copilot uses descriptive names without the issue number. Now uses three strategies per iteration: A) Issue timeline API for cross-referenced PRs (most reliable) B) PR body search for 'Fixes #N' references C) Title/branch regex match (original fallback) Also increased timeout from 10 to 15 minutes since Copilot can take 5-12 minutes to produce a PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat: add shepherd-task shell scripts (PowerShell + bash) Orchestrates two separate copilot --yolo sessions for Phase 1 and Phase 2, with gh CLI verification between phases. Avoids context window exhaustion by using independent copilot instances instead of /compact. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * [Java] Tool-as-lambda 4.4: Add unit tests for API behavior and validation (#1879) * Initial plan * Phase 4.4: Add ToolDefinitionLambdaTest – unit tests for lambda tool API behavior and validation Co-authored-by: edburns <75821+edburns@users.noreply.github.com> * Address Copilot review: tighten test assertions - requiredParam test: assert IllegalArgumentException directly (not generic Exception via .get()) and verify message mentions param name - resultFormatting test: parse result as JSON and assert specific fields instead of loose string contains check Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: edburns <75821+edburns@users.noreply.github.com> Co-authored-by: Ed Burns <edburns@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: strip remote prefix when comparing merged base branch name GitHub's baseRefName API never includes the remote prefix (e.g., upstream/), so normalize BaseBranch before comparison to avoid false failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * On branch edburns/1810-java-tool-ergonomics-tool-as-lambda new file: 1810-java-tool-ergonomics-tool-as-lambda-remove-before-merge/20260701-prompts.md modified: 1810-java-tool-ergonomics-tool-as-lambda-remove-before-merge/1810-ignorance-reduction-for-implementation-plan.md - When writing the new E2E test, rely on the existing skill. Signed-off-by: Ed Burns <edburns@microsoft.com> * [Java] Add replay-proxy E2E coverage for inline lambda-defined tools (#1881) * Initial plan * Add Java lambda-based E2E tool definition coverage Co-authored-by: edburns <75821+edburns@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: edburns <75821+edburns@users.noreply.github.com> * Extract workflow approval into reusable sub-skill Extract Steps 4-5 (approve pending workflow runs and wait for completion) from shepherd-task-from-assignment-to-ready into a new standalone skill: shepherd-task-approve-workflows-and-wait-for-completion. The original skill now invokes the sub-skill by reference. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add workflow approval calls to ready-to-merged skill Insert invocations of shepherd-task-approve-workflows-and-wait-for-completion before gathering review comments (Step 5), before re-requesting review (Step 11), and before final checks (Step 14). Renumber all steps sequentially (0-21). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * On branch edburns/1810-java-tool-ergonomics-tool-as-lambda modified: 1810-java-tool-ergonomics-tool-as-lambda-remove-before-merge/20260701-prompts.md - GUTDODP Signed-off-by: Ed Burns <edburns@microsoft.com> * Use agent_assignment API to guarantee base branch on Copilot assignment Replace the body-prepend workaround with the REST API's agent_assignment.base_branch field when assigning issues to Copilot. This is the programmatic equivalent of selecting the branch in the GitHub UI and guarantees Copilot creates its topic branch from BASE_BRANCH instead of defaulting to main. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Revert "Use agent_assignment API to guarantee base branch on Copilot assignment" This reverts commit 2a45faf. * Strengthen base branch enforcement for Copilot assignment Add three layers of reinforcement to ensure Copilot uses the correct base branch: 1. More prominent body prepend using GitHub IMPORTANT callout syntax with explicit DO NOT instructions 2. Reinforcing comment posted immediately after assignment 3. Stronger fallback: if PR targets wrong base, fix it AND request Copilot rebase with a changes-requested review This is critical because issue descriptions reference plan files that only exist on the feature branch, not on main. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * On branch edburns/1810-java-tool-ergonomics-tool-as-lambda Your branch is up to date with 'upstream/edburns/1810-java-tool-ergonomics-tool-as-lambda'. Changes not staged for commit: (use "git add <file>..." to update what will be committed) (use "git restore <file>..." to discard changes in working directory) modified: 1810-java-tool-ergonomics-tool-as-lambda-remove-before-merge/20260701-prompts.md no changes added to commit (use "git add" and/or "git commit -a") Signed-off-by: Ed Burns <edburns@microsoft.com> * [Java] Align inline tool docs with final lambda API and ADR links (#1885) * Initial plan * [Java] Align inline tool docs with final lambda API and ADR links - README: Added inline lambda tool authoring section with ToolDefinition.from(...) examples - Documented Param.of(...) required/default behavior and fluent modifiers - ADR-006: Updated to reflect final API (Param.of vs Params.of/ParamDef) - ADR-006: Added ADR-005 cross-reference and README coverage note - Plan: Marked Phase 4.6 as complete Fixes #1884 Co-authored-by: edburns <75821+edburns@users.noreply.github.com> * fix: add CompletableFuture import to async snippet and remove invalid ToolDefer.ALWAYS - Added missing import statement to make the async handler code example self-contained and compilable. - Removed ALWAYS from ToolDefer values list; enum only has NONE, AUTO, NEVER. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: edburns <75821+edburns@users.noreply.github.com> Co-authored-by: Ed Burns <edburns@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Removing these files from this topic branch Skills: • `.github/skills/shepherd-task-approve-workflows-and-wait-for-completion/SKILL.md` • `.github/skills/shepherd-task-from-assignment-to-ready/SKILL.md` • `.github/skills/shepherd-task-from-ready-to-merged-to-base/SKILL.md` • `.github/skills/shepherd-task/SKILL.md` Scripts: • `1810-java-tool-ergonomics-tool-as-lambda-remove-before-merge/shepherd-task.ps1` • `1810-java-tool-ergonomics-tool-as-lambda-remove-before-merge/shepherd-task.sh` This work is now being tracked in **Feature** #1893. * Remove prompts before seeking review * java: delegate ToolDefinition schema/coercion to ParamSchema and ParamCoercion Move ParamSchema and ParamCoercion from com.github.copilot.tool to com.github.copilot.rpc (package-private). Update all from*/fromAsync*/ fromWithToolInvocation*/fromAsyncWithToolInvocation* overloads to delegate to these helpers instead of inline private methods. Remove dead private methods from ToolDefinition: buildSchemaFromParams, schemaForClass, coerceArg, coerceDefaultValue, emptyOptionalOrNull, requireNonNullParam, requireUniqueParamNames. Addresses PR reviewer feedback about unused classes and duplicated logic. * java: cache getConfiguredMapper() in local variable per invocation Capture the singleton ObjectMapper once at the top of each from* factory method instead of calling getConfiguredMapper() multiple times. The lambda closes over the local, making it explicit that schema building, coercion, and result formatting all use the same mapper instance. Addresses Copilot AI reviewer suggestion (discussion r3514594600). --------- Signed-off-by: Ed Burns <edburns@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: edburns <75821+edburns@users.noreply.github.com>
1 parent e7d5e9a commit 9eedd7f

11 files changed

Lines changed: 3123 additions & 2 deletions

File tree

‎java/README.md‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,73 @@ public String onlyContext(ToolInvocation invocation) { ... }
165165
public String report(@CopilotToolParam("Phase") String phase, ToolInvocation invocation, @CopilotToolParam("Limit") int limit) { ... }
166166
```
167167

168+
## Inline lambda tool definitions (experimental)
169+
170+
For inline tool authoring at the session construction site, use `ToolDefinition.from(...)` with explicit parameter metadata:
171+
172+
```java
173+
import com.github.copilot.rpc.ToolDefinition;
174+
import com.github.copilot.rpc.ToolDefer;
175+
import com.github.copilot.tool.Param;
176+
177+
ToolDefinition search = ToolDefinition
178+
.from(
179+
"search_items",
180+
"Searches indexed items by keyword",
181+
Param.of(String.class, "keyword", "Search keyword"),
182+
keyword -> "Searching for: " + keyword)
183+
.skipPermission(true)
184+
.defer(ToolDefer.AUTO);
185+
```
186+
187+
### Parameter metadata with `Param.of(...)`
188+
189+
`Param.of(type, name, description)` creates a required parameter. For optional parameters with defaults:
190+
191+
```java
192+
Param<Integer> limit = Param.of(Integer.class, "limit", "Max results", false, "10");
193+
```
194+
195+
### Async handlers
196+
197+
Use `fromAsync` for asynchronous tool handlers:
198+
199+
```java
200+
import java.util.concurrent.CompletableFuture;
201+
202+
ToolDefinition fetchData = ToolDefinition.fromAsync(
203+
"fetch_data",
204+
"Fetches data from remote source",
205+
Param.of(String.class, "url", "Data source URL"),
206+
url -> CompletableFuture.supplyAsync(() -> fetchRemote(url))
207+
);
208+
```
209+
210+
### ToolInvocation context injection
211+
212+
Inline tools can access `ToolInvocation` runtime context using `fromWithToolInvocation`:
213+
214+
```java
215+
ToolDefinition reportPhase = ToolDefinition.fromWithToolInvocation(
216+
"report_phase",
217+
"Reports the current phase with invocation context",
218+
Param.of(String.class, "phase", "The current phase"),
219+
(phase, invocation) -> "phase=" + phase + ", toolCallId=" + invocation.getToolCallId()
220+
);
221+
```
222+
223+
For async with `ToolInvocation`, use `fromAsyncWithToolInvocation`.
224+
225+
### Fluent option modifiers
226+
227+
Chain fluent modifiers to set tool options:
228+
229+
- `.skipPermission(boolean)` — bypass permission prompts
230+
- `.defer(ToolDefer)` — control deferred execution (`AUTO`, `NEVER`)
231+
- `.overridesBuiltInTool(boolean)` — shadow built-in tools
232+
233+
For design context and decision rationale, see [ADR-006](docs/adr/adr-006-tool-definition-inline.md).
234+
168235
## Memory
169236

170237
Sessions can opt into persistent memory, allowing the agent to read and write memory across turns. Memory is configured per session and applies to both `createSession` and `resumeSession`.
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
# ADR-006: Inline tool definition with lambdas
2+
3+
## Context and problem statement
4+
5+
[ADR-005](adr-005-tool-definition.md) introduced an ergonomic Java tools API based on `@CopilotTool` method annotations, `@CopilotToolParam` parameter annotations, and `ToolDefinition.fromObject(...)` for reflection-based tool registration. That model works well when teams define tools as methods on a class.
6+
7+
The next ergonomics goal is an inline style comparable to C# `CopilotTool.DefineTool(...)`, where developers can define a tool at the call site without creating a separate tool container class.
8+
9+
For this decision, we evaluated two alternatives:
10+
11+
* Method-reference registration (`ToolDefinition.from(tools::setCurrentPhase)`)
12+
* Inline lambda registration (`ToolDefinition.from(..., phase -> ...)`)
13+
14+
The key factor is metadata quality: tool name, description, parameter names, parameter descriptions, required/default semantics, and schema stability.
15+
16+
## Considered options
17+
18+
### Option 1: Method-reference API
19+
20+
Example:
21+
22+
```java
23+
ToolDefinition setPhase = ToolDefinition.from(tools::setCurrentPhase);
24+
```
25+
26+
In this model, metadata is sourced from existing method-level annotations (`@CopilotTool`, `@Param`) on the referenced method.
27+
28+
Advantages:
29+
30+
* Closest Java analog to C# method-group ergonomics
31+
* High-quality metadata with minimal additional API surface
32+
* Reuses ADR-005 metadata and invocation behavior directly
33+
34+
Drawbacks:
35+
36+
* Not truly inline: still requires a declared method (and usually annotations) elsewhere
37+
* Does not solve the "define the whole tool at the call site" use case
38+
* Method-reference resolution adds runtime/reflection complexity
39+
40+
### Option 2: Inline lambda API with explicit metadata
41+
42+
Example:
43+
44+
```java
45+
ToolDefinition setPhase = ToolDefinition.from(
46+
"set_current_phase",
47+
"Sets the current phase of the agent",
48+
Param.of(String.class, "phase", "The phase to transition to"),
49+
(String phase) -> {
50+
currentPhase = phase;
51+
return "Phase set to " + phase;
52+
});
53+
```
54+
55+
In this model, handler logic is inline, and metadata is provided explicitly through `Param.of(...)` parameter definitions.
56+
57+
Advantages:
58+
59+
* True inline authoring at the session construction site
60+
* No dependence on lambda parameter-name reflection or `-parameters`
61+
* Deterministic metadata and schema generation
62+
* Independent from annotation processing and generated companion classes
63+
64+
Drawbacks:
65+
66+
* Slightly more verbose than method-reference style because metadata is explicit
67+
* Introduces new public API types for parameter definitions and typed lambda overloads
68+
* Requires careful API design to stay concise for common one-parameter tools
69+
70+
## Decision outcome
71+
72+
Chosen: **Option 2 for ADR-006 scope** — inline lambda API with explicit metadata.
73+
74+
Rationale:
75+
76+
1. The primary requirement for this ADR is inline definition. Option 2 satisfies it directly; Option 1 does not.
77+
1. Metadata quality is the critical requirement. Option 2 keeps metadata explicit and stable, instead of relying on fragile lambda introspection.
78+
1. Option 2 can ship independently of method-reference support and without changes to annotation processing.
79+
1. Option 2 preserves behavior parity with existing tool execution by delegating to `ToolDefinition` construction and current invocation semantics.
80+
81+
Option 1 remains valuable and can be added independently as a separate ergonomic layer. It is not blocked by this decision.
82+
83+
## Design constraints and non-goals
84+
85+
Constraints for the inline lambda API:
86+
87+
* Require explicit tool name and description.
88+
* Require explicit parameter metadata (at minimum name and type, with optional description/required/default).
89+
* Support both sync and async handlers (`R` and `CompletableFuture<R>`).
90+
* Keep result semantics aligned with existing behavior (`String` passthrough, `void` maps to `"Success"`, non-string objects serialized to JSON).
91+
* Keep override/permission/defer flags available through options, consistent with existing `ToolDefinition` fields.
92+
93+
Non-goals for this ADR:
94+
95+
* Replacing `@CopilotTool`/`fromObject` APIs.
96+
* Defining method-reference registration behavior in detail.
97+
* Introducing compile-time code generation for lambda metadata.
98+
99+
## Consequences
100+
101+
The SDK now provides an explicit inline path for developers who prefer to keep tool declarations at session creation while preserving high-quality schema metadata. Implemented API families include:
102+
103+
- `ToolDefinition.from(name, description, [params...], handler)` — sync handlers
104+
- `ToolDefinition.fromAsync(name, description, [params...], asyncHandler)` — async handlers returning `CompletableFuture<R>`
105+
- `ToolDefinition.fromWithToolInvocation(...)` — sync with `ToolInvocation` context injection
106+
- `ToolDefinition.fromAsyncWithToolInvocation(...)` — async with `ToolInvocation` context injection
107+
108+
Parameter metadata is defined using `Param.of(type, name, description)` for required parameters and `Param.of(type, name, description, required, defaultValue)` for optional parameters with defaults.
109+
110+
Fluent option modifiers (`.skipPermission(boolean)`, `.defer(ToolDefer)`, `.overridesBuiltInTool(boolean)`) allow post-construction customization.
111+
112+
The annotation-driven API from [ADR-005](adr-005-tool-definition.md) remains the recommended path for larger tool surfaces where co-locating metadata with method implementations improves maintainability. For usage examples and complete API coverage, see the Java SDK README.
113+
114+
## Related work items
115+
116+
* #1682
117+
* #1792
118+
* #1810
Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
*--------------------------------------------------------------------------------------------*/
4+
5+
package com.github.copilot.rpc;
6+
7+
import java.util.Map;
8+
9+
import com.fasterxml.jackson.databind.ObjectMapper;
10+
import com.github.copilot.tool.Param;
11+
12+
/**
13+
* Internal runtime helper: coerces raw invocation arguments to the typed values
14+
* declared by {@link Param} descriptors.
15+
*
16+
* <p>
17+
* Reuses the SDK-configured {@link ObjectMapper} for complex type conversions,
18+
* matching the coercion policy applied by existing ergonomic tooling. No
19+
* bespoke conversion paths are introduced.
20+
*
21+
* <p>
22+
* Package-private: not part of the public API.
23+
*/
24+
class ParamCoercion {
25+
26+
/** Utility class; do not instantiate. */
27+
private ParamCoercion() {
28+
}
29+
30+
/**
31+
* Coerces the named argument from an invocation argument map to the Java type
32+
* declared by {@code param}.
33+
*
34+
* <p>
35+
* Resolution order:
36+
* <ol>
37+
* <li>If the argument is present, convert it to {@code T} via
38+
* {@link ObjectMapper#convertValue}.</li>
39+
* <li>If absent and a default value is set, parse the string default via
40+
* {@link #coerceDefault}.</li>
41+
* <li>If absent and the parameter is optional ({@code required=false}), return
42+
* an empty Optional variant or {@code null}.</li>
43+
* <li>If absent and required, throw {@link IllegalArgumentException} with the
44+
* parameter name.</li>
45+
* </ol>
46+
*
47+
* @param <T>
48+
* the target Java type
49+
* @param args
50+
* the invocation argument map; may be {@code null} for zero-argument
51+
* tools
52+
* @param param
53+
* the parameter descriptor
54+
* @param mapper
55+
* the configured {@link ObjectMapper} for complex type conversion
56+
* @return the coerced argument value
57+
* @throws IllegalArgumentException
58+
* if a required parameter is missing or coercion fails
59+
*/
60+
@SuppressWarnings("unchecked")
61+
static <T> T coerce(Map<String, Object> args, Param<T> param, ObjectMapper mapper) {
62+
Object raw = (args != null) ? args.get(param.name()) : null;
63+
64+
if (raw == null) {
65+
if (param.hasDefaultValue()) {
66+
return coerceDefault(param, mapper);
67+
} else if (!param.required()) {
68+
return (T) emptyOptionalOrNull(param.type());
69+
} else {
70+
throw new IllegalArgumentException(
71+
"Required parameter '" + param.name() + "' is missing from tool invocation");
72+
}
73+
}
74+
75+
Class<T> type = param.type();
76+
77+
// Handle Optional* types explicitly before delegating to ObjectMapper
78+
if (type == java.util.OptionalInt.class) {
79+
try {
80+
return (T) java.util.OptionalInt.of(((Number) raw).intValue());
81+
} catch (ClassCastException ex) {
82+
throw new IllegalArgumentException("Parameter '" + param.name()
83+
+ "' expected a numeric value for OptionalInt, got: " + raw.getClass().getSimpleName(), ex);
84+
}
85+
}
86+
if (type == java.util.OptionalLong.class) {
87+
try {
88+
return (T) java.util.OptionalLong.of(((Number) raw).longValue());
89+
} catch (ClassCastException ex) {
90+
throw new IllegalArgumentException("Parameter '" + param.name()
91+
+ "' expected a numeric value for OptionalLong, got: " + raw.getClass().getSimpleName(), ex);
92+
}
93+
}
94+
if (type == java.util.OptionalDouble.class) {
95+
try {
96+
return (T) java.util.OptionalDouble.of(((Number) raw).doubleValue());
97+
} catch (ClassCastException ex) {
98+
throw new IllegalArgumentException("Parameter '" + param.name()
99+
+ "' expected a numeric value for OptionalDouble, got: " + raw.getClass().getSimpleName(), ex);
100+
}
101+
}
102+
103+
try {
104+
return mapper.convertValue(raw, type);
105+
} catch (IllegalArgumentException ex) {
106+
throw new IllegalArgumentException(
107+
"Failed to coerce parameter '" + param.name() + "' to type " + type.getSimpleName(), ex);
108+
}
109+
}
110+
111+
/**
112+
* Parses a {@link Param}'s string default value into the declared Java type.
113+
*
114+
* <p>
115+
* Handles primitives, boxed types, {@link String}, {@link Boolean}, and enums
116+
* explicitly, mirroring the validation logic in {@link Param}. The
117+
* {@link ObjectMapper#readValue} fallback exists as a safety net but is not
118+
* expected to be reached in practice, since {@link Param} construction rejects
119+
* defaults for non-primitive/boxed/String/Boolean/enum types.
120+
*
121+
* @param <T>
122+
* the target Java type
123+
* @param param
124+
* the parameter descriptor carrying the default value
125+
* @param mapper
126+
* the configured {@link ObjectMapper} used as fallback for complex
127+
* types
128+
* @return the parsed default value
129+
* @throws IllegalArgumentException
130+
* if parsing fails
131+
*/
132+
@SuppressWarnings({"rawtypes", "unchecked"})
133+
static <T> T coerceDefault(Param<T> param, ObjectMapper mapper) {
134+
String defaultValue = param.defaultValue();
135+
Class<T> type = param.type();
136+
try {
137+
if (type == String.class) {
138+
return type.cast(defaultValue);
139+
}
140+
if (type == Integer.class || type == int.class) {
141+
return (T) Integer.valueOf(defaultValue);
142+
}
143+
if (type == Long.class || type == long.class) {
144+
return (T) Long.valueOf(defaultValue);
145+
}
146+
if (type == Double.class || type == double.class) {
147+
return (T) Double.valueOf(defaultValue);
148+
}
149+
if (type == Float.class || type == float.class) {
150+
return (T) Float.valueOf(defaultValue);
151+
}
152+
if (type == Short.class || type == short.class) {
153+
return (T) Short.valueOf(defaultValue);
154+
}
155+
if (type == Byte.class || type == byte.class) {
156+
return (T) Byte.valueOf(defaultValue);
157+
}
158+
if (type == Boolean.class || type == boolean.class) {
159+
return (T) Boolean.valueOf(defaultValue);
160+
}
161+
if (type.isEnum()) {
162+
Class<? extends Enum> enumType = (Class<? extends Enum>) type;
163+
return type.cast(Enum.valueOf(enumType, defaultValue));
164+
}
165+
// Fallback: let ObjectMapper parse the JSON-encoded default string
166+
return mapper.readValue(defaultValue, type);
167+
} catch (IllegalArgumentException ex) {
168+
throw ex;
169+
} catch (Exception ex) {
170+
throw new IllegalArgumentException("Failed to apply default value '" + defaultValue + "' for parameter '"
171+
+ param.name() + "' of type " + type.getSimpleName(), ex);
172+
}
173+
}
174+
175+
/**
176+
* Returns an empty Optional variant for Optional primitive types, or
177+
* {@code null} for all other types.
178+
*
179+
* @param type
180+
* the declared parameter type
181+
* @return {@link java.util.OptionalInt#empty()},
182+
* {@link java.util.OptionalLong#empty()},
183+
* {@link java.util.OptionalDouble#empty()}, or {@code null}
184+
*/
185+
static Object emptyOptionalOrNull(Class<?> type) {
186+
if (type == java.util.OptionalInt.class) {
187+
return java.util.OptionalInt.empty();
188+
}
189+
if (type == java.util.OptionalLong.class) {
190+
return java.util.OptionalLong.empty();
191+
}
192+
if (type == java.util.OptionalDouble.class) {
193+
return java.util.OptionalDouble.empty();
194+
}
195+
return null;
196+
}
197+
}

0 commit comments

Comments
 (0)