Audit C's command switch against commandHandlers for other silently-dropped keys #31

New Issue

2026-08-09T10:24:30+02:00

clawbot commented

2026-08-09 10:24:30 +02:00

Problem

'+' (#11) was missing from commandHandlers and answered
"illegal command '+'" where C answers "sorry". Nobody found it by
auditing — it surfaced incidentally during a survey. Nothing has ever
checked C's command switch against the Go dispatch table, so there is no
reason to believe '+' was the only one.

The port is otherwise function-by-function faithful, which makes a
key-by-key gap especially easy to miss: every function has a counterpart,
so a missing dispatch entry leaves no dangling reference and no compile
error. It fails silently, at runtime, only when a player presses the key.

'+' also showed the failure is subtler than "the key does nothing": it lived
in C's main switch rather than the if (wizard) sub-switch, so the
divergence existed in normal play too, not just wizard mode. An audit needs to
respect that distinction.

Definition of done

Every case label in C's command.c command switch — both the main
switch and the if (wizard) sub-switch — is accounted for, as exactly one
of:
- present in Go's commandHandlers (or the wizard dispatch, matching C's
  placement); or
- recorded in ARCHITECTURE.md §9 as deliberately dropped, with the reason.
A test enforces this going forward, so the two lists cannot silently
drift again. The mechanism is yours to choose — a table of expected keys
checked against commandHandlers, with a comment pointing at the C switch,
is probably enough. It must fail if a key is removed from the dispatch
table.
Any key found missing is either implemented (if small and unambiguous) or
filed as its own issue — do NOT bulk-implement a pile of behaviours in
an audit commit. Report what you found and split.
#ifdef MASTER blocks are handled explicitly: state whether the port
targets the MASTER build or not, and apply that consistently. ('+' was
inside #ifdef MASTER, and was ported.)
make check fully green.
TODO.md updated in the same commit — Completed Steps entry, do not
rotate "Next Step".
Commit title ends with (closes #N).

Implementation requirements

Read git show origin/c-master:command.c in full. Do NOT check out or
modify origin/c-master or modern-rogue.
Beware the when macro — it expands to break; case, so case labels do not
look like ordinary C case statements. A naive grep for case will miss
them. '+' is written when '+':.
Mind the two switches. A key in C's main switch that Go handles only in
wizard mode (or vice versa) is a divergence even if the key "works".
Where a key exists in both but behaves differently, that is out of scope for
this audit — note it and file separately. This issue is about presence.
Verify before asserting. On this repo, issue bodies and briefs have been
wrong repeatedly, including several written by me. If this description is
wrong when you check it, say so and stop.
Lint: golangci-lint on this host shares one cache and lock across ~18
concurrent sessions and has produced a false green here. Export
GOLANGCI_LINT_CACHE to a fresh empty private directory before
make lint/make check and confirm no foreign paths appear in the output.
make targets only. Do NOT modify .golangci.yml. No Dockerfile/CI/script/.
Do NOT regenerate goldens under game/testdata/.
Never mention Claude or Anthropic anywhere.

Priority

Medium. No known defect behind it — but '+' proves the class exists, and
the audit is bounded, one-time, and leaves a regression test behind.

## Problem `'+'` (#11) was missing from `commandHandlers` and answered *"illegal command '+'"* where C answers *"sorry"*. Nobody found it by auditing — it surfaced incidentally during a survey. **Nothing has ever checked C's command switch against the Go dispatch table**, so there is no reason to believe `'+'` was the only one. The port is otherwise function-by-function faithful, which makes a key-by-key gap especially easy to miss: every *function* has a counterpart, so a missing *dispatch entry* leaves no dangling reference and no compile error. It fails silently, at runtime, only when a player presses the key. `'+'` also showed the failure is subtler than "the key does nothing": it lived in C's **main** switch rather than the `if (wizard)` sub-switch, so the divergence existed in normal play too, not just wizard mode. An audit needs to respect that distinction. ## Definition of done 1. Every case label in C's `command.c` command switch — **both** the main switch and the `if (wizard)` sub-switch — is accounted for, as exactly one of: - present in Go's `commandHandlers` (or the wizard dispatch, matching C's placement); or - recorded in `ARCHITECTURE.md` §9 as deliberately dropped, with the reason. 2. A **test** enforces this going forward, so the two lists cannot silently drift again. The mechanism is yours to choose — a table of expected keys checked against `commandHandlers`, with a comment pointing at the C switch, is probably enough. It must fail if a key is removed from the dispatch table. 3. Any key found missing is either implemented (if small and unambiguous) or **filed as its own issue** — do NOT bulk-implement a pile of behaviours in an audit commit. Report what you found and split. 4. `#ifdef MASTER` blocks are handled explicitly: state whether the port targets the MASTER build or not, and apply that consistently. (`'+'` was inside `#ifdef MASTER`, and was ported.) 5. `make check` fully green. 6. `TODO.md` updated in the same commit — Completed Steps entry, do **not** rotate "Next Step". 7. Commit title ends with ` (closes #N)`. ## Implementation requirements - Read `git show origin/c-master:command.c` in full. Do NOT check out or modify `origin/c-master` or `modern-rogue`. - Beware the `when` macro — it expands to `break; case`, so case labels do not look like ordinary C `case` statements. A naive grep for `case ` will miss them. `'+'` is written `when '+':`. - Mind the two switches. A key in C's main switch that Go handles only in wizard mode (or vice versa) is a divergence even if the key "works". - Where a key exists in both but behaves differently, that is out of scope for this audit — note it and file separately. This issue is about *presence*. - **Verify before asserting.** On this repo, issue bodies and briefs have been wrong repeatedly, including several written by me. If this description is wrong when you check it, say so and stop. - **Lint:** golangci-lint on this host shares one cache and lock across ~18 concurrent sessions and has produced a false green here. Export `GOLANGCI_LINT_CACHE` to a fresh empty private directory before `make lint`/`make check` and confirm no foreign paths appear in the output. - `make` targets only. Do NOT modify `.golangci.yml`. No Dockerfile/CI/`script/`. - Do NOT regenerate goldens under `game/testdata/`. - Never mention Claude or Anthropic anywhere. ## Priority Medium. No known defect behind it — but `'+'` proves the class exists, and the audit is bounded, one-time, and leaves a regression test behind.

clawbot referenced this issue

2026-08-09 10:30:18 +02:00

Port the '+' wizard-mode toggle-off (closes #11) #30

clawbot commented

2026-08-09 12:19:24 +02:00

Audit done (read-only); implementation plan

Verified the issue's premises before planning — all three hold:

origin/c-master:rogue.h 52-53 really are #define when break;case and
#define otherwise break;default, so a grep 'case ' finds ~6 of ~75 labels.
CTRL is extern.h:113, (c & 037); ESCAPE is rogue.h:121, 27.
There really are two switches: the main one at command.c 151-427 and
if (wizard) switch (ch) at 369-423, inside the otherwise: arm.
#ifdef MASTER appears exactly 4 times in command.c — lines 67
(wizard implies noscore), 128 (^D/^A in the count-suppression
switch), 317 ('+'), 368 (the whole wizard sub-switch). All four are
ported unconditionally, so this port targets the MASTER build, and that
is consistent. Matches sticks.c:237 from #13.

Result: no further missing keys. `'+'` was the only one.

Mechanically extracted every when/case/otherwise label in 151-427 and
matched it against the Go dispatch. 60 main-switch labels, 15 wizard-switch
labels, all accounted for, and all on the correct side of the main/wizard
split. Nothing needs adding to ARCHITECTURE.md §9 — no case label is
dropped. (The one §9 entry that touches this switch, the passwd() enter
arm of '+', is a sub-arm of a present key, not a missing label.)

Cross-check in the other direction also passes: Go's commandHandlers has no
key C's main switch lacks, and the wizard-only keys (notably 'C') sit in
wizardCommand, not in commandHandlers — C's placement.

Plan

No behaviour change; the deliverable is the regression test.

New game/dispatch_test.go with three tables transcribed from the C
switch, each carrying the command.c line range it came from:
- cMainSwitchTableKeys — the 48 main-switch labels Go answers from
  commandHandlers;
- cMainSwitchMultiStepKeys — the 12 whose arms need a re-dispatch
  (goto over) or a fallthrough and so live in dispatchKey's own
  switch: the 8 ctrl-directions, F, f, a, m;
- cWizardSwitchKeys — the 15 sub-switch labels.
Three tests:
- set equality between cMainSwitchTableKeys and the actual keys of
  commandHandlers — both directions, so it fails on a removed key and
  on a wizard key wrongly promoted into the main table;
- each cMainSwitchMultiStepKeys key is claimed by dispatchKey rather
  than falling through to illcom, asserted on its return value and
  flags (runCommand only sets flags; 'a' re-dispatches LastComm;
  F/f/m get Escape at the direction prompt, which makes them
  side-effect-free while still proving the arm ran);
- each cWizardSwitchKeys key is claimed by wizardCommand, checked as
  "the top line does not read illegal command", one fresh game per key.
  Safe against prompts: testTerm.ReadChar never blocks or exhausts.
Non-vacuity proof for each of the three, by deleting one key and showing
the failure, then restoring.
TODO.md Completed Steps entry, no Next Step rotation.

Nothing to file separately: no missing key, and no key that is present but
behaves differently turned up in the presence pass.

## Audit done (read-only); implementation plan Verified the issue's premises before planning — all three hold: - `origin/c-master:rogue.h` 52-53 really are `#define when break;case` and `#define otherwise break;default`, so a `grep 'case '` finds ~6 of ~75 labels. `CTRL` is `extern.h:113`, `(c & 037)`; `ESCAPE` is `rogue.h:121`, `27`. - There really are two switches: the main one at `command.c` 151-427 and `if (wizard) switch (ch)` at 369-423, inside the `otherwise:` arm. - `#ifdef MASTER` appears exactly 4 times in `command.c` — lines 67 (`wizard` implies `noscore`), 128 (`^D`/`^A` in the count-suppression switch), 317 (`'+'`), 368 (the whole wizard sub-switch). **All four are ported unconditionally**, so this port targets the MASTER build, and that is consistent. Matches `sticks.c:237` from #13. ### Result: no further missing keys. `'+'` was the only one. Mechanically extracted every `when`/`case`/`otherwise` label in 151-427 and matched it against the Go dispatch. 60 main-switch labels, 15 wizard-switch labels, all accounted for, and all on the correct side of the main/wizard split. Nothing needs adding to ARCHITECTURE.md §9 — no case label is dropped. (The one §9 entry that touches this switch, the `passwd()` enter arm of `'+'`, is a sub-arm of a present key, not a missing label.) Cross-check in the other direction also passes: Go's `commandHandlers` has no key C's main switch lacks, and the wizard-only keys (notably `'C'`) sit in `wizardCommand`, not in `commandHandlers` — C's placement. ### Plan No behaviour change; the deliverable is the regression test. 1. New `game/dispatch_test.go` with three tables transcribed from the C switch, each carrying the `command.c` line range it came from: - `cMainSwitchTableKeys` — the 48 main-switch labels Go answers from `commandHandlers`; - `cMainSwitchMultiStepKeys` — the 12 whose arms need a re-dispatch (`goto over`) or a fallthrough and so live in `dispatchKey`'s own switch: the 8 ctrl-directions, `F`, `f`, `a`, `m`; - `cWizardSwitchKeys` — the 15 sub-switch labels. 2. Three tests: - **set equality** between `cMainSwitchTableKeys` and the actual keys of `commandHandlers` — both directions, so it fails on a removed key *and* on a wizard key wrongly promoted into the main table; - each `cMainSwitchMultiStepKeys` key is claimed by `dispatchKey` rather than falling through to `illcom`, asserted on its return value and flags (`runCommand` only sets flags; `'a'` re-dispatches `LastComm`; `F`/`f`/`m` get `Escape` at the direction prompt, which makes them side-effect-free while still proving the arm ran); - each `cWizardSwitchKeys` key is claimed by `wizardCommand`, checked as "the top line does not read `illegal command`", one fresh game per key. Safe against prompts: `testTerm.ReadChar` never blocks or exhausts. 3. Non-vacuity proof for each of the three, by deleting one key and showing the failure, then restoring. 4. `TODO.md` Completed Steps entry, **no** `Next Step` rotation. Nothing to file separately: no missing key, and no key that is present but behaves differently turned up in the presence pass.

clawbot referenced this issue from a commit

2026-08-09 16:17:23 +02:00

test: pin command dispatch to C's command.c switch (closes #31)

clawbot referenced a pull request that will close this issue

2026-08-09 16:17:46 +02:00

Pin command dispatch to C's command.c switch (closes #31) #33

clawbot referenced this issue

2026-08-09 16:26:03 +02:00

Pin command dispatch to C's command.c switch (closes #31) #33

clawbot referenced this issue from a commit

2026-08-09 16:27:28 +02:00

test: pin command dispatch to C's command.c switch (closes #31)