Skip to content

feat(executor): allow MXCLI_EXEC_TIMEOUT to override the per-statement timeout#315

Open
hjotha wants to merge 1 commit intomendixlabs:mainfrom
hjotha:submit/executor-configurable-timeout
Open

feat(executor): allow MXCLI_EXEC_TIMEOUT to override the per-statement timeout#315
hjotha wants to merge 1 commit intomendixlabs:mainfrom
hjotha:submit/executor-configurable-timeout

Conversation

@hjotha
Copy link
Copy Markdown
Contributor

@hjotha hjotha commented Apr 26, 2026

Part of #332.

Closes #314.

Summary

Adds MXCLI_EXEC_TIMEOUT so long-running executor operations can use a larger per-statement timeout without rebuilding mxcli.

Configuration

Accepted values:

  • Go duration strings, for example 12m, 2h30m, or 45s.
  • Bare seconds, for example 900.

Unset, empty, or invalid values fall back to the existing default.

Root cause

The executor used a hard-coded timeout constant, which is too rigid for large-project audit and describe workflows.

Fix

Replace direct use of the constant with a configuredExecuteTimeout() helper read on each Execute() call.

Tests

Added helper tests for Go duration values, bare-second values, and invalid-value fallback.

Validation

  • make build
  • make test
  • make lint-go
  • make test-integration

Agentic Code Testing

  • No new MDL syntax.
  • Environment-variable behavior is documented in the PR and covered by unit tests.

Test plan

  • Duration override works.
  • Bare-second override works.
  • Invalid override falls back to the default.

…t timeout

Long-running audit runs against large projects (1k+ microflows) can hit
the hard-coded 5-minute per-statement ceiling on commands like
`describe project`, `describe module`, or bulk `show callers`. Currently
the only options are to skip those commands or to fork mxcli.

Add an MXCLI_EXEC_TIMEOUT env var that overrides the default. Accepts:

- Go duration strings: "12m", "2h30m", "45s".
- Bare seconds: "900" → 15 minutes.

When unset, empty, or unparseable, falls back to the existing 5-minute
default. Read on every Execute() call so an operator can adjust mid-run
without restarting.

Refactor:

- Replace the `executeTimeout = 5 * time.Minute` const with
  `defaultExecuteTimeout` and a `configuredExecuteTimeout()` helper.
- `Execute()` reads the timeout via the helper instead of the const.
- Update the existing comment on `Execute()` to reference "the configured
  timeout" rather than the const name.

Tests:

- `TestConfiguredExecuteTimeoutUsesDurationEnv` — "12m" → 12*time.Minute.
- `TestConfiguredExecuteTimeoutUsesSecondEnv` — "900" → 15*time.Minute.
- `TestConfiguredExecuteTimeoutFallsBackForInvalidEnv` — "invalid" →
  defaultExecuteTimeout.

Tests use `t.Setenv` so they cannot leak across runs and are
parallel-safe with the rest of the executor test suite.
@github-actions
Copy link
Copy Markdown

AI Code Review

Critical Issues

None

Moderate Issues

None

Minor Issues

None

What Looks Good

  • The change is scoped to a single concern: making the executor timeout configurable via environment variable
  • No MDL syntax is added or modified, so full-stack consistency and syntax design checks don't apply
  • Tests cover all specified cases (duration format, bare seconds, invalid fallback)
  • The implementation correctly handles parsing with appropriate fallbacks
  • The change maintains backward compatibility (unset/empty/invalid values use existing default)
  • Documentation in PR matches implementation
  • All validation steps pass (build, test, lint, integration)

Recommendation

Approve. The PR is minimal, well-tested, and addresses the stated need without violating any project conventions. Since it doesn't introduce MDL syntax, the full-stack consistency and syntax design checklist items are not applicable and are correctly not flagged as issues.


Automated review via OpenRouter (Nemotron Super 120B) — workflow source

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.

Executor timeout is hard-coded for long-running project operations

2 participants