sneak/rgoue
1
0
Fork 0
Code Issues 10 Pull Requests Actions Packages Projects Releases Wiki Activity

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

New Issue
Closed
opened 2026-08-09 10:24:30 +02:00 by clawbot · 1 comment
clawbot commented 2026-08-09 10:24:30 +02:00
Collaborator
Copy Link

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.

## 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 commented 2026-08-09 12:19:24 +02:00
Author
Collaborator
Copy Link

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.

## 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.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
Review passed and gate is green; ready to merge
 needs-checks
make check is failing on this PR
 needs-rebase
PR conflicts with main and must be rebased
 needs-review
PR awaiting independent adversarial review
 needs-rework
Review found problems; author must rework
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/rgoue#31
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?