SPDD 2026-05-24: Strengthen MCP Scripts norms, expand ET sync notes, add forecast/fuzzy-schedule tests#34479
Merged
Conversation
9 tasks
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
… fuzzy schedule E2E Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Add sync notes section to effective tokens specification
SPDD 2026-05-24: Strengthen MCP Scripts norms, expand ET sync notes, add forecast/fuzzy-schedule tests
May 24, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens several specification “MUST/MUST NOT” norms (MCP Scripts + Effective Tokens), updates Forecast sync notes, fixes a Monte Carlo edge case in the forecast implementation (non-finite λ), and adds tests for forecast Monte Carlo and fuzzy schedule end-to-end behavior. It also updates a generated workflow lockfile.
Changes:
- Guard
runMonteCarloagainst NaN/±Inf λ to prevent hangs/overflow, and add targeted Monte Carlo tests. - Add/extend CLI tests to cover fuzzy schedule scattering through the schedule calendar pipeline.
- Strengthen/expand specs: MCP Scripts isolation + input-size norms, Effective Tokens extension constraints + implementation mapping table, and Forecast sync follow-ups.
Show a summary per file
| File | Description |
|---|---|
| pkg/cli/forecast_test.go | Adds a new λ consistency test for forecast output/MC integration (currently needs formatting + stronger linkage to production path). |
| pkg/cli/forecast_montecarlo.go | Adds NaN/Inf guard to Monte Carlo λ handling. |
| pkg/cli/forecast_montecarlo_test.go | Adds tests for non-finite and zero/degenerate λ behavior. |
| pkg/cli/compile_schedule_calendar_test.go | Adds an end-to-end fuzzy schedule → calendar pipeline test (currently needs gofmt + minor doc/assertion alignment). |
| docs/src/content/docs/reference/mcp-scripts-specification.md | Strengthens JavaScript sandboxing and string input length requirements with new normative IDs. |
| docs/src/content/docs/reference/forecast-specification.md | Marks some follow-ups resolved and links remaining items to tracking issues. |
| docs/src/content/docs/reference/effective-tokens-specification.md | Adds ET extension “MUST NOT” constraints and a spec→implementation mapping + sync checklist. |
| .github/workflows/daily-malicious-code-scan.lock.yml | Regenerates the lock workflow with updated setup/actions and metadata. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (4)
pkg/cli/forecast_test.go:132
- The new test body isn’t gofmt-formatted (missing indentation inside the function). This makes the file inconsistent with the rest of the Go codebase and may fail formatting checks if CI enforces gofmt. Please run gofmt on this file (or reindent the block).
func TestObservedRunsPerPeriodConsistency(t *testing.T) {
// Reproduce the λ calculation from forecastWorkflow.
const (
historyDays = 30
sampledRuns = 15
pkg/cli/forecast_test.go:163
- The assertion here compares result.ObservedRunsPerPeriod to observedRunsPerPeriod, but both values are set from the same local calculation in this test. This doesn’t verify that the real forecastWorkflow pipeline uses the same λ for JSON output and Monte Carlo execution. If the goal is to lock R-MC-002 down, the test needs to observe λ coming from the production computation path (or explicitly validate the runMonteCarlo call site in forecastWorkflow).
// runMonteCarlo uses result.ObservedRunsPerPeriod as λ — the same field that
// appears in JSON output. Verify both the field value and the simulation are
// consistent (non-nil, same λ).
rng := rand.New(rand.NewSource(99)) //nolint:gosec
mc := runMonteCarlo(etObs, successCount, result.ObservedRunsPerPeriod, rng)
require.NotNil(t, mc, "runMonteCarlo must return non-nil for positive ObservedRunsPerPeriod")
// The field exposed in JSON output must equal what was used for MC.
assert.Equal(t, observedRunsPerPeriod, result.ObservedRunsPerPeriod,
"ObservedRunsPerPeriod JSON field must equal the λ passed to runMonteCarlo")
pkg/cli/compile_schedule_calendar_test.go:328
- The new end-to-end test block isn’t gofmt-formatted (missing indentation throughout the function). Please run gofmt on this file so the formatting is consistent with the rest of the Go codebase and to avoid potential formatting-check failures.
func TestFuzzyScheduleEndToEnd(t *testing.T) {
fuzzyExpressions := []struct {
fuzzyCron string
workflowID string
expectedHours int // how many distinct hour values we expect (1 for DAILY patterns)
}{
{"FUZZY:DAILY * * *", "ci-doctor", 1},
{"FUZZY:DAILY_WEEKDAYS * * *", "daily-planner", 1},
{"FUZZY:DAILY_AROUND:14:0 * * *", "weekly-audit", 1},
}
pkg/cli/compile_schedule_calendar_test.go:374
- This test redirects os.Stderr via os.Pipe(), but it never closes the read end (r). Please ensure both ends of the pipe are closed (e.g., defer r.Close() after creation) to avoid leaking file descriptors across the test suite.
// Step 4: displayScheduleCalendar should produce output referencing the hour.
oldStderr := os.Stderr
r, w, pipeErr := os.Pipe()
require.NoError(t, pipeErr)
os.Stderr = w
- Files reviewed: 8/8 changed files
- Comments generated: 4
Comment on lines
+119
to
+127
| // TestObservedRunsPerPeriodConsistency verifies that the λ value stored in the | ||
| // JSON-serialisable ForecastWorkflowResult.ObservedRunsPerPeriod field is the same | ||
| // value that would be passed to runMonteCarlo (R-MC-002). | ||
| // | ||
| // This is a structural test: it constructs a result whose ObservedRunsPerPeriod is | ||
| // set by the same arithmetic used in forecastWorkflow, then calls runMonteCarlo with | ||
| // that field directly and asserts the simulation produces sensible output — confirming | ||
| // that no intermediate recalculation or mutation of λ occurs between JSON output and | ||
| // Monte Carlo execution. |
Comment on lines
92
to
97
| // | ||
| // Returns nil when etObservations is empty or observedRunsPerPeriod ≤ 0. | ||
| func runMonteCarlo(etObservations []int, successCount int, observedRunsPerPeriod float64, rng *rand.Rand) *ForecastMonteCarloSummary { | ||
| n := len(etObservations) | ||
| if n == 0 || observedRunsPerPeriod <= 0 { | ||
| if n == 0 || observedRunsPerPeriod <= 0 || math.IsNaN(observedRunsPerPeriod) || math.IsInf(observedRunsPerPeriod, 0) { | ||
| forecastMonteCarloLog.Printf("Skipping Monte Carlo: observations=%d, runs_per_period=%.2f", n, observedRunsPerPeriod) |
Comment on lines
+103
to
+116
| // TestRunMonteCarloNonFiniteLambda verifies that runMonteCarlo returns nil for | ||
| // non-finite λ inputs (NaN and +Inf) without hanging or panicking. | ||
| // Specification reference: R-MC-001 requires graceful handling of degenerate λ values. | ||
| func TestRunMonteCarloNonFiniteLambda(t *testing.T) { | ||
| obs := []int{1000, 2000, 3000} | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| lambda float64 | ||
| }{ | ||
| {"NaN lambda", math.NaN()}, | ||
| {"+Inf lambda", math.Inf(1)}, | ||
| {"-Inf lambda", math.Inf(-1)}, | ||
| } |
| // 1. A fuzzy expression is scattered to a valid 5-field cron string. | ||
| // 2. The scattered cron is accepted by parseCronSchedule without error. | ||
| // 3. buildScheduleGrid registers at least one non-zero slot for the workflow. | ||
| // 4. displayScheduleCalendar produces output that contains the workflow name. |
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.
Five spec files reviewed in the daily SPDD rotation had weak safeguard norms, missing file mappings, and untested edge cases. This addresses all P0–P2 items from the plan.
Spec strengthening
JavaScript tools SHOULD provide→MUST providefor V8 sandbox isolation; adds normative SM-JS-01 requiring sandboxed V8 context with restricted global scopeSHOULD be at least 10KB→ SM-IS-01 (MUST enforce a maximum input string length of at least 10KB; reject oversized inputs, never truncate silently)MUST NOTconstraints: ET-EXT-01 (no weight redefinition without version bump), ET-EXT-02 (no new mandatory schema fields without conformance revision), ET-EXT-03 (no reserved token class name reuse)effective_tokens.go,logs_models.go,logs_episode.go,audit_report.go,token_usage.go, etc.) and a sync procedure checklistBug fix
runMonteCarlodid not guard against non-finiteλ—NaNcaused an infinite loop in Knuth's Poisson algorithm (p <= NaNis always false), and+Infproduced integer overflow:New tests
TestRunMonteCarloNonFiniteLambda— table-driven: NaN, +Inf, -Inf all return nil without hangingTestRunMonteCarloZeroLambdaFallback— table-driven: zero/negative/empty/positive λ cases covering R-MC-001TestFuzzyScheduleEndToEnd— exercisesFUZZY:DAILY,FUZZY:DAILY_WEEKDAYS,FUZZY:DAILY_AROUND:14:0through the full pipeline:parser.ScatterSchedule→parseCronSchedule→buildScheduleGrid→displayScheduleCalendarTestObservedRunsPerPeriodConsistency— assertsForecastWorkflowResult.ObservedRunsPerPeriod(JSON field / λ) is identical to the value passed torunMonteCarlo, codifying R-MC-002