Skip to content

🐛 fix(api): accept lexical_gate as a /api/v1/search param - #83

Merged
cwest merged 1 commit into
mainfrom
wt/t_379f7bfd
Aug 3, 2026
Merged

🐛 fix(api): accept lexical_gate as a /api/v1/search param#83
cwest merged 1 commit into
mainfrom
wt/t_379f7bfd

Conversation

@cwest

@cwest cwest commented Aug 3, 2026

Copy link
Copy Markdown
Owner

Why

#81 (feat(api): expose the lexical gate) landed on main but its own push CI is red: the /api/v1/search handler validates query keys against a closed allowlist before dispatch, and #81 added the gate handler (lookupGateParam) without adding lexical_gate to the allowlist. Every gated request was rejected with 400 unknown query parameter "lexical_gate" at internal/apiserver/search.go before the handler ran — the param the feature exposes was refused by the surface meant to accept it. Main build-test has been FAILURE since 69c3673, inheriting 6 apiserver failures into every downstream PR (this is what dependency-blocks PR #82).

What

  • Add lexical_gate to the searchParams allowlist. Only the canonical underscore spelling is listed; canonicalParam normalizes -_, so the same entry also admits the lexical-gate alias lookupGateParam reads. Scope is the allowlist only — the gate logic is untouched.
  • Add a regression test (TestSearch_LexicalGateIsAcceptedParam) with both controls: a valid gate value (both separator spellings) → 200; a genuinely unknown param → still 400. This keeps the allowlist from silently dropping the param again and proves the fix isn't a relaxation of the closed-set validation #80 established.

Verification

Layer 1 — spec-conformance suite (go test ./internal/okf/ ./cmd/ -run Conformance -race):

ok  github.com/cwest/okfctl/internal/okf   1.495s
ok  github.com/cwest/okfctl/cmd            1.484s

Layer 2 — full suite (gofmt -l . empty; go vet ./... clean; go test ./... -race):

ok  github.com/cwest/okfctl/cmd                  9.236s
ok  github.com/cwest/okfctl/cmd/okfctl-api       2.060s
ok  github.com/cwest/okfctl/cmd/okfctl-search    2.317s
ok  github.com/cwest/okfctl/internal/apiserver   1.784s
ok  github.com/cwest/okfctl/internal/okf         6.446s
ok  github.com/cwest/okfctl/internal/okfconfig   1.783s
ok  github.com/cwest/okfctl/internal/plugin      2.319s
ok  github.com/cwest/okfctl/internal/search      1.717s

The 6 previously-failing TestSearch_Gate* / TestSearch_CrossSurfaceParity tests now pass; TestSearch_LexicalGateIsAcceptedParam passes.

Layer 3 — real corpus (~/src/knowledge-base/bundles/knowledge, ~239 nodes), before = main/69c3673, after = this branch:

surface before after
validate OK (exit 0) OK (exit 0)
lint --strict findings 4 4

diff of the full lint --strict output before vs after is byte-identical — the count that did not move is the control proving this apiserver-allowlist fix touches no OKF-defined behavior (validate/lint unchanged by construction). CGO_ENABLED=0 go build succeeds.

No spec-mandated search behavior changed: the closed-allowlist strictness (§ n/a — an implementation policy from #80, not a spec clause) is preserved, and the added param is a tool feature, not an OKF-defined field.

Done when

  1. go test ./internal/apiserver/ -race green — all 6 gate/parity tests pass.
  2. gofmt -l . empty, go vet ./... clean, go test ./... -race green.
  3. go test ./internal/okf/ ./cmd/ -run Conformance -race green.
  4. ✅ Real-corpus layer-3 run, before/after counts pinned above.
  5. ✅ Regression test with both controls (200 on valid gate; 400 on unknown param).
  6. ⏳ On merge, main push CI build-test returns to SUCCESS, unblocking PR 🐛 fix(template): accept a v0.1 flat-string sources list for required_fields #82.

The /api/v1/search handler validates query keys against a closed
allowlist before dispatch. The lexical-gate feature added the gate
handler (lookupGateParam) but not the allowlist entry, so every gated
request was rejected with 400 "unknown query parameter" before the
handler ran — the param the feature exposes was refused by the surface
meant to accept it.

Add lexical_gate to searchParams. Only the canonical underscore spelling
is listed; canonicalParam normalizes '-' to '_', so the same entry also
admits the lexical-gate alias lookupGateParam reads. Scope is the
allowlist only — the gate logic is unchanged.

Add a regression test asserting lexical_gate is accepted (both separator
spellings -> 200) while a genuinely unknown param still -> 400, so the
allowlist cannot silently drop it again and stays closed.

@cwest cwest left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

No changes needed.

The fix is the one allowlist entry the break called for. #81 wired up lookupGateParam but never added lexical_gate to searchParams, so canonicalizeQuery rejected every gated request with a 400 before the handler ran. Adding the single underscore spelling is enough: canonicalParam normalizes - to _, so the same entry also admits the lexical-gate alias without a second list item. The gate logic is untouched.

I reproduced the break and confirmed the fix on a fresh checkout. main at 69c3673 fails on exactly six apiserver tests, all with the same 400 unknown query parameter "lexical_gate" symptom; on this branch all six go green and the full go test ./... -race suite is clean across every package, with gofmt and vet clean and the conformance layer passing. No new failures anywhere.

The regression test earns its place: it drives both separator spellings to 200 and holds an unknown param at 400, so the allowlist can't silently drop the param again and the fix can't drift into a blanket accept-everything that would undo the closed-set strictness from #80.

Green main here is what lets #82 rebase off a clean base.

@cwest
cwest marked this pull request as ready for review August 3, 2026 15:10
@cwest

cwest commented Aug 3, 2026

Copy link
Copy Markdown
Owner Author

Verified this over live HTTP rather than from the diff, control versus candidate, both built from a fresh clone with Go 1.26.5. Control is 69c3673 (current main); candidate is 0745746 (this branch head). Both served the same eight-node synthetic bundle built in /tmp, same index, same embedder.

The break on main reproduces, and this clears it

request control 69c3673 candidate 0745746
lexical_gate=true 400 unknown query parameter "lexical_gate" 200
lexical-gate=true (dash alias) 400 200
lexical_gate=false 400 200
gate omitted 200 200
bogus=1 400 400
lexical_gate=notabool 400 400

The last two are the regression controls. The closed allowlist from #80 survives: an unknown parameter is still rejected, and a malformed boolean is rejected with "lexical_gate" must be a boolean rather than being folded back into the unknown-parameter message. Distinguishing those two failures matters more than it sounds, because a caller who typos the value and a caller who typos the key need different advice.

With the gate omitted, the candidate's response is byte-identical to the control's, all 1,385 bytes. That is the control saying this adds a parameter without perturbing the existing surface.

The gate is actually plumbed through, not merely accepted

A 200 on its own would prove nothing, so I checked that the parameter reaches the search path. On my first query the gated and ungated results were identical, which is the documented no-op on an over-broad query, but indistinguishable from a parameter that is parsed and dropped.

So I found queries where the gate demonstrably bites and checked the two surfaces against each other:

query gated result CLI and HTTP agree gate changes the result
codling moth one node yes yes
nitrogen one node yes yes
yeast aromatics one node yes yes
keeved one node yes yes
tartaric one node yes yes

On all five, okfctl-search --lexical-gate and GET /api/v1/search?...&lexical_gate=true return the same node in the same order, and both differ from the ungated result. Cross-surface parity is real here, not just asserted by a test name.

The gate also composes. Paired with path, type, tag, half_life with decay_floor, and min_relevance with half_life, every combination returns 200 and the expected single result. The control rejects each of those same requests with a 400, which pins the difference to this change rather than to anything already present.

Suite state

On this branch go test ./... is green across every package. On 69c3673 I reproduced the failure independently: six failing tests, the four TestSearch_Gate* cases and five TestSearch_CrossSurfaceParity subtests, every one of them a 400 where a 200 was wanted.

The interaction is worth naming, since neither pull request was wrong on its own. #80 and #81 were both cut from 78ae7c3 and both green there. #80 closed the parameter allowlist; #81 added a parameter and its tests. Merged in sequence, each against a base that lacked the other, the new parameter was never added to the new allowlist and main went red on the second merge. A closed allowlist turns every concurrent parameter-adding branch into this same latent conflict, and the merge queue will not see it, because both branches are green right up to the moment they are both in.

This also unblocks #82, whose red check is inherited from main rather than its own. I merged the two locally to confirm rather than assume: they touch disjoint files, the merge is clean, and the full suite is green on the result.

@cwest
cwest merged commit 21a078d into main Aug 3, 2026
1 check passed
@cwest
cwest deleted the wt/t_379f7bfd branch August 3, 2026 19:04
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