[Schema Consistency] Runtime Type Behavior Inconsistencies: engine.version Bug & Type Coercion Gaps #5708
Closed
Replies: 2 comments 1 reply
-
|
/speckit plan |
Beta Was this translation helpful? Give feedback.
1 reply
-
|
⚓ Avast! This discussion be marked as outdated by Schema Consistency Checker. |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
This analysis focused on runtime type behavior vs schema type definitions, examining where the Go compiler accepts different types than the JSON Schema declares. This reveals gaps where external validation tools would fail but the runtime would succeed (or vice versa).
Executive Summary
Critical Issues Requiring Action
1. Engine.version Field: Schema Promises Number Support, Code Only Accepts String
Severity: 🔴 CRITICAL
The schema explicitly documents that engine.version accepts both string and number types, with examples including numeric values:
However, the runtime code only handles strings:
Impact:
version: 20orversion: 3.11(as documented in schema examples) will have their version field silently ignoredRecommendation: Add numeric type handling:
2. Port Field: Accepts float64 But Schema Says Integer Only
Severity: 🟡 MODERATE
Schema declares port as integer type:
But runtime accepts float64 with silent truncation:
Impact:
port: 8080.5but runtime accepts itRecommendation: Either:
"type": ["integer", "number"]with note about truncation3. Max Fields: parseIntValue() Does NOT Support String Conversion
Severity: 🟡 MODERATE
Schema declares max fields as integer:
The
parseIntValue()function used for safe-outputs max fields does NOT accept string-to-int conversion:Contrast with ConvertToInt() which DOES accept strings:
Impact:
max: "5"(string) would be rejected, defaulting to 1Recommendation: Either:
parseIntValue()for consistency withConvertToInt()4. Max-Patch-Size: Float Truncation Only Logged, Not User-Visible
Severity: 🟢 LOW
Runtime accepts float64 for max-patch-size with truncation warning logged to internal logs:
Impact:
max-patch-size: 1024.5Recommendation:
Detailed Analysis
Type Coercion Comparison Matrix
Key Patterns Discovered
Pattern 1: Two Different Type Conversion Strategies
The codebase uses two different helper functions for integer parsing:
parseIntValue() - Strict, No String Support
pkg/workflow/map_helpers.go:9-27ConvertToInt() - Permissive, String Support
pkg/workflow/metrics.go:243-262This creates inconsistent type handling across the codebase.
Pattern 2: Schema Promises vs Runtime Reality
Pattern 3: Silent Failure Modes
Silent Ignore: engine.version numeric values ignored, no error
Silent Truncate: port 8080.5 → 8080, no user warning
Silent Default: max "5" (string) → 1 (default), no error message
Files Analyzed
Schema Files
pkg/parser/schemas/main_workflow_schema.json- type definitions for all fieldsType Conversion Code
pkg/workflow/map_helpers.go- parseIntValue() implementationpkg/workflow/metrics.go- ConvertToInt()/ConvertToFloat() implementationpkg/workflow/engine.go:66-70- version field parsingpkg/workflow/tools_types.go:650-654- port field parsingpkg/workflow/safe_output_config.go:13-16- max field parsingpkg/workflow/safe_outputs.go:284-294- max-patch-size parsingTest Coverage
pkg/workflow/map_helpers_test.go- parseIntValue testspkg/workflow/safe_outputs_max_test.go- max field testsPositive Findings
✅ Comprehensive Type Handling: Code handles int, int64, uint64, float64 for numeric fields
✅ Truncation Warnings: Float-to-int conversions log warnings (though not user-visible)
✅ Consistent Pattern:
parseIntValue()used consistently across safe-outputs max fields✅ Good Defensive Coding: Type switches prevent crashes from unexpected types
✅ Test Coverage: Truncation scenarios tested in
map_helpers_test.goExternal Validation Impact
These type mismatches create external tooling problems:
JSON Schema Validators (jsonschema, ajv, etc.)
port: 8080.5(float)engine.version: 20(number)IDE/Editor Schema Integration
CI/CD Pre-Commit Validation
Recommendations
Priority 1: Fix engine.version Bug (Critical)
Problem: Schema promises number support with examples, code ignores it
Solution:
Files to update:
pkg/workflow/engine.go(add numeric handling)pkg/workflow/engine_test.go(add tests for numeric versions)Priority 2: Document Type Coercion in Schema
Problem: Schema doesn't document float truncation or string rejection
Solution: Add
$commentfields and update descriptions:{ "port": { "type": ["integer", "number"], "$comment": "Runtime truncates decimal values to integer (8080.5 → 8080)", "description": "Port number for MCP gateway (decimal values truncated)" }, "max": { "type": "integer", "$comment": "Runtime accepts integer or number (floats truncated). Strings NOT supported.", "description": "Maximum number of items (default: 1). Numeric values only." } }Files to update:
pkg/parser/schemas/main_workflow_schema.jsonPriority 3: Unify Type Conversion Strategy
Problem: parseIntValue() and ConvertToInt() have different string handling
Option A: Add String Support to parseIntValue()
Option B: Document String Rejection Explicitly
Recommendation: Option A for consistency, but document behavior either way
Priority 4: Surface Truncation Warnings to Users
Problem: Float truncation warnings only in internal logs
Solution: Add compiler warnings for data-loss scenarios:
Files to update:
pkg/workflow/safe_outputs.gopkg/workflow/safe_output_config.gopkg/workflow/compiler_types.go(add Warning type if needed)Strategy Performance
Strategy Evaluation
Why This Strategy Works
Comparison to Previous Run (2025-11-19)
Previous findings:
New findings (this run):
Conclusion: Strategy continues to find new issues. Very High effectiveness rating justified.
Recommendation
Reuse Frequency: Every 4-6 analyses
Best Paired With:
When to Use:
Next Steps
Immediate Actions (This Week)
Short Term (This Sprint)
Long Term (Next Quarter)
Methodology
Strategy Repository
Successfully updated strategy cache at
/tmp/gh-aw/cache-memory/strategies.json:Analysis Quality: ⭐⭐⭐⭐⭐ Very High
Beta Was this translation helpful? Give feedback.
All reactions