Skip to content

Collect missing New-agent inputs without inventing empty channel membership - #1322

Open
trissim wants to merge 3 commits into
mainfrom
work/commander-layout--P927--toad
Open

trissim wants to merge 3 commits into
mainfrom
work/commander-layout--P927--toad

Conversation

@trissim

@trissim trissim commented Oct 29, 2026 •

Copy link
Copy Markdown

Summary

3 commits; 2 files; +50/-12 against origin/main. P927 companion to OpenHCSDev/agent-comms#3595. This head is bc2dec11c0bdf03b8d2159f8ceaa89070f4a40a9; paired Core is ebab03ca4ca2182b63c96c3d3527d26e563a2fa9.

  • Dialog presence decisions: two branches -> zero; Core's field owns them.
  • Silent editor commit implementations: two -> zero; OptionalEditor and CatalogField implement the abstract contract.
  • Infrastructure failures presented as validation refusal: broad CLI_FAILURES catches deleted from both form preparation paths.

Published source, not application acceptance. Root resumed publication of the retained correction. No author tests, type checker, app launch, provider input, environment creation or installation in this turn. The previous 144a65bed head failed hosted Debt; that is not a passing result for this replacement.

1. Presence belongs to the declared field

Before, CommandDialog widened the editor annotation when required and not supplied, then combined None with required when gathering arguments. The widget was rediscovering Core's missing-input fact.

After, Core's nominal TargetField family supplies editor_annotation and argument(value). UnsuppliedTargetField owns the unset control and omission; SuppliedTargetField retains actual values, including None. The dialog consumes those answers without a required/None dispatch or touched-field store. ObjectState remains the one draft owner.

Core's New binding is pending until required tags exist. An untouched blank is not permission to create a private agent; an explicit Set commits empty text intentionally. The normal NewThreadAction/ThreadAction -> TargetAction -> CommandDialog path is retained; no second New UI or copied schema is added.

IDEN-2: fact = input has not been supplied; at = former dialog annotation/argument branches; derive = subtype behavior; by = Core UnsuppliedTargetField.

2. Explicit commit is an editor contract

Before:

ValueEditor.commit (silent no-op)
├── CatalogField (inherited no-op)
└── OptionalEditor (inherited no-op)

After:

ValueEditor.commit (abstract)
├── existing text/number/bool/choice/multiline implementations
├── CatalogField.commit (send canonical field value)
└── OptionalEditor.commit (held member commit, otherwise send None)

OptionalEditor owns Set/unset. Checking Set calls its member's commit, including unchanged empty text. Apply does not flush a hidden unset member. No editor-kind switch or force flag is retained.

3. Catalog preparation and refusal

Catalog refresh passes draft arguments to Core's binding preparation, which can edit native catalog context without completing New. Only CatalogBinding carries CatalogSelection; ordinary complete commands do not fabricate that context.

The two form preparation catches now accept ValueError/MalformedRecord, not CLI_FAILURES including OSError. A storage/network failure is not quietly relabeled invalid user input. PreparedChoices has its concrete dictionary type. Provider-login and refusal-feedback sites owned by toad-keymap remain unchanged.

4. Source evidence

The retained map covered 308 paired Toad modules and the actual application/target/form/editor family, with zero parse omissions at that mapped revision. Publication reproof parsed all 16 changed Python files across the pair, zero omissions, zero controls executed. Both repositories' git diff --check passed. These are source checks, not imports, typing or runtime proof.

Core#3595 documents the full hierarchy, native FieldPlan completion, represented editor wire changes, deleted mechanisms and old send-back closure. Both current bodies replace the earlier stale MISSING/suppliedness and Context-edit claims.

5. Required Root application proof — unrun here

  1. Open New through the real action on a saved-session copy before tags exist.
  2. Leave tags unset: Apply stays disabled; no creation.
  3. Choose real membership, create once, and verify a channel message reaches the agent and receives its reply.
  4. Deliberately Set empty text: private agent is valid; untouched blank must not do this.
  5. Unset tags, change model/thinking and refresh catalogs without premature completion or a crash.
  6. Exercise existing optional bool/choice/multiline controls and other complete-command dialogs, including their represented defaults.
  7. Fork through the real UI, verifying actual-parent membership and retained history.

Automated Toad tests are prohibited by project instructions. Root owns the final paired source/type review, combined Core controls, real UI journey and installation. No installed or working claim is made.

Assumed: both exact heads must be qualified and installed together; shared codec represented scalar/record/null/union behavior remains unproved. P916 acceptance stays separate, and P930 HUD belongs to commander-screen.

old form: untouched blank signs its own consent form
new field owner: no answer is no answer
the keyboard still gets the final vote

Copilot AI balanced review requested due to automatic review settings October 29, 2026 07:16

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

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.

2 participants