fix(interactive_config): one spelling per axis, and the keys now do something - #1154
Open
emooreatx wants to merge 2 commits into
Open
fix(interactive_config): one spelling per axis, and the keys now do something#1154emooreatx wants to merge 2 commits into
emooreatx wants to merge 2 commits into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…omething The adapter wizard language (`interactive_config`) had three structural drifts and three keys that were parsed and then ignored. This makes the schema say each thing one way, migrates the tree mechanically, and gives the keys the behaviour their names promise. SCHEMA (ciris_engine/schemas/runtime/manifest.py) - `required` is the only key on its axis. `optional` is gone; two booleans defaulting to False let a step be "neither", which no reader can act on. - `fields` is the only way to declare what a step writes. `field` and `field_name` are gone; a single field is a list of one. - `ConfigurationFieldDefinition.name` is required. `field_id` is gone. - `depends_on` is a list of step ids, nothing else. Its dict arm moved to `condition`, which is the one containment predicate. - `options`, `sensitive`, `readonly` are declared on fields. The tree wrote them and the client reads `options`; extra="allow" was hiding that. MIGRATION (tools/dev/migrate_interactive_config.py) Data-level rules are the oracle; the text patch edits only the lines that must change so the 49 mixed-style manifests keep their formatting. main() refuses any file where the patched text does not parse to exactly what the oracle says. 9 manifests, +40/-41, idempotent. `--check` for CI. ENFORCEMENT (adapter_configuration/service.py) - `condition` is EVALUATED. Three shapes, exhaustively: equals, not_equals, values. One evaluator, used on start, on advance, and by every status route -- so a conditional step is never shown by one path and skipped by another. external_data_sql showed both its sqlite and server steps to everyone. - A select step WRITES THE FIELD IT DECLARES. Adapters read the declared name (`dialect`, `response_mode`, `auth_method`) but only the step_id was ever stored: external_data_sql's validate_config failed with "dialect is required" after a completed wizard, mock_llm's response_mode never reached the environment, and no `condition` could ever hold. The step_id key is kept for anything reading it. - Step-level `required` blocks an empty submission. It was decorative. - The retired reads are gone: getattr(step,"optional"), f.field_id, and ConfigStepInfo now carries `required` with `optional` derived from it. LINT (tests/ciris_adapters/test_interactive_config_lint.py, 113 checks) Every key declared; no retired key; unique step ids; every field named; depends_on names an EARLIER step; condition.field is WRITTEN by an earlier step and has exactly one operator; a select declares at most one field; the migration is a fixed point over the tree. Also migrates the three get_config_schema() copies in navigation, mcp_client and a2a, which nothing calls but which used the retired keys. Client: CIRISClient maps steps through mapConfigStep() with null-tolerant reads of `required` and `fields`, renders select steps from options rather than fields, and follows the server's current_step on every advance -- so skipped steps and the new keys need no client change (CIRISClient#39). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Db9uHKMfhnANvagjWSo59x
emooreatx
force-pushed
the
fix/interactive-config-drift
branch
from
September 7, 2026 12:49
a3028a1 to
64aa8dc
Compare
The Type Check was the one red check on this PR: `actual` comes out of a Dict[str, Any], so `actual == cond["equals"]` is Any and mypy's no-any-return refuses it from a function declared -> bool (service.py:227, :229). Wrapped both comparisons in bool(); the third shape (`in`) already returns bool. No behaviour change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
emooreatx
added a commit
that referenced
this pull request
Sep 8, 2026
…estion, login after auto-login, verified reset, iOS 9091 - P1 manifest: `name` became mandatory in #1154 and the in-tree lint keeps it that way, but an out-of-tree adapter authored against the previous `field_id` spelling was rejected at ingestion and dropped from discover_services() with one error line. A before-validator now reads `field_id` as `name` with a warning naming the migration tool; the canonical key wins when both are present; a nameless field is still rejected. - P1 gate login-noai: with the #48 fix (client 0.5.212) the hand-off auto-logs in and lands on Interact, and desktop-login would have waited 30 s for a Login screen that never comes. It now logs out first (`_logout`, factored out of the reset flow) and drives the credential login, which is the assertion this leg exists for. - P2 desktop-reset: a confirmed click was reported as a reset. It now asserts the app restarted into first run (Login chooser / Setup) and, on desktop, that the home's .env no longer records the install -- the next step rewrites that .env itself, so a reset that stopped clearing the flag would otherwise still read green. - P2 iOS post-reset: :9091 is freed too on iOS, since the relaunched simulator app binds its test server first and the next --launch refuses while :9091 has an owner. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
What
The adapter wizard language (
interactive_config) had three structural drifts in its schema and three keys that were parsed and then ignored. This PR makes the schema say each thing one way, migrates the tree mechanically (9 manifests, +40/−41, oracle-verified, idempotent), and givescondition, step-levelrequired, and a select step's declared field the behaviour their names promise. It is the CIRISAgent half of the design in CIRISClient#39.Schema — one spelling per axis
requiredandoptional, both defaultFalse→ a step can be "neither"requiredonlyfieldsorfieldorfield_namefieldsonly; a single field is a list of onenameorfield_idname, requireddepends_on: List | Dict, plusconditiondepends_on: List[step_id];conditionis the one predicateoptions,sensitive,readonlyslipped throughextra="allow"ConfigurationFieldDefinitiontools/dev/migrate_interactive_config.pyholds the data-level rules (the oracle) and a same-line text patch so the 49 mixed-style manifests keep their formatting; it refuses any file whose patched text does not parse to exactly what the oracle says.--checkis a CI gate.Enforcement — the keys now do something
conditionis evaluated. Three shapes, exhaustively:equals,not_equals,values. One evaluator (AdapterConfigurationService.current_step) is used on start, on advance, and by all three status routes — so a conditional step can never be shown by one path and skipped by another. Before:external_data_sqlshowed both its SQLite and server-connection steps to everyone.dialect,response_mode,auth_method); the engine only ever stored thestep_id. Soexternal_data_sql.validate_configfailed with "dialect is required" after a completed wizard,mock_llm'sresponse_modenever reached the environment, and noconditioncould ever hold. Thestep_idkey is kept for anything that reads it.requiredblocks an empty submission. Only field-level was checked; a required step with all-optional fields advanced on{}.getattr(step, "optional"),f.name or f.field_id.ConfigStepInfocarriesrequired;optionalstays on the wire, derived, so they cannot disagree.external_data_sql/select_dialectnow declaresfields: [{name: dialect}]— the lint caught that its two conditional steps read a field nothing wrote.Lint —
tests/ciris_adapters/test_interactive_config_lint.py113 checks over the 22 manifests: every key declared; no retired key; unique
step_ids; every field named;depends_onnames an earlier step;condition.fieldis written by an earlier step and carries exactly one operator; a select declares at most one field; the migration is a fixed point over the tree; the schema has one key per axis.Tests
test_adapter_configuration.py: 65 → 75 (conditions shown/skipped per branch, uncollected field does not hold, three operators and nothing else, conditional first step on start with/withoutexisting_config, status and execute agree, required input refuses{}, optional advances, select skippable iff not required, select writes its declared field).tests/adapters/api/routes/system: 130 passed.tests/adapters+ touchedtests/ciris_adapters: 415 passed after adapting two ownership-test mocks to the new seam (config_service.current_step).Client
No client change needed for this to land: CIRISClient maps steps through
mapConfigStep()with null-tolerant reads ofrequiredandfields, renders select steps fromoptions_methodresults rather thanfields, and re-fetches the server'scurrent_stepon every advance — so skipped steps just arrive as the next step. The one visible effect is the step counter ("3 of 5") jumping when a step is skipped.Noted, not fixed here
mcp_server/manifest.jsondeclaresoptions_method: get_config_optionson three select steps but there is no configurable class underciris_adapters/mcp_server/that implements it. The lint validates the manifest; it cannot see that nothing serves it.get_config_schema()in navigation / mcp_client / a2a duplicates the manifest inline and nothing calls it; migrated for consistency, but it should probably go.🤖 Generated with Claude Code
https://claude.ai/code/session_01Db9uHKMfhnANvagjWSo59x