feat: record user agent in AI Bridge interceptions

[pawbana]

Depends on: coder/aibridge#158

[pawbana]

• feat: add --client flag to list interceptions command #21951
• feat: record user agent in AI Bridge interceptions #21769 👈 (View in Graphite)
• feat: add client column to aibridge_interceptions table #21839
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[coder-tasks]

Code Review

Reviewed the implementation of user agent recording in AI Bridge interceptions. The changes look solid overall - the dual approach of storing both the parsed Client field and the raw UserAgent in metadata provides good flexibility for analytics and debugging.

Summary

✅ Strengths:

• Comprehensive test coverage with both "all optional values set" and "no optional values set" test cases
• Proper handling of nullable fields using sql.NullString
• Consistent API schema updates across swagger, codersdk, and TypeScript types
• Good separation between structured Client field and raw UserAgent metadata

⚠️ Issues Found: 2 minor issues

Issues

1. Inconsistent user agent handling in test - The test expects "unknown" as the client value but doesn't verify the actual user agent parsing logic
2. Missing nil check in translator - The proto translator doesn't validate that req.UserAgent is non-empty before adding to metadata

Questions

• The PR depends on coder/aibridge#158 - has that been merged? The aibridge.InterceptionRecord struct needs to have Client and UserAgent fields for this to compile.
• What's the parsing logic that converts UserAgent → Client? Is that handled in the aibridge dependency or elsewhere in this codebase?

[coder-tasks]

The test expects "unknown" as the client value, but the test setup sends "userAgent123" as the User-Agent header. This suggests there's user agent parsing logic that should be tested more explicitly.

Consider:

1. Testing with a real user agent string that would parse to a known client (e.g., "claude-code/1.0.0")
2. Adding a test case that verifies the parsing logic works correctly
3. Documenting what the expected format is and what "unknown" means

This would make the test more meaningful and verify the actual user agent parsing behavior.

[coder-tasks]

The code overwrites the metadata key if it already exists with a warning, but this only happens if in.UserAgent != "". Consider being consistent:

Either:

1. Always check for key conflicts (even with empty user agent), or
2. Skip the entire block if user agent is empty

Current code has a gap: if UserAgent is empty, the existing metadata key would be preserved silently.

Suggested change

	if in.UserAgent != "" {
	if _, ok := metadata[MetadataUserAgentKey]; ok {
	s.logger.Warn(ctx, fmt.Sprintf("interception metadata contains user agent key, will be overwritten"))
	}
	metadata[MetadataUserAgentKey] = in.UserAgent
	}
	if in.UserAgent != "" {
	if _, ok := metadata[MetadataUserAgentKey]; ok {
	s.logger.Warn(ctx, "interception metadata contains user agent key, will be overwritten")
	}
	metadata[MetadataUserAgentKey] = in.UserAgent
	}

This change makes the conditional more explicit and groups the warning with the overwrite operation.

[dannykopping]

LGTM!

[dannykopping]

aib will be meaningless to most folks.

Suggested change

	MetadataUserAgentKey = "aib-request-user-agent"
	MetadataUserAgentKey = "request_user_agent"

[dannykopping]

You could just log this as its own key in the structured log output I think.

[pawbana]

I wanted to keep the same schema the same between log and DB.
I could add it as separate field and even store it in separate column instead of keeping it as part of metadata column.

[dannykopping]

Thanks for adding these 👍

[ssncferreira]

This is related to my other comment, but this contains the User-Agent header, which can be something like copilot/0.0.403 (client/cli linux v24.11.1), which is more relevant as metadata. Couldn't we just add it directly to actor.Metadata in aibridge? That would let us skip the new UserAgent field in InterceptionRecord, the proto changes, and the server-side metadata injection here entirely.

[pawbana]

In case AI Bridge is running outside of coder I'd rather keep current setup.

[pawbana]

Merged in: #21839