Skip to content

settings-merge tests: pin jq pipe-precedence, and fix a stale suite-routing reason label #358

Open

Description

@brenpike

Two small follow-ups surfaced by the local review on PR #357 and deliberately not expanded into it. Filed together because both are diagnostic/coverage quality on the same change, neither is a functional defect today.

1. The type-domain sweep does not pin pipe-precedence

plugin/skills/_shared/settings-merge.sh routes its element reads through canon_elem using the pipe form:

if (canon_elem
    | .type == "command"
      and (has("args") | not)
      and (.command | type) == "string"
      and ...)

This relies on jq binding | lower than and, so the single pipe scopes the whole conjunct chain. That is correct today — verified with a discriminating micro-test:

jq -n '... "x" | (canon_elem | .a == null and .b == null and .c == null)'   # -> true

If | bound tighter, .b would evaluate against "x" and jq would error.

The problem: the 150-assertion type-domain sweep in tools/test_shared_libs.sh does not pin this property. A bad precedence would be invisible to every scalar-input functional test in it, because .type == "command" is always false on the {} view and short-circuits before the later conjuncts are ever evaluated. So a future edit that adds parentheses scoping the pipe to the first conjunct only would keep the suite green while reintroducing the abort.

That is the same silently-never-wired shape this surface has now produced three times (#352 substring identity, the .args type-test, and #355 element access) — a correctness property that holds but is not mechanically defended.

Acceptance: a test that fails if the pipe is scoped to the first conjunct. It must not rely on short-circuit-reachable conjuncts — the discriminating input needs a value where a later conjunct would actually be evaluated against the un-normalized element.

2. Stale reason label in the suite-routing map

tools/validate.sh (~line 622) shares one add_selected call whose reason string is "$p (seed-hive entrypoint)". PR #357 added plugin/skills/_shared/settings-merge.sh to that leg, so the label is now printed for a _shared library, not an entrypoint.

Impact: zero beyond a misleading diagnostic. The reason string is consumed only by an echo (~line 847); --self-test asserts on suite names, never on reasons.

Fix: split the two patterns into separately-labelled add_selected calls, or relabel to something accurate for both (e.g. seed-hive behavior oracle).

Context

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    type:bugtriage type rating

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions