Skip to content

feat: additive retries/assertions + schemaVersion MAJOR version-gate#36

Open
rdwr-taly wants to merge 1 commit into
devfrom
cross-repo-cli-additive-fields
Open

feat: additive retries/assertions + schemaVersion MAJOR version-gate#36
rdwr-taly wants to merge 1 commit into
devfrom
cross-repo-cli-additive-fields

Conversation

@rdwr-taly

Copy link
Copy Markdown
Owner

Summary

Adds the flowrunner-cli half of the cross-app FlowMap contract for Sprint Wave 3: the CLI now honors the new additive request-step fields (retries, assertions) and gates on schemaVersion MAJOR, all backward-compatible and TDD-driven.

Built GATE-FIRST — a golden conformance test landed and passed on current code before any version logic was touched, and stays green throughout.

1. Golden conformance test (the invariant)

tests/unit/test_golden_old_flow.py + tests/fixtures/golden_old_flow.json: a real pre-sprint flow parses to an identical execution model with no schemaVersion, with "1.0", and with an unknown MINOR "1.5" (absence ⇒ 1.0). This mirrors the FlowRunner UI repo's __tests__/goldenOldFlow.test.js and is the load-bearing guard that keeps the 24/7 CLI alive when it meets a slightly-newer file.

2. schemaVersion version-gate (FlowMap)

OPTIONAL top-level "MAJOR.MINOR" string, absence ⇒ "1.0".

  • Absent / "1.x" ⇒ accepted; unknown MINOR tolerated with a warning (unrecognized constructs still degrade gracefully).
  • Unknown MAJOR (>= 2) ⇒ rejected loudly with a ValidationError naming the version — no best-effort mis-execution against live traffic.
  • Non-string (e.g. int 2) ⇒ coerced-and-warned, then gated. Never a silent crash.

3. step.retries = {count, delayMs}

Mirrors the JS engine: outer retry loop re-issues the whole request on non-2xx or network error, delayMs between passes, fresh request per pass. count default 0 == prior behavior. User-stop is never retried. Orthogonal to the built-in connection/5xx resilience loop.

4. step.assertions[]

Reuses the frozen conditionData operator vocabulary via _evaluate_structured_condition. Records response_<id>_assertions[] ({name,variable,operator,value,passed}) + response_<id>_assertions_passed. Diagnostic only — never changes flow control, never crashes; unknown operators / missing targets degrade to a failed assertion with a warning.

Both new fields are ignored by older CLIs (extra='ignore'), so files that use them still run everywhere.

Testing

  • python -m pytest tests/unit/107 passed (73 baseline → +34 new: golden 5, gate 12, retries 8, assertions 7, plus model tests). Golden invariant green throughout.
  • Pre-existing test_container_control.py collection error (missing requests dep in the test env) is unrelated to this change.

Docs

Severity note added to RELEASE_NOTES.md (HIGH for the MAJOR-reject gate; LOW for the additive opt-in fields).

🤖 Generated with Claude Code

…-gate

Cross-app FlowMap conformance for the shared .flow.json contract (adds the
CLI's half of Sprint Wave 3's schema-evolution + additive-fields work).

GATE-FIRST, TDD:

1. Golden conformance test (tests/unit/test_golden_old_flow.py + fixture):
   a real pre-sprint flow parses to an IDENTICAL execution model with no
   schemaVersion, with "1.0", and with an unknown MINOR "1.5" (absence => 1.0).
   This is the load-bearing invariant and stays green after every change.

2. schemaVersion version-gate on FlowMap: OPTIONAL "MAJOR.MINOR" string,
   absence => "1.0". Absent/"1.x" accepted (unknown MINOR tolerated with a
   warning); unknown MAJOR (>= 2) REJECTED loudly with a ValidationError that
   names the offending version. Non-string values (e.g. int 2) are
   coerced-and-warned, then gated on MAJOR — never a silent crash. Turns silent
   mis-execution into a principled, auditable refusal.

3. step.retries={count,delayMs} on request steps, mirroring the JS engine: an
   outer retry loop re-issues the whole request on non-2xx OR network error,
   sleeping delayMs between passes, fresh request per pass. count default 0 ==
   prior behavior (single attempt). A user-requested stop is never retried.
   Orthogonal to the built-in connection/5xx resilience loop.

4. step.assertions[] on request steps, reusing the frozen conditionData
   operator vocabulary via _evaluate_structured_condition. Records pass/fail
   into context (response_<id>_assertions[] + response_<id>_assertions_passed).
   Diagnostic only: never changes flow control, never crashes; unknown
   operators / missing targets degrade to a failed assertion with a warning.

Both new fields are ignored by older CLIs (extra='ignore'). Adds a severity
note to RELEASE_NOTES. Unit suite: 73 -> 107 passing.

Co-Authored-By: Claude Opus 4.8 <[email protected]>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a6c2051cf9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +56 to +57
with caplog.at_level(logging.WARNING):
FlowMap.model_validate(data)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Fix caplog capture for FlowRunner warnings

When this test runs, flow_runner.logger has already installed its own handler and disabled propagation, so caplog.at_level(logging.WARNING) only sees root-propagated records; the schema-version warning is printed to stderr but caplog.records remains empty. This makes pytest tests/unit/test_schema_version_gate.py::test_unknown_minor_warns_but_accepts fail consistently unless the test captures the FlowRunner logger explicitly or adjusts propagation/handlers for the assertion.

Useful? React with 👍 / 👎.

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