Skip to content

test fixes#2570

Merged
akshaydeo merged 1 commit intov1.5.0from
04-08-test_fixes
Apr 8, 2026
Merged

test fixes#2570
akshaydeo merged 1 commit intov1.5.0from
04-08-test_fixes

Conversation

@akshaydeo
Copy link
Copy Markdown
Contributor

@akshaydeo akshaydeo commented Apr 8, 2026

Summary

This PR adds new workspace modules to the Go workspace configuration, fixes a null pointer dereference in the Replicate provider, updates migration table naming for consistency, and adjusts transport configuration schema.

Changes

  • Added ./plugins/prompts and ./cli modules to the Go workspace setup script
  • Fixed potential null pointer dereference in Replicate provider by adding nil check for ReplicateKeyConfig
  • Updated migration table name from gomigrate to migrations in test setup functions and comments to match migrator defaults
  • Added routing_chain_max_depth configuration option to transport schema with default value of 10
  • Removed calendar_aligned field from budget configuration schema

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (Next.js)
  • Docs

How to test

Validate the workspace setup and provider functionality:

# Test Go workspace setup
./.github/workflows/scripts/setup-go-workspace.sh

# Test Replicate provider with nil config
go test ./core/providers/replicate/...

# Test migration functionality
go test ./framework/configstore/...

# Run all tests
go test ./...

Test the new routing chain configuration by setting routing_chain_max_depth in your transport config and verifying it's respected during routing rule evaluation.

Screenshots/Recordings

N/A

Breaking changes

  • Yes
  • No

Related issues

N/A

Security considerations

The routing chain max depth limit helps prevent potential infinite loops or excessive recursion in routing rule evaluation.

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 8, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 99c08ded-8e37-4495-b97e-afcac3e0792c

📥 Commits

Reviewing files that changed from the base of the PR and between 6c5f942 and 679b203.

📒 Files selected for processing (4)
  • .github/workflows/scripts/setup-go-workspace.sh
  • core/providers/replicate/replicate.go
  • framework/configstore/migrations_test.go
  • transports/config.schema.json

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Fixed a potential crash in Replicate provider routing when certain configurations are not set.
  • New Features

    • Added routing_chain_max_depth configuration option to control routing depth limit (default: 10).
  • Removed

    • Removed calendar_aligned option from budget governance settings.

Walkthrough

This pull request includes four heterogeneous changes: extending the Go workspace setup script to include two additional modules (./plugins/prompts and ./cli), adding a nil-safety guard in the Replicate provider's deployment-listing function, updating test database migration table names from gomigrate to migrations, and adjusting the configuration schema by introducing a routing_chain_max_depth limit while removing the calendar_aligned budget field.

Changes

Cohort / File(s) Summary
Go Workspace Initialization
.github/workflows/scripts/setup-go-workspace.sh
Added two modules (./plugins/prompts and ./cli) to the Go workspace during setup.
Replicate Provider Safety
core/providers/replicate/replicate.go
Added nil-safety check to listDeploymentsByKey to prevent dereferencing key.ReplicateKeyConfig when nil, returning empty response instead.
Migration Test Database Configuration
framework/configstore/migrations_test.go
Updated test helpers to use migrations table name instead of gomigrate, aligning with migrator.DefaultOptions.TableName.
Configuration Schema Updates
transports/config.schema.json
Added client.routing_chain_max_depth (integer, minimum 1, default 10) and removed governance.budgets[].calendar_aligned field.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 We hop through workspaces with care,
Adding modules here and there,
Nil-checks keep us safe from falls,
Migrations renamed in our halls,
Schema shifts, routing chains run deep—
All these changes, ours to keep! 🌿

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 04-08-test_fixes

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor Author

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

@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@akshaydeo akshaydeo marked this pull request as ready for review April 8, 2026 16:14
@akshaydeo akshaydeo requested a review from a team as a code owner April 8, 2026 16:14
Copy link
Copy Markdown
Contributor Author

akshaydeo commented Apr 8, 2026

Merge activity

  • Apr 8, 4:14 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Apr 8, 4:15 PM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo akshaydeo merged commit 047d101 into v1.5.0 Apr 8, 2026
10 of 15 checks passed
@akshaydeo akshaydeo deleted the 04-08-test_fixes branch April 8, 2026 16:15
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 8, 2026

Confidence Score: 5/5

Safe to merge; only one P2 style finding in the shell script guard.

All four changes are correct and well-scoped. The replicate nil-check is properly targeted to the one code path (ListModels iterating all keys) where nil is actually possible; the other callers receive pre-validated keys. The remaining finding is a minor robustness inconsistency in the shell script early-exit guard, which does not affect normal usage.

.github/workflows/scripts/setup-go-workspace.sh — redundant and less-robust first early-exit guard.

Vulnerabilities

No security concerns identified.

Important Files Changed

Filename Overview
.github/workflows/scripts/setup-go-workspace.sh Adds ./plugins/prompts and ./cli to the Go workspace; first go.work guard uses a bare return that fails when the script is executed directly instead of sourced.
core/providers/replicate/replicate.go Adds a nil guard for ReplicateKeyConfig in listDeploymentsByKey, which is called by ListModels over all keys (including those without ReplicateKeyConfig). Other methods are safe because they receive pre-validated keys.
framework/configstore/migrations_test.go Updates migration table name from gomigrate to migrations to match migrator defaults; test helpers and SQL DDL are consistent with the production migration code.
transports/config.schema.json Adds routing_chain_max_depth (integer, min 1, default 10) to the transport schema; calendar_aligned remains in the virtual-key section (consistent with governance.go) and appears to have been removed only from the budget-level schema.

Comments Outside Diff (1)

  1. .github/workflows/scripts/setup-go-workspace.sh, line 9 (link)

    P2 Bare return will fail when script is executed directly

    The first early-exit guard uses a plain return (line 9), but the second guard (line 18) correctly uses return 0 2>/dev/null || exit 0. With set -euo pipefail active, if someone runs the script directly (bash setup-go-workspace.sh) and go.work already exists, the bare return exits with code 1 and the error message is printed to stderr. The first guard is also entirely redundant — the second guard (lines 16-19) already covers the same condition more robustly. Consider removing lines 7-10 entirely.

Reviews (1): Last reviewed commit: "test fixes" | Re-trigger Greptile

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants