fix(types): an omitted TYPE_* key degrades as documented instead of crashing under set -u (DIVE-2076, reported in #196) - #206
Merged
Conversation
…ng under set -u (DIVE-2076) Reported from outside by A-MO7SEN (github #196) while adding a new agent type: `${TYPE_CHANNELS[$type]}` was read bare under `set -uo pipefail`, so a type registered without a TYPE_CHANNELS entry did not degrade to "no channels" — it hard-crashed `agent types` with an unbound-variable error naming the ARRAY rather than the missing type. Readers now supply the documented default. TYPE_CHANNELS defaults to 0, which routes an omitted key into the existing validation error that names the type ("type 'x' does not support channels") — the behaviour the reporter arranged by hand with an explicit [devin]=0. The sweep half found the same defect one map over, and live rather than hypothetical: cmd_auth_set read TYPE_API_VAR/TYPE_API_FILE bare at cmd_auth.sh one line above a graceful `fail` written for exactly the absent case, which even names hermes/openclaw. Those two maps are SPARSE BY DESIGN and their own comments claim "cmd_auth_set already fails gracefully when a type isn't in this map" — under set -u it never reached that line. 4 of 8 types (hermes, openclaw, antigravity, pi) crashed instead of getting the message. The comment described behaviour the code did not have; it does now. cmd_agent_create's error also hardcoded the supported-type list, one more place a new type must be registered and silently would not be; it derives the list. cmd_account's read is safe by construction (it iterates that map's own keys) but takes the default anyway: the contract is deliberately exemption-free, because a rule with no carve-outs is one a contributor can follow. The ticket's second half was the documentation, so it is the larger half of the diff: header.sh now states the absent-vs-zero contract above TYPE_BIN, splitting the maps into REQUIRED (TYPE_BIN — it IS the registry, is_known_type tests it, so bare reads are correct by construction) and OPTIONAL (everything else, where omission is a meaningful signal with a documented default every reader must supply). TYPE_CHANNELS states its own default at its declaration. tests/type_map_registration_contract_unit.sh enforces both halves in two dimensions, because either alone is defeatable. The static pass rejects any bare variable-keyed read of an optional map, so a bare read added tomorrow reds. The behavioural pass registers a synthetic type in TYPE_BIN only and runs the real reader expressions under set -u — lifted FROM the source, not written in the test, since a probe hardcoding `:-0` proves the default works while passing happily against a tree whose readers are still bare. A control probe asserts a bare read of a missing key still crashes, so a `set -u` that quietly stopped applying cannot read as green. Against origin/main the harness reds with 14 graded failures; post-fix it passes.
Resolves the 5dive.sha256 conflict by rebuilding from source rather than picking a side. Assigns the release number and CHANGELOG entry at merge time per CONTRIBUTING; the branch was correctly left unversioned by its author. 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.
Second of the two defects @A-MO7SEN reported in #196 while registering a new agent type. Does not close #196 — that issue asks for the
devintype itself, which is his own #197.A bare
\${TYPE_CHANNELS[\$type]}read underset -umade a type registered without that key hard-crashagent typeswith an unbound-variable error naming the array — an error pointing at our internals rather than the missing key. Seven readers fixed (the ticket named five); a:-0default routes an omitted key into the existing "type X does not support channels" path.The sweep found a live instance, not a future-type hypothetical.
cmd_auth.shreadTYPE_API_VAR/TYPE_API_FILEbare one line above a gracefulfailwritten for exactly the absent case — which even names hermes and openclaw. Both maps are sparse by design and their comments claimedcmd_auth_set"already fails gracefully when a type isn't in this map". Underset -uexecution never reached that line, so 4 of 8 types (hermes, openclaw, antigravity, pi) crashed onauth set-keyinstead of getting the message. The comment described behaviour the code did not have.Author: dev3, who reproduced the reporter's symptom end-to-end on the built bundle (inject
[devin]intoTYPE_BINonly: pre-fix diesTYPE_CHANNELS[\$type]: unbound variablerc=1; fixed printsdevin … channels=norc=0) and graded the harness rather than just running it — 14 graded failures pre-fix. Notably their first harness version hardcoded:-0and passed against a still-bare tree, i.e. proved the default works rather than that the code uses it; the shipped version lifts the expressions from source.Stated weakest link: no genuinely new type was provisioned end-to-end (
agent create --type=devinagainst a real binary), so this proves the maps degrade, not that a full create/auth/channel lifecycle works. That belongs to #197's review.Release: 0.16.13 assigned at merge with the CHANGELOG entry, per CONTRIBUTING.
🤖 Generated with Claude Code