Skip to content

fix: cap Session.Name to 255 chars to fix btree index on upgrade - #1601

Merged
chocobar merged 5 commits into
mainfrom
fix/session-name-index-size
Feb 11, 2026
Merged

fix: cap Session.Name to 255 chars to fix btree index on upgrade#1601
chocobar merged 5 commits into
mainfrom
fix/session-name-index-size

Conversation

@chocobar

@chocobar chocobar commented Feb 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Session.Name was unbounded text and some production sessions had AI-generated names up to 7654 chars (full prompts, <think> reasoning). Adding the search index in 2.6.0 fails because multi-byte (CJK) characters push the byte size over PostgreSQL's 2704-byte B-tree limit.
  • Adds size:255 constraint to Session.Name so GORM creates varchar(255) instead of text
  • Adds a pre-migration UPDATE to truncate existing oversized names before AutoMigrate creates the index

Test plan

  • Verified fix on production (app.helix.ml) — API starts cleanly after truncation
  • Verify go build ./... passes in CI
  • Confirm existing session name generation produces names under 255 chars

🤖 Generated with Claude Code

chocobar and others added 2 commits February 9, 2026 10:35
Session.Name was unbounded text, and some production sessions had
AI-generated names up to 7654 chars (full prompts, <think> tags).
Adding the search index in 2.6.0 fails because multi-byte characters
push the byte size over PostgreSQL's 2704-byte btree limit.

Adds a pre-migration truncation and size:255 constraint.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@chocobar

Copy link
Copy Markdown
Collaborator Author

Code Review

Found two bugs and two improvement areas:

Bugs (should fix before merging)

  1. Silently swallowed error — The Exec() return value is discarded. If the UPDATE fails, AutoMigrate will still try to create the index on oversized data, hitting the same error this PR tries to fix.

    // Current (broken) — error ignored
    s.gdb.WithContext(context.Background()).Exec("UPDATE ...")
    
    // Should be:
    if err := s.gdb.WithContext(...).Exec("UPDATE ...").Error; err != nil {
        return fmt.Errorf("failed to truncate oversized session names: %w", err)
    }
  2. WHERE condition mismatch — The WHERE uses OCTET_LENGTH(name) > 2704 (bytes) but truncates to LEFT(name, 255) (characters). Names between 256-2704 bytes are left untouched. GORM's AutoMigrate will then issue ALTER COLUMN name TYPE varchar(255), which PostgreSQL will reject if any row exceeds 255 characters. The condition should be LENGTH(name) > 255.

Improvements (should consider)

  1. No input validation added — Several code paths write to Session.Name without length checks (LLM-generated names, user input, Zed thread titles). After this migration, any of those paths could produce value too long for type character varying(255) errors at runtime. The migration fixes existing data but doesn't prevent recurrence.

  2. Runs on every startup — The UPDATE executes on every API boot. It's a no-op scan when no rows match, but it would be cleaner as a numbered migration that runs once.

Verdict

Not safe to merge as-is. The WHERE condition bug (#2) means the migration will likely fail on the exact databases it's trying to fix — names between 256 chars and 2704 bytes won't be truncated, causing the ALTER COLUMN to error out. Combined with the swallowed error (#1), this could fail silently or loudly depending on the data.

chocobar and others added 3 commits February 11, 2026 15:58
…ation

- Fix silently swallowed Exec() error in migration (bug #1)
- Fix WHERE condition: LENGTH(name) > 255 instead of OCTET_LENGTH > 2704 (bug #2)
- Add Go-level name truncation in CreateSession, UpdateSession,
  UpdateSessionMeta, and UpdateSessionName to prevent cryptic GORM errors
- Add 6 unit tests covering truncation for ASCII, multibyte (CJK), and
  boundary cases across all session name write paths

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The UPDATE sessions query fails when the sessions table doesn't exist
yet (fresh database in CI). Guard with HasTable check since AutoMigrate
creates the table after this step.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@chocobar
chocobar merged commit f4c720a into main Feb 11, 2026
9 checks passed
@chocobar
chocobar deleted the fix/session-name-index-size branch February 11, 2026 23:00
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.

1 participant