Skip to content

fix: add mcp_enable_temp_token_auth dynamic column and replace cut with awk for column-order-safe snapshot comparison - #3889

Merged
Pratham-Mishra04 merged 1 commit into
devfrom
05-29-fix_migration_test_fixes
May 29, 2026
Merged

Pratham-Mishra04 merged 1 commit into
devfrom
05-29-fix_migration_test_fixes

Conversation

@Pratham-Mishra04

Copy link
Copy Markdown
Collaborator

Summary

Adds migration test support for the new mcp_enable_temp_token_auth config column and fixes a column-ordering bug in the Postgres snapshot comparison logic that caused false failures when columns were dropped and re-added via ALTER TABLE ADD COLUMN.

Changes

  • Added mcp_enable_temp_token_auth to the dynamic column append logic for both Postgres and SQLite migration tests, setting it to false/0 when the column exists.
  • Replaced cut -f with awk in compare_postgres_snapshots for extracting columns during before/after snapshot comparison. cut always emits fields in ascending positional order regardless of the column spec, which causes row misalignment when a column's physical position in the after-schema differs from its logical position (e.g. a column dropped and re-added ends up at the end of the table). awk respects the specified field order, producing correct alignment.

Type of change

  • Bug fix
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

Run the migration test suite and verify that snapshot comparisons pass correctly, including for tables where columns have been dropped and re-added.

Breaking changes

  • No

Security considerations

None.

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

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@Pratham-Mishra04, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 9 minutes and 59 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c0810c1f-84d6-4d29-9f5a-fff316fa783e

📥 Commits

Reviewing files that changed from the base of the PR and between e09c643 and 04310b2.

📒 Files selected for processing (1)
  • .github/workflows/scripts/run-migration-tests.sh
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 05-29-fix_migration_test_fixes

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

Pratham-Mishra04 commented May 29, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile-apps

greptile-apps Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

Changes are isolated to CI/migration test infrastructure and carry no production risk.

Both changes are straightforward: the awk rewrite is a correct fix for a real cut ordering bug, the mcp_enable_temp_token_auth entries follow the established pattern exactly, and the psql snapshot format (-t -A -F'|') is pipe-separated without padding so awk -F'|' field access is sound. No logic regressions or edge-case holes found.

No files require special attention.

Important Files Changed

Filename Overview
.github/workflows/scripts/run-migration-tests.sh Adds mcp_enable_temp_token_auth dynamic column to Postgres and SQLite snapshot normalization, and replaces cut -f with awk in compare_postgres_snapshots to preserve column-index ordering when before/after schemas differ in physical column positions.

Reviews (2): Last reviewed commit: "fix: migration test fixes" | Re-trigger Greptile

Comment thread .github/workflows/scripts/run-migration-tests.sh
Comment thread .github/workflows/scripts/run-migration-tests.sh

Pratham-Mishra04 commented May 29, 2026

Copy link
Copy Markdown
Collaborator Author

Merge activity

  • May 29, 2:02 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • May 29, 2:04 PM UTC: Graphite rebased this pull request as part of a merge.
  • May 29, 2:05 PM UTC: @Pratham-Mishra04 merged this pull request with Graphite.

@Pratham-Mishra04
Pratham-Mishra04 changed the base branch from 05-28-refactor_extract_out_ismodelrequired_method_form_resolver to graphite-base/3889 May 29, 2026 14:02
@Pratham-Mishra04
Pratham-Mishra04 changed the base branch from graphite-base/3889 to dev May 29, 2026 14:02
@Pratham-Mishra04
Pratham-Mishra04 force-pushed the 05-29-fix_migration_test_fixes branch from 2946c13 to 04310b2 Compare May 29, 2026 14:03
@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.

@Pratham-Mishra04
Pratham-Mishra04 merged commit 986161d into dev May 29, 2026
14 of 15 checks passed
@Pratham-Mishra04
Pratham-Mishra04 deleted the 05-29-fix_migration_test_fixes branch May 29, 2026 14:05
akhsaul pushed a commit to akhsaul/bifrost that referenced this pull request Aug 27, 2026
…` with `awk` for column-order-safe snapshot comparison (maximhq#3889)

## Summary

Adds migration test support for the new `mcp_enable_temp_token_auth` config column and fixes a column-ordering bug in the Postgres snapshot comparison logic that caused false failures when columns were dropped and re-added via `ALTER TABLE ADD COLUMN`.

## Changes

- Added `mcp_enable_temp_token_auth` to the dynamic column append logic for both Postgres and SQLite migration tests, setting it to `false`/`0` when the column exists.
- Replaced `cut -f` with `awk` in `compare_postgres_snapshots` for extracting columns during before/after snapshot comparison. `cut` always emits fields in ascending positional order regardless of the column spec, which causes row misalignment when a column's physical position in the after-schema differs from its logical position (e.g. a column dropped and re-added ends up at the end of the table). `awk` respects the specified field order, producing correct alignment.

## Type of change

- [x] Bug fix
- [x] Chore/CI

## Affected areas

- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

Run the migration test suite and verify that snapshot comparisons pass correctly, including for tables where columns have been dropped and re-added.

## Breaking changes

- [x] No

## Security considerations

None.

## 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
occcat pushed a commit to occcat/bifrost that referenced this pull request Sep 2, 2026
…` with `awk` for column-order-safe snapshot comparison (maximhq#3889)

## Summary

Adds migration test support for the new `mcp_enable_temp_token_auth` config column and fixes a column-ordering bug in the Postgres snapshot comparison logic that caused false failures when columns were dropped and re-added via `ALTER TABLE ADD COLUMN`.

## Changes

- Added `mcp_enable_temp_token_auth` to the dynamic column append logic for both Postgres and SQLite migration tests, setting it to `false`/`0` when the column exists.
- Replaced `cut -f` with `awk` in `compare_postgres_snapshots` for extracting columns during before/after snapshot comparison. `cut` always emits fields in ascending positional order regardless of the column spec, which causes row misalignment when a column's physical position in the after-schema differs from its logical position (e.g. a column dropped and re-added ends up at the end of the table). `awk` respects the specified field order, producing correct alignment.

## Type of change

- [x] Bug fix
- [x] Chore/CI

## Affected areas

- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

Run the migration test suite and verify that snapshot comparisons pass correctly, including for tables where columns have been dropped and re-added.

## Breaking changes

- [x] No

## Security considerations

None.

## 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
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