[highb]

Squire (openai/gpt-6.1-sol):

Add connector-defined nonsecret request inputs and declared output binding to native credential issuance. This PR is stacked above the simple JSON builders in #1194, which includes output declarations from #1192. Its tip is the combined SDK revision C1 can pin.

Selected option and output resource identity choose the descriptor. request_schema reuses config.Field/Constraint; structured values flow through the native request, issuer input and task bridge. Request schema, bounds, defaults, presence and redaction checks remain. Hosts send request data after the selected descriptor advertises that schema; older connectors may ignore the additive request field.

Optional secret_content_type field 12 declares the payload type independently of output resource identity and PlaintextData.schema. Absence preserves legacy generic behavior; invalid declarations reject. Non-generic delivery retains the matching JWE recipient purpose/version/type context before Issue. These delivery checks do not validate the internal payload schema.

Producer helpers are now plain JSON/text builders. The native test demonstrates that an empty password value still serializes and encrypts. No copied payload catalog, semantic validation or Multipass contract-export PR is required. Existing type/version metadata is used without a new JSON wrapper or crypto format. Multipass handles its own display/error behavior.

Validation: six affected packages pass tests/build/vet and CI-pinned affected lint; protobuf build/lint/format and breaking checks against the exact current upstream commit pass. Root independently ran uncached builder and connector-builder tests. Existing request, declaration, selected-output, recipient/context, encryption and legacy controls are preserved. Historical race/full-build/proof receipts retain their original revisions and do not establish a new live provider or C1 storage/retrieval run. The prior unrelated uhttp lint baseline is not waived.

C1 adoption, source review and hosted checks are tracked separately. No merge, SDK release, deployment, live-provider or permission expansion is authorized by this PR.

[linear-code]

IGA-4093

[github-actions]

Superseded — see the current review report for commit 666853a76d23

General PR Review: IGA-4093: validate typed inputs for native credential issuance

Blocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 76264ae5a90c.
Review mode: full
View review run

Review Summary

What the change does:

• Adds an optional CredentialIssueRequestSchema, built from config.Field and Constraint, as field 11 on CredentialIssueOptionDescriptor.
• Adds a request_data Struct as field 6 on IssueCredentialRequest and field 5 on the baton IssueCredentialTask, and passes it through the task handler into CredentialIssueInput.RequestData.
• Checks the schema when capabilities are published (validateCredentialIssueCapabilityDetails). Checks the data in validateCredentialIssueInput before issuer.Issue runs. A connector schema error maps to codes.Internal; a caller data error maps to InvalidArgument.

Coverage: I scanned the full PR diff for security and correctness, and read credential_issue_validation.go in full. I sampled the generated pb files and the 1.5k-line test file; I did not read them line by line.

Criteria applied:

• Triage: Silence is low, because validation fails closed with errors. Durability is yes: there are new wire fields on capability, RPC and task protos. Uncontrolled dimensions is yes, because the outcome depends on which SDK version the connector runs (see the suggestion). Consumer distance is high: the c1 platform and connectors read these fields. Verdict: HIGH (version-pair dependence). This falls in the review-blind multi-artifact/version class. The instrument would be a two-version harness (old-SDK connector receiving request_data). The PR has extensive permutation tables but no cross-version test, so a pass-set review per docs/BUG_CATCHING.md §6 of the host contract is recommended before the C1 integration layer lands.
• Proto compatibility: The changes are additive with unused field numbers, and the generated code is regenerated.
• Go API: Exported changes are additive only: a new RequestData field, ValidateCredentialIssueRequestSchema/Data, ErrInvalidCredentialIssueRequestSchema, and field.ValidateInt64Rules. ValidateIntRules now delegates to ValidateInt64Rules with the same behavior. Changing the unexported builder.m to an interface does not change behavior.
• Defaults: Unchanged for connectors that don't advertise a schema.
• Version bump: None is needed for this additive change. Dependency manifests are unchanged.

Security Issues

None found. Error messages leave out submitted values and unknown keys. Size, field-count and item-count caps bound allocations. Patterns are RE2 (Go regexp), so they can't cause catastrophic backtracking. Secret fields are rejected at schema admission.

Correctness Issues

None found.

Suggestions

• New (medium confidence) — proto/c1/connector/v2/resource.proto:179: A connector on an older SDK silently drops request_data (field 6 is unknown to it) and still issues the credential. Only new-SDK connectors fail closed (ValidateCredentialIssueRequestData rejects any key when the schema is nil). The safety of the feature therefore depends on an unstated host contract: send request_data only when the selected descriptor advertises request_schema. Document this on the field. Consider adding a test that pins "nil schema + non-empty data → InvalidArgument" at the IssueCredential boundary.

Prompt for AI agents

Verify each finding against the current code and only fix it if needed.

## Suggestions

In `proto/c1/connector/v2/resource.proto`:
- Around line 177-179: Extend the comment on `request_data` to state the host contract: hosts must send request_data only when the selected CredentialIssueOptionDescriptor advertises request_schema. Connectors built with older SDKs ignore this field as an unknown field and would issue without the inputs. Regenerate pb/c1/connector/v2/*.pb.go after editing. Optionally add a test in pkg/connectorbuilder/credentials_issue_test.go asserting that IssueCredential returns codes.InvalidArgument when the descriptor has no request_schema but request_data is non-empty.

[github-actions]

🟡 Suggestion (medium confidence): A connector on an older SDK drops field 6 as an unknown field and still calls Issue. Only the new SDK fails closed (an unknown key against a nil schema is rejected). So this feature is safe only if hosts send request_data when the selected descriptor advertises a request_schema. Please state that host contract in this comment, and consider a test that pins how a nil schema plus non-empty data is handled.

[github-actions]

Superseded — see the current review report for commit d1a58263345c

General PR Review: IGA-4093: validate typed inputs for native credential issuance

Blocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 76264ae5a90c.
Review mode: full
View review run

Review Summary

What the change does:

• Adds an optional CredentialIssueRequestSchema, built from config.Field and Constraint, as field 11 on CredentialIssueOptionDescriptor.
• Adds a request_data Struct as field 6 on IssueCredentialRequest and field 5 on the baton IssueCredentialTask, and passes it through the task handler into CredentialIssueInput.RequestData.
• Checks the schema when capabilities are published (validateCredentialIssueCapabilityDetails). Checks the data in validateCredentialIssueInput before issuer.Issue runs. A connector schema error maps to codes.Internal; a caller data error maps to InvalidArgument.

Coverage: I scanned the full PR diff for security and correctness, and read credential_issue_validation.go in full. I sampled the generated pb files and the 1.5k-line test file; I did not read them line by line.

Criteria applied:

• Triage: Silence is low, because validation fails closed with errors. Durability is yes: there are new wire fields on capability, RPC and task protos. Uncontrolled dimensions is yes, because the outcome depends on which SDK version the connector runs (see the suggestion). Consumer distance is high: the c1 platform and connectors read these fields. Verdict: HIGH (version-pair dependence). This falls in the review-blind multi-artifact/version class. The instrument would be a two-version harness (old-SDK connector receiving request_data). The PR has extensive permutation tables but no cross-version test, so a pass-set review per docs/BUG_CATCHING.md §6 of the host contract is recommended before the C1 integration layer lands.
• Proto compatibility: The changes are additive with unused field numbers, and the generated code is regenerated.
• Go API: Exported changes are additive only: a new RequestData field, ValidateCredentialIssueRequestSchema/Data, ErrInvalidCredentialIssueRequestSchema, and field.ValidateInt64Rules. ValidateIntRules now delegates to ValidateInt64Rules with the same behavior. Changing the unexported builder.m to an interface does not change behavior.
• Defaults: Unchanged for connectors that don't advertise a schema.
• Version bump: None is needed for this additive change. Dependency manifests are unchanged.

Security Issues

None found. Error messages leave out submitted values and unknown keys. Size, field-count and item-count caps bound allocations. Patterns are RE2 (Go regexp), so they can't cause catastrophic backtracking. Secret fields are rejected at schema admission.

Correctness Issues

None found.

Suggestions

• New (medium confidence) — proto/c1/connector/v2/resource.proto:179: A connector on an older SDK silently drops request_data (field 6 is unknown to it) and still issues the credential. Only new-SDK connectors fail closed (ValidateCredentialIssueRequestData rejects any key when the schema is nil). The safety of the feature therefore depends on an unstated host contract: send request_data only when the selected descriptor advertises request_schema. Document this on the field. Consider adding a test that pins "nil schema + non-empty data → InvalidArgument" at the IssueCredential boundary.

Prompt for AI agents

Verify each finding against the current code and only fix it if needed.

## Suggestions

In `proto/c1/connector/v2/resource.proto`:
- Around line 177-179: Extend the comment on `request_data` to state the host contract: hosts must send request_data only when the selected CredentialIssueOptionDescriptor advertises request_schema. Connectors built with older SDKs ignore this field as an unknown field and would issue without the inputs. Regenerate pb/c1/connector/v2/*.pb.go after editing. Optionally add a test in pkg/connectorbuilder/credentials_issue_test.go asserting that IssueCredential returns codes.InvalidArgument when the descriptor has no request_schema but request_data is non-empty.

---

Reviewed commit: 666853a76d23

[github-actions]

No blocking issues found — see the full review report

[github-actions]

Superseded — see the current review report for commit d1a58263345c

General PR Review: IGA-4093: validate typed inputs for native credential issuance

Blocking Issues: 0 | Suggestions: 3 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 76264ae5a90c.
Review mode: incremental since 666853a7
View review run

Review Summary

What the PR does: It adds a connector-defined CredentialIssueRequestSchema (field 11) and a request_data Struct (field 6 on IssueCredentialRequest, field 5 on the baton IssueCredentialTask). The SDK checks the schema when capabilities are published and checks the data before issuer.Issue runs.

What the new commits change: They add typed credential output. There is a new secret_content_type (field 12) on CredentialIssueOptionDescriptor. IssueCredential refuses a non-generic type unless the request has exactly one JWE recipient whose AAD names that type. The AAD/type helpers moved from pkg/actions into pkg/crypto with the same logic. Request schemas now reject output-only secret_content_type markers. Generated pb/validate code is regenerated to match the proto changes.

Coverage: I scanned the full PR diff (24 files) for security and correctness issues. The incremental artifact also contains vendor/jsonschema, go.mod/go.sum, Makefile, pkg/secretcontracts and pkg/actions hunks. None of those appear in the PR-vs-base diff, so they are already on the base and are not this PR's surface.

Criteria: I applied the trusted criteria as follows:

• Triage:
  • Silence: low. It fails closed with InvalidArgument/Internal.
  • Durability: yes. New proto wire fields 11/12/6/5.
  • Version-pair dependence: yes. An old-SDK connector ignores request_data.
  • Consumer distance: high. The c1 platform reads these fields.
  • Verdict: HIGH. It falls in the multi-artifact/version class. The instrument that would cover it is a two-version harness, and the PR doesn't include one. A §6 pass-set review is still recommended before the C1 integration layer.
• Wire compatibility: All proto changes are additive with new field numbers and regenerated code.
• Go API: Exported changes are additive. New: RequestData, ValidateCredentialIssueRequestSchema/Data, crypto.ValidSecretContentType, crypto.TypedOutputContextMatches, field.ValidateInt64Rules.
• Defaults: Unchanged when secret_content_type is absent or generic.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

• Prior — still present (medium confidence): proto/c1/connector/v2/resource.proto:177-179. The request_data comment still doesn't state the host contract. Hosts must send request_data only when the selected descriptor advertises request_schema, because connectors on older SDKs drop field 6 as unknown and issue without the inputs.
• New: pkg/connectorbuilder/credentials.go:238. The PR description doesn't mention the typed-output feature (descriptor field 12 plus the JWE AAD gate). Hosts must send the typed AAD for typed descriptors or issuance is refused. Describe this in the PR as a rollout note.
• New (medium confidence): pkg/connectorbuilder/credential_typed_output_test.go:122,177. The rejection tests assert only require.Error and calls == 0. They would still pass if a different preflight check failed first, so they don't show the typed-context guard is what rejects. Assert the status code and message.

Prompt for AI agents

Verify each finding against the current code and only fix it if needed.

## Suggestions

In `proto/c1/connector/v2/resource.proto`:
- Around line 177-179: Extend the comment on `request_data` to state the host contract. Hosts must send request_data only when the selected CredentialIssueOptionDescriptor advertises request_schema, because connectors built with older SDKs ignore this field as unknown and would issue without the inputs. Regenerate pb/c1/connector/v2/*.pb.go afterward.

In `pkg/connectorbuilder/credentials.go`:
- Around line 238: Update the PR description to cover the typed-output addition: CredentialIssueOptionDescriptor.secret_content_type (field 12), plus the rule that a non-generic type requires exactly one JWE recipient whose AAD has purpose "c1/fk-credential-delivery", version 1 and a matching content_type. Include the host rollout requirement.

In `pkg/connectorbuilder/credential_typed_output_test.go`:
- Around lines 122 and 177: In TestNativeTypedRejectsContextBeforeIssue and TestNativeTypedRejectsOtherRecipientsBeforeIssue, add require.Equal(t, codes.InvalidArgument, status.Code(err)) and require.ErrorContains(t, err, "typed credential requires a matching JWE recipient context"). Then the tests fail if the typed-context guard is removed.

[github-actions]

No blocking issues found — see the full review report

[github-actions]

Superseded — see the current review report for commit f76f69bb274b

General PR Review: IGA-4093: Bind native credential inputs and typed outputs

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: none loaded - base ref highb/secret-payload-contract-bundle is not the default branch main for .claude/skills/ci-review.md.
Review mode: full
View review run

Review Summary

What the change does

• Request schema: credential issue descriptors get a connector-owned request_schema (field 11), built from config.Field and Constraint. It is checked when capabilities are published, against limits on size, field count, constraint count, regex validity, supported types, defaults, and whether the required fields can fit at all.
• Request data: request_data (a Struct) is carried from the task to IssueCredentialRequest to CredentialIssueInput. ValidateCredentialIssueRequestData checks it before Issue. A bad schema returns Internal; bad caller data returns InvalidArgument.
• Typed output: a descriptor can declare an optional secret_content_type (field 12). Any non-generic type requires exactly one JWE recipient whose AAD matches the type.
• Refactor: the AAD matcher and type-pattern check move unchanged from pkg/actions into pkg/crypto.
• New helper: field.ValidateInt64Rules is added so 64-bit integer rules aren't narrowed to the platform's int.

Coverage

• I scanned the full PR diff for security and correctness.
• I checked the generated pb/* files only for consistency with the .proto changes, not line by line.
• No repo-local criteria loaded, so I applied the base criteria and the SDK-compatibility criteria. The exported Go and proto changes are additive, and an absent or generic content type keeps the old behavior.

Checks that came back clean

• Input limits: request data is capped at 64 fields and 64 KiB, and lists and maps at 64 items.
• Integers: values must be finite whole numbers within ±2^53, so converting to int64 is safe.
• Regex: patterns use Go's RE2 engine, which runs in linear time.
• Metrics: builder.m changed from *metrics.M to an unexported interface. All existing call sites are unchanged.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

Resolved prior findings

• Fixed: pkg/connectorbuilder/credentials.go:238 (typed-output rollout not described in the PR). The PR description now documents secret_content_type field 12, the legacy-generic default, and the JWE recipient-context requirement before Issue. The gate is at credentials.go:238-245, and the host contract for request_data is at proto/c1/connector/v2/resource.proto:177-180.

[github-actions]

Superseded — see the current review report for commit 7da2bbacafb1

General PR Review: IGA-4093: Bind native credential inputs and typed outputs

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: none loaded - base ref highb/secret-payload-contract-bundle is not the default branch main for .claude/skills/ci-review.md.
Review mode: full
View review run

Review Summary

What the change does

• Request schema: credential issue descriptors get a connector-owned request_schema (field 11), built from config.Field and Constraint. It is checked when capabilities are published, against limits on size, field count, constraint count, regex validity, supported types, defaults, and whether the required fields can fit at all.
• Request data: request_data (a Struct) is carried from the task to IssueCredentialRequest to CredentialIssueInput. ValidateCredentialIssueRequestData checks it before Issue. A bad schema returns Internal; bad caller data returns InvalidArgument.
• Typed output: a descriptor can declare an optional secret_content_type (field 12). Any non-generic type requires exactly one JWE recipient whose AAD matches the type.
• Refactor: the AAD matcher and type-pattern check move unchanged from pkg/actions into pkg/crypto.
• New helper: field.ValidateInt64Rules is added so 64-bit integer rules aren't narrowed to the platform's int.

Coverage

• I scanned the full PR diff for security and correctness.
• I checked the generated pb/* files only for consistency with the .proto changes, not line by line.
• No repo-local criteria loaded, so I applied the base criteria and the SDK-compatibility criteria. The exported Go and proto changes are additive, and an absent or generic content type keeps the old behavior.

Checks that came back clean

• Input limits: request data is capped at 64 fields and 64 KiB, and lists and maps at 64 items.
• Integers: values must be finite whole numbers within ±2^53, so converting to int64 is safe.
• Regex: patterns use Go's RE2 engine, which runs in linear time.
• Metrics: builder.m changed from *metrics.M to an unexported interface. All existing call sites are unchanged.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

Resolved prior findings

• Fixed: pkg/connectorbuilder/credentials.go:238 (typed-output rollout not described in the PR). The PR description now documents secret_content_type field 12, the legacy-generic default, and the JWE recipient-context requirement before Issue. The gate is at credentials.go:238-245, and the host contract for request_data is at proto/c1/connector/v2/resource.proto:177-180.

---

Reviewed commit: f76f69bb274b

[github-actions]

No blocking issues found — see the full review report

[github-actions]

Superseded — see the current review report for commit 7da2bbacafb1

General PR Review: IGA-4093: Bind native credential inputs and typed outputs

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: none loaded - base ref highb/secret-payload-contract-bundle is not the default branch main for .claude/skills/ci-review.md.
Review mode: full
View review run

Review Summary

This PR adds two things to native credential issuance:

• Request schema and data: a connector-owned request_schema (descriptor field 11, built from config.Field/Constraint) and a request_data Struct. The data is carried from the task to IssueCredentialRequest to CredentialIssueInput, and ValidateCredentialIssueRequestData checks it before Issue. A bad schema returns Internal; bad caller data returns InvalidArgument.
• Typed output: an optional secret_content_type (field 12). Any non-generic value requires exactly one JWE recipient whose AAD matches the type, checked before Issue (credentials.go:231-239).

The AAD matcher and type-pattern check move unchanged into pkg/crypto, field.ValidateInt64Rules is added, and builder.m becomes an unexported interface.

Coverage:

• I scanned the full PR diff for security and correctness issues.
• I read credential_issue_validation.go in full. I checked the size and count limits on schema and data, rejection of unknown fields, integers being bounded to ±2^53, regex compilation, and constraint semantics.
• I checked that b.m == nil still behaves correctly after the interface change, and that request data is not logged in the task handler.
• I checked the generated pb/* files only for consistency with the .proto changes, not line by line.

Criteria and compatibility: no repo-local criteria loaded, so I applied the base criteria and the SDK-compatibility criteria. The proto and exported Go changes are additive, and an absent content type keeps the legacy generic behavior.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

Resolved prior findings

• Fixed: pkg/connectorbuilder/credentials.go:238 (typed-output rollout not described in the PR). The PR description now documents secret_content_type field 12, the legacy-generic default, and the JWE recipient-context requirement before Issue. The gate is at credentials.go:231-239, and the host contract for request_data is at proto/c1/connector/v2/resource.proto:177-180.

[github-actions]

Superseded — see the current review report for commit 6cf27494f412

General PR Review: IGA-4093: Bind native credential inputs and typed outputs

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: none loaded - base ref highb/secret-payload-contract-bundle is not the default branch main for .claude/skills/ci-review.md.
Review mode: full
View review run

Review Summary

This PR adds two things to native credential issuance:

• Request schema and data: a connector-owned request_schema (descriptor field 11, built from config.Field/Constraint) and a request_data Struct. The data is carried from the task to IssueCredentialRequest to CredentialIssueInput, and ValidateCredentialIssueRequestData checks it before Issue. A bad schema returns Internal; bad caller data returns InvalidArgument.
• Typed output: an optional secret_content_type (field 12). Any non-generic value requires exactly one JWE recipient whose AAD matches the type, checked before Issue (credentials.go:231-239).

The AAD matcher and type-pattern check move unchanged into pkg/crypto, field.ValidateInt64Rules is added, and builder.m becomes an unexported interface.

Coverage:

• I scanned the full PR diff for security and correctness issues.
• I read credential_issue_validation.go in full. I checked the size and count limits on schema and data, rejection of unknown fields, integers being bounded to ±2^53, regex compilation, and constraint semantics.
• I checked that b.m == nil still behaves correctly after the interface change, and that request data is not logged in the task handler.
• I checked the generated pb/* files only for consistency with the .proto changes, not line by line.

Criteria and compatibility: no repo-local criteria loaded, so I applied the base criteria and the SDK-compatibility criteria. The proto and exported Go changes are additive, and an absent content type keeps the legacy generic behavior.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

Resolved prior findings

• Fixed: pkg/connectorbuilder/credentials.go:238 (typed-output rollout not described in the PR). The PR description now documents secret_content_type field 12, the legacy-generic default, and the JWE recipient-context requirement before Issue. The gate is at credentials.go:231-239, and the host contract for request_data is at proto/c1/connector/v2/resource.proto:177-180.

---

Reviewed commit: 7da2bbacafb1

[github-actions]

No blocking issues found — see the full review report

[github-actions]

Superseded — see the current review report for commit 6cf27494f412

General PR Review: IGA-4093: Bind native credential inputs and typed outputs

Blocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0
Criteria: Criteria status: none loaded - base ref highb/secret-payload-contract-bundle is not the default branch main for .claude/skills/ci-review.md.
Review mode: full
View review run

Review Summary

What the change does

• Request inputs: adds request_data (Struct) to IssueCredentialRequest and to the issue-credential task. The task handler forwards it, and IssueCredential validates it against the selected descriptor's schema before Issue.
• Where the schema comes from: either the legacy request_schema (field 11) or the new direct arguments/constraints (fields 13-14). If both are set and disagree, the descriptor is rejected.
• Typed output: either secret_content_type (field 12) or a single return_types field (15) tagged multipass-type-*. This gates delivery on one JWE recipient whose context matches the type, and checks the plaintext and encrypted output field names.
• Metrics: builder.m is now an interface.

Coverage

• I scanned the full PR diff for security and correctness.
• I read credentials.go, credential_issue_fields.go, the schema/data entry points of credential_issue_validation.go, crypto/field_metadata.go, the proto changes and the task bridge.
• I checked the generated pb/* files only for consistency with the .proto changes, not line by line.
• No repo-local criteria loaded (base is not main), so only the base criteria and the SDK-compatibility criteria were applied.

Security Issues

None found. Request schema fields cannot be secret, request data has size and field-count limits, and validation errors do not echo submitted values (tested in credential_issue_fields_test.go).

Correctness Issues

None found. A malformed connector schema is consistently mapped to Internal (credentials.go:231-233, credential_issue_validation.go:121-127,676-678). Bad caller data maps to InvalidArgument.

Suggestions

• New, proto/c1/connector/v2/connector.proto:154-161: the PR description doesn't match the head.
  • Fields 11 and 12 are now deprecated = true, and the new public fields arguments/constraints/return_types (13-15) have no proto comments.
  • The SDK enforces a contract on these fields: exactly one named, required, secret string return field, with the logical type taken from a multipass-type-* tag (credential_issue_fields.go:41-75, pkg/crypto/field_metadata.go:49-55).
  • Neither that contract nor the deprecation is documented in the proto or the PR body, which still presents fields 11/12 as the contract.
  • Confidence: high. Severity: suggestion (documentation/rollout).

Resolved prior findings

• Fixed: pkg/connectorbuilder/credentials.go (typed-output rollout was not described). The PR body now describes secret_content_type field 12, the legacy-generic default and the JWE recipient-context requirement. The gate is at credentials.go:247-257.

Prompt for AI agents

Verify each finding against the current code and only fix it if needed.

## Suggestions

In `proto/c1/connector/v2/connector.proto`:
- Around line 154-161: Fields 11 (request_schema) and 12 (secret_content_type) are now deprecated, and the new fields arguments (13), constraints (14) and return_types (15) have no comments. Add proto comments on 13-15 that state:
  - arguments/constraints replace request_schema, and if both are set they must agree, ignoring content_type and tags;
  - return_types must hold exactly one named, required, secret string Field;
  - its logical delivery type comes from a single `multipass-type-<type>` tag or from the deprecated secret_content_type, and the two must not conflict;
  - a declared output requires exactly one recipient, and a non-generic type requires a JWE recipient whose AAD matches.
  Add a deprecation note on 11/12 pointing to the replacements. Regenerate pb/* and update the PR description to match.

[github-actions]

🟡 Suggestion (confidence: high): This head marks request_schema (11) and secret_content_type (12) as deprecated and adds undocumented arguments/constraints/return_types (13-15). The SDK enforces a contract on them: return_types must be exactly one named, required, secret string field, and the logical type comes from a multipass-type-* tag (credential_issue_fields.go:41-75, crypto/field_metadata.go:49-55). The PR description still presents fields 11/12 as the contract and never mentions 13-15 or the tag convention. Add proto comments for fields 13-15 and update the PR body so host/C1 implementers know which fields to send and read.

[github-actions]

Superseded — see the current review report for commit 0e5341fd7d77

General PR Review: IGA-4093: Bind native credential inputs and typed outputs

Blocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0
Criteria: Criteria status: none loaded - base ref highb/secret-payload-contract-bundle is not the default branch main for .claude/skills/ci-review.md.
Review mode: full
View review run

Review Summary

What the change does

• Request inputs: adds request_data (Struct) to IssueCredentialRequest and to the issue-credential task. The task handler forwards it, and IssueCredential validates it against the selected descriptor's schema before Issue.
• Where the schema comes from: either the legacy request_schema (field 11) or the new direct arguments/constraints (fields 13-14). If both are set and disagree, the descriptor is rejected.
• Typed output: either secret_content_type (field 12) or a single return_types field (15) tagged multipass-type-*. This gates delivery on one JWE recipient whose context matches the type, and checks the plaintext and encrypted output field names.
• Metrics: builder.m is now an interface.

Coverage

• I scanned the full PR diff for security and correctness.
• I read credentials.go, credential_issue_fields.go, the schema/data entry points of credential_issue_validation.go, crypto/field_metadata.go, the proto changes and the task bridge.
• I checked the generated pb/* files only for consistency with the .proto changes, not line by line.
• No repo-local criteria loaded (base is not main), so only the base criteria and the SDK-compatibility criteria were applied.

Security Issues

None found. Request schema fields cannot be secret, request data has size and field-count limits, and validation errors do not echo submitted values (tested in credential_issue_fields_test.go).

Correctness Issues

None found. A malformed connector schema is consistently mapped to Internal (credentials.go:231-233, credential_issue_validation.go:121-127,676-678). Bad caller data maps to InvalidArgument.

Suggestions

• New, proto/c1/connector/v2/connector.proto:154-161: the PR description doesn't match the head.
  • Fields 11 and 12 are now deprecated = true, and the new public fields arguments/constraints/return_types (13-15) have no proto comments.
  • The SDK enforces a contract on these fields: exactly one named, required, secret string return field, with the logical type taken from a multipass-type-* tag (credential_issue_fields.go:41-75, pkg/crypto/field_metadata.go:49-55).
  • Neither that contract nor the deprecation is documented in the proto or the PR body, which still presents fields 11/12 as the contract.
  • Confidence: high. Severity: suggestion (documentation/rollout).

Resolved prior findings

• Fixed: pkg/connectorbuilder/credentials.go (typed-output rollout was not described). The PR body now describes secret_content_type field 12, the legacy-generic default and the JWE recipient-context requirement. The gate is at credentials.go:247-257.

Prompt for AI agents

Verify each finding against the current code and only fix it if needed.

## Suggestions

In `proto/c1/connector/v2/connector.proto`:
- Around line 154-161: Fields 11 (request_schema) and 12 (secret_content_type) are now deprecated, and the new fields arguments (13), constraints (14) and return_types (15) have no comments. Add proto comments on 13-15 that state:
  - arguments/constraints replace request_schema, and if both are set they must agree, ignoring content_type and tags;
  - return_types must hold exactly one named, required, secret string Field;
  - its logical delivery type comes from a single `multipass-type-<type>` tag or from the deprecated secret_content_type, and the two must not conflict;
  - a declared output requires exactly one recipient, and a non-generic type requires a JWE recipient whose AAD matches.
  Add a deprecation note on 11/12 pointing to the replacements. Regenerate pb/* and update the PR description to match.

---

Reviewed commit: 6cf27494f412

[github-actions]

No blocking issues found — see the full review report

[github-actions]

Superseded — see the current review report for commit 0e5341fd7d77

General PR Review: IGA-4093: Bind native credential inputs and typed outputs

Blocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0
Criteria: Criteria status: none loaded - base ref highb/secret-payload-contract-bundle is not the default branch main for .claude/skills/ci-review.md.
Review mode: full
View review run

Review Summary

What the change does:

• Request inputs: adds request_data (Struct) to IssueCredentialRequest and to the issue-credential task. The task handler forwards it, and IssueCredential validates it against the selected descriptor's schema before Issue.
• Where the schema comes from: either the legacy request_schema (field 11) or the new direct arguments/constraints (fields 13-14). If both are set and disagree, the descriptor is rejected.
• Schema errors vs. request errors: a bad connector schema returns codes.Internal; bad request data returns InvalidArgument.
• Typed output: either secret_content_type (field 12) or a single return_types field (15) tagged multipass-type-*. This gates delivery on one JWE recipient whose context matches the type, and checks the plaintext and encrypted output field names.
• Other changes: field.ValidateInt64Rules is added (no narrowing to platform int), and builder.m is now an interface.

Coverage:

• I scanned the full PR diff for security and correctness.
• I read credentials.go, credential_issue_fields.go, all of credential_issue_validation.go, the pkg/crypto helpers, the proto changes and the task bridge.
• Request-data validation is bounded: 64 KiB, 64 fields, 64 items per collection, safe JSON integer range, and RE2 patterns. An unknown request-data field is rejected even when the descriptor has no schema.
• I checked the generated pb/* files only for consistency with the .proto changes, not line by line. I skimmed the test files rather than auditing them.
• No repo-local criteria loaded (base is not main), so only the base criteria and the SDK-compatibility criteria were applied.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

• Prior — still present, proto/c1/connector/v2/connector.proto:152-164: the PR description doesn't match the head.
  • The body presents request_schema (11) and secret_content_type (12) as the contract, but both are now deprecated = true.
  • The new public fields arguments/constraints/return_types (13-15) have no proto comments.
  • The SDK enforces a contract on these fields: exactly one named, required, secret string return field, with the logical type taken from a multipass-type-* tag (credential_issue_fields.go:41-75, pkg/crypto/field_metadata.go:49-55).
  • Confidence: high. This was raised in the previous summary but had no inline thread, so I posted one.

Resolved prior findings

• Fixed: pkg/connectorbuilder/credentials.go (typed-output rollout was not described). The PR body now describes secret_content_type field 12, the legacy-generic default and the JWE recipient-context requirement. The gate is at credentials.go:247-257. The outdated thread can't be auto-resolved (resolution_blocked).

Prompt for AI agents

Verify each finding against the current code and only fix it if needed.

## Suggestions

In `proto/c1/connector/v2/connector.proto`:
- Around line 152-164: request_schema (11) and secret_content_type (12) are marked deprecated, but the new replacement fields arguments (13), constraints (14) and return_types (15) have no comments. Add proto comments that:
  - say arguments/constraints replace request_schema, and if both are set they must match after content_type and tags are ignored;
  - say return_types must hold exactly one named, required, is_secret string Field;
  - say its logical delivery type comes from a single `multipass-type-<type>` tag or from the deprecated secret_content_type, and the two must not conflict;
  - say a non-generic type requires exactly one baton/jwe/v1 recipient with a matching context.
  Also update the IssueCredentialRequest.request_data comment in resource.proto so it refers to arguments/constraints as well as request_schema. Then regenerate pb/* and update the PR description to match.

[github-actions]

🟡 Suggestion: The PR body and the field comments here don't match the head. The body presents request_schema (11) and secret_content_type (12) as the contract, but both are now deprecated = true. The fields that actually replace them, arguments/constraints/return_types (13-15), are public but have no comments. They also carry an SDK-enforced contract (credential_issue_fields.go:41-75): return_types must be exactly one named, required, secret string field; its type comes from a multipass-type-* tag, and it must not conflict with field 12. Please document 13-15 and update the PR description so hosts and connectors know which fields to use. (Confidence: high)

[github-actions]

General PR Review: IGA-4093: Bind native credential inputs and typed outputs

Blocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0
Criteria: Criteria status: none loaded - base ref highb/secret-payload-contract-bundle is not the default branch main for .claude/skills/ci-review.md.
Review mode: full
View review run

Review Summary

What the change does:

• Request inputs: adds request_data (Struct) to IssueCredentialRequest and to the issue-credential task. The task handler forwards it, and IssueCredential validates it against the selected descriptor's schema before Issue.
• Where the schema comes from: either the legacy request_schema (field 11) or the new direct arguments/constraints (fields 13-14). If both are set and disagree, the descriptor is rejected.
• Schema errors vs. request errors: a bad connector schema returns codes.Internal; bad request data returns InvalidArgument.
• Typed output: either secret_content_type (field 12) or a single return_types field (15) tagged multipass-type-*. This gates delivery on one JWE recipient whose context matches the type, and checks the plaintext and encrypted output field names.
• Other changes: field.ValidateInt64Rules is added (no narrowing to platform int), and builder.m is now an interface.

Coverage:

• I scanned the full PR diff for security and correctness.
• I read credentials.go, credential_issue_fields.go, all of credential_issue_validation.go, the pkg/crypto helpers, the proto changes and the task bridge.
• Request-data validation is bounded: 64 KiB, 64 fields, 64 items per collection, safe JSON integer range, and RE2 patterns. An unknown request-data field is rejected even when the descriptor has no schema.
• I checked the generated pb/* files only for consistency with the .proto changes, not line by line. I skimmed the test files rather than auditing them.
• No repo-local criteria loaded (base is not main), so only the base criteria and the SDK-compatibility criteria were applied.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

• Prior — still present, proto/c1/connector/v2/connector.proto:152-164: the PR description doesn't match the head.
  • The body presents request_schema (11) and secret_content_type (12) as the contract, but both are now deprecated = true.
  • The new public fields arguments/constraints/return_types (13-15) have no proto comments.
  • The SDK enforces a contract on these fields: exactly one named, required, secret string return field, with the logical type taken from a multipass-type-* tag (credential_issue_fields.go:41-75, pkg/crypto/field_metadata.go:49-55).
  • Confidence: high. This was raised in the previous summary but had no inline thread, so I posted one.

Resolved prior findings

• Fixed: pkg/connectorbuilder/credentials.go (typed-output rollout was not described). The PR body now describes secret_content_type field 12, the legacy-generic default and the JWE recipient-context requirement. The gate is at credentials.go:247-257. The outdated thread can't be auto-resolved (resolution_blocked).

Prompt for AI agents

Verify each finding against the current code and only fix it if needed.

## Suggestions

In `proto/c1/connector/v2/connector.proto`:
- Around line 152-164: request_schema (11) and secret_content_type (12) are marked deprecated, but the new replacement fields arguments (13), constraints (14) and return_types (15) have no comments. Add proto comments that:
  - say arguments/constraints replace request_schema, and if both are set they must match after content_type and tags are ignored;
  - say return_types must hold exactly one named, required, is_secret string Field;
  - say its logical delivery type comes from a single `multipass-type-<type>` tag or from the deprecated secret_content_type, and the two must not conflict;
  - say a non-generic type requires exactly one baton/jwe/v1 recipient with a matching context.
  Also update the IssueCredentialRequest.request_data comment in resource.proto so it refers to arguments/constraints as well as request_schema. Then regenerate pb/* and update the PR description to match.

---

Reviewed commit: 0e5341fd7d77

[github-actions]

No blocking issues found — see the full review report

[highb]

Squire (claude-opus-5-5): Closing as superseded by #1218 (IGA-5308). The output contract is now one proto field on CredentialIssueOptionDescriptor (c1.config.v1.Field output = 11), which reuses #1213's content_type and tags. Input schemas, typed constructors and the tag-prefix selector are out of scope for now.