feat(spanner): add native read-only transactional support

[anubhav756]

Overview

This PR implements read-only transactional support for Cloud Spanner.

Changes

• Updates the core execution logic to route queries through spanner.Single().Query() when connected in read-only mode
  • This guarantees the execution is within absolute read-only transaction boundaries.
• Includes a vulnerability integration test to prove that write queries are fundamentally rejected at the client/API layer when operating under this configuration, preventing agent bypass.

[gemini-code-assist]

Code Review

This pull request introduces a read-only mode for the Spanner data source and its tools, adding configuration options, documentation, and validation. The review feedback highlights two key issues: first, a bug in the conflict validation logic where omitting the legacy readOnly field triggers a false conflict with readOnlyHint: true; second, the need to fail fast with a clear error message when write operations are attempted on a read-only source to prevent Spanner API errors.

[anubhav756]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a read-only mode configuration (SPANNER_READONLY) for the Spanner data source and its tools, ensuring write operations are blocked and queries are executed on read-only endpoints when enabled. Feedback on the changes includes simplifying a redundant boolean condition in the query execution path and correcting the newly added unit tests, which currently lack assertions on the overridden read-only configuration and contain dead code.

[gemini-code-assist]

The condition readOnly || s.ReadOnly is redundant and can be simplified to just readOnly. Since any case where !readOnly && s.ReadOnly is true is already handled and rejected with an error on line 182, we are guaranteed that if s.ReadOnly is true, readOnly must also be true. Simplifying this condition improves readability and avoids potential confusion for future maintainers.

Suggested change

	if readOnly || s.ReadOnly {
	if readOnly {

[gemini-code-assist]

The test TestInitialize_ReadOnlyValidation does not actually assert that the ReadOnly configuration is correctly overridden by the annotations. Since all test cases have wantErr: false, the test would pass even if the override logic was completely missing. Additionally, the test contains dead code (wantErr and errContains fields) and an unidiomatic manual substring search loop.

We should rewrite the test to assert the expected ReadOnly value on the initialized tool using ToConfig() and clean up the unused fields.

func TestInitialize_ReadOnlyValidation(t *testing.T) {
	ctx := context.Background()

	ptr := func(b bool) *bool { return &b }

	tcs := []struct {
		desc         string
		cfg          spannerexecutesql.Config
		wantReadOnly bool
	}{
		{
			desc: "no conflict - both true",
			cfg: spannerexecutesql.Config{
				ConfigBase:  tools.ConfigBase{Name: "test-tool", Description: "desc"},
				ReadOnly:    true,
				Annotations: &tools.ToolAnnotations{ReadOnlyHint: ptr(true)},
			},
			wantReadOnly: true,
		},
		{
			desc: "no conflict - both false",
			cfg: spannerexecutesql.Config{
				ConfigBase:  tools.ConfigBase{Name: "test-tool", Description: "desc"},
				ReadOnly:    false,
				Annotations: &tools.ToolAnnotations{ReadOnlyHint: ptr(false)},
			},
			wantReadOnly: false,
		},
		{
			desc: "no conflict - readOnlyHint nil",
			cfg: spannerexecutesql.Config{
				ConfigBase:  tools.ConfigBase{Name: "test-tool", Description: "desc"},
				ReadOnly:    true,
				Annotations: &tools.ToolAnnotations{ReadOnlyHint: nil},
			},
			wantReadOnly: true,
		},
		{
			desc: "precedence - readOnly false, readOnlyHint true overrides to true",
			cfg: spannerexecutesql.Config{
				ConfigBase:  tools.ConfigBase{Name: "test-tool", Description: "desc"},
				ReadOnly:    false,
				Annotations: &tools.ToolAnnotations{ReadOnlyHint: ptr(true)},
			},
			wantReadOnly: true,
		},
		{
			desc: "precedence - readOnly true, readOnlyHint false overrides to false",
			cfg: spannerexecutesql.Config{
				ConfigBase:  tools.ConfigBase{Name: "test-tool", Description: "desc"},
				ReadOnly:    true,
				Annotations: &tools.ToolAnnotations{ReadOnlyHint: ptr(false)},
			},
			wantReadOnly: false,
		},
	}

	for _, tc := range tcs {
		t.Run(tc.desc, func(t *testing.T) {
			tool, err := tc.cfg.Initialize(ctx)
			if err != nil {
				t.Fatalf("unexpected error: %v", err)
			}
			gotCfg, ok := tool.ToConfig().(spannerexecutesql.Config)
			if !ok {
				t.Fatalf("expected Config type, got %T", tool.ToConfig())
			}
			if gotCfg.ReadOnly != tc.wantReadOnly {
				t.Errorf("expected ReadOnly to be %t, got %t", tc.wantReadOnly, gotCfg.ReadOnly)
			}
		})
	}
}

References

1. For mock test tools, kebab-case naming is acceptable to maintain consistency with existing legacy integration tests, even if the general style guide recommends snake_case.