feat(mcp/connect): catch slug-length 50-char trap before Connect 500s#347
Merged
Conversation
Open
3 tasks
Connect's LearnModule.slug / DeliverUnit.slug are SlugField() with the Django default max_length=50. Nova's compile_app derives slugs as module_<index>_<slugified-name>; module names ~40 chars overflow and trigger Postgres `DataError: value too long for type character varying(50)` inside Connect's sync_learn_modules_and_deliver_units, which falls through program/api/views.py:102's narrow except clause and surfaces as HTTP 500 with empty body from connect_create_opportunity. Same shape as the 2026-05-12 short_description 50-char trap but at a different boundary — the slug is server-extracted from CCZ XML, not sent through any serializer. The 2026-05-12 generalized serializer-vs- model length probe (still pending) would NOT have caught this. Four-layer defense-in-depth: - mcp/connect/backends/commcare.ts: simulateConnectSync projection now exposes slug_length_limit (constant 50), max_slug_length, and per-type oversized_slugs[]. New exported SLUG_LENGTH_LIMIT constant. - skills/app-release/SKILL.md § Step 6: gate extended to require every oversized_slugs.* array empty; [BLOCKER] brief lists each offender. Structural backstop — catches even operator-driven manual Nova edits. - skills/pdd-to-learn-app + pdd-to-deliver-app SKILL.md: new REQUIRED clause in the architect brief template — module/deliver-unit names must be ≤ 40 chars. Includes removal criterion tied to upstream fix. - docs/learnings/2026-05-17-connect-slug-length-50-char-trap.md + registry update: documents the bug, the prior-framing refutations, the Sentry proof, and the upstream Connect fix needed. 5 new vitest cases. All 13 tests pass. Reproducer: leep-paint-collection/20260517-1515 Phase 4 — M6 name "Stage 2: Sample Preparation, Drying, Bagging, Shipment" → slug module_6_stage_2_sample_prep_drying_bagging_shipment (52 chars). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
a1db7ac to
98abab9
Compare
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the
LearnModule.slug/DeliverUnit.slug50-char trap that 500'd Phase 4 ofleep-paint-collection/20260517-1515opaquely. Connect's slug columns areSlugField()with the Django defaultmax_length=50; Nova'scompile_appderives slugs asmodule_<index>_<slugified-name>; module names ≥ ~40 chars overflow. PostgresDataError: value too long for type character varying(50)raised insidesync_learn_modules_and_deliver_unitsfalls throughprogram/api/views.py:102's narrowexceptand surfaces as HTTP 500 with empty body fromconnect_create_opportunity.Same shape as the 2026-05-12
short_description50-char trap, different boundary (slug is server-extracted from CCZ XML, not sent through any serializer). The 2026-05-12 generalized serializer-vs-model length probe (still pending) would NOT have caught this.Four-layer fix (defense-in-depth)
mcp/connect/backends/commcare.ts—simulateConnectSyncprojection now exposesslug_length_limit: 50,max_slug_length, and per-typeoversized_slugs[]. New exportedSLUG_LENGTH_LIMITconstant for lock-step bumping when the upstream column widens.skills/app-release/SKILL.md§ Step 6 — release-time gate extended fromcollision_count === 0 && per-type > 0to ALSO require everyoversized_slugs.*empty.[BLOCKER]brief lists each offender as<type>: <slug> (<length> chars, in <first_seen_in>). Structural backstop — catches even operator-driven manual Nova edits.skills/pdd-to-learn-app/SKILL.md+skills/pdd-to-deliver-app/SKILL.md— new REQUIRED clause in the architect brief template: module / deliver-unit names must be ≤ 40 chars (Nova prefix + slugify + Connect column limit). Includes removal criterion tied to the upstream commcare-connect fix.docs/learnings/2026-05-17-connect-slug-length-50-char-trap.md+ registry update — documents the bug, the prior-framing refutations (CCZ-marker over-strip andtime_estimate: NULLwere both wrong before Sentry pinned it), the Sentry proof, and the upstream Connect fix needed (SlugField(max_length=255)+ migration).docs/learnings/2026-05-12-boundary-probe-registry.mdupdated with the new shipped probe + annotation on the still-pending generalized probe explaining why it'd miss this class.Tests
5 new vitest cases in
test/mcp/connect/unit/connect-sync-projection.test.ts(boundary projection — every projection exposes the new fields; learn_module + deliver_unit slugs > 50 flagged; empty projection sensible defaults; 50-char slugs do NOT trigger). All 13 tests pass.Reproducer
leep-paint-collectionrun20260517-1515Phase 4. Module 6 name"Stage 2: Sample Preparation, Drying, Bagging, Shipment"produces slugmodule_6_stage_2_sample_prep_drying_bagging_shipment(52 chars). Pre-fix: opaque HTTP 500. Post-fix:app-releaseStep 6 halts with a[BLOCKER]naming the offender + remediation; the architect brief constraint also prevents the long name being emitted in the first place.Recovery for the affected run
Rename Nova module 6 to a shorter active title (e.g.
"Stage 2: Sample Prep + Shipment"→ slugmodule_6_stage_2_sample_prep_shipment= 38 chars), re-build + re-release Learn, resume/ace:run leep-paint-collection/20260517-1515.Upstream follow-up
Separate Connect PR will bump
LearnModule.slug+DeliverUnit.slugtoSlugField(max_length=255)+ migration. Also worth proposing: extendprogram/api/views.py:102'sexceptclause to catchDataError/IntegrityErrorand return HTTP 400 with the offending column name — converts every future column-width trap (any field, any path) from "opaque 500" to "actionable 400."Test plan
npx vitest run test/mcp/connect/unit/connect-sync-projection.test.ts— 13 pass/ace:update, re-run/ace:run leep-paint-collection/20260517-1515with a shortened M6 name and confirm Phase 4 proceeds.app-releaseStep 6 with the new[BLOCKER]brief instead of leaking to Connect.🤖 Generated with Claude Code