feat: rebuild YatzCLI v2 from scratch

[edge2992]

Summary

• Engine: Pure game logic with state machine (4 phases), scoring for all 13 Yahtzee categories, deterministic dice, greedy AI, and GameClient interface abstracting local/remote play
• CLI: Interactive TUI with bubbletea v2 (rolling/choosing states, keyboard controls)
• MCP: Server with 6 tools for LLM integration (new_game, roll_dice, hold_dice, score, get_state, get_scorecard)
• P2P: Host-authority online play with length-prefixed JSON over TCP, handshake, turn management, error handling
• Matchmaking: Lambda + API Gateway WebSocket + DynamoDB serverless backend, with Go client
• CI/CD: GitHub Actions (unit + E2E test jobs), GoReleaser for multi-platform releases

Old client/server/game/messages/network packages removed and replaced entirely.

Test plan

• go test -short ./... runs unit tests only (E2E skipped)
• go test ./... runs all tests including E2E (MCP full game, P2P full game)
• go build ./cmd/yatz/ produces working binary
• yatz play launches local game against AI
• yatz mcp starts MCP server (verify with Claude Code)
• CI passes: unit job then e2e job

[Copilot]

Pull request overview

Rebuilds YatzCLI as a single yatz binary with a new engine/ core, Bubble Tea TUI, MCP server integration, and a new P2P + serverless matchmaking networking stack, replacing the legacy gob-based client/server implementation.

Changes:

• Replaced legacy client/, server/, game/, messages/, network/ with a new pure engine/ (state machine + scoring + AI + GameClient).
• Added new interactive TUI (cli/), MCP server (mcp/), P2P protocol + host/guest (p2p/), matchmaking client (match/) and AWS Lambda handler (lambda/).
• Updated module path, docs, and CI/release configuration for the new architecture and distribution.

Reviewed changes

Copilot reviewed 62 out of 66 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
server/server.go	Removes legacy TCP server implementation
server/room.go	Removes legacy room model
server/room_manager.go	Removes legacy room manager
server/room_controller.go	Removes legacy room controller
server/room_controller_test.go	Removes legacy room controller tests
server/player.go	Removes legacy Player model
server/handler.go	Removes legacy connection handler
server/gameplay_controller.go	Removes legacy gameplay controller
server/gameplay_controller_test.go	Removes legacy gameplay controller tests
server/controller.go	Removes legacy controller interface
server/controller_test.go	Removes legacy commented-out tests
cmd/server/main.go	Removes legacy server entrypoint
cmd/client/main.go	Removes legacy client entrypoint
client/client.go	Removes legacy gob-based client
client/client_test.go	Removes legacy client tests
client/console_io_handler.go	Removes legacy survey-based UI
client/iohandler.go	Removes legacy IO abstraction
client/mock_iohandler.go	Removes legacy IO mocks
network/connections.go	Removes legacy gob connection abstraction
network/mock_connection.go	Removes legacy mock connection
messages/messages.go	Removes legacy message schema
game/game.go	Removes legacy shared game types
game/score.go	Removes legacy scoring implementation
game/yahtzee.go	Removes legacy gameplay constants/types
engine/category.go	Adds new category definitions/constants
engine/dice.go	Adds deterministic dice rolling/rerolling helpers
engine/dice_test.go	Adds dice unit tests
engine/scoring.go	Adds scoring for all categories
engine/scoring_test.go	Adds scoring unit tests
engine/scorecard.go	Adds new scorecard representation and helpers
engine/scorecard_test.go	Adds scorecard unit tests
engine/game.go	Adds new game state machine + turn progression
engine/game_test.go	Adds game state machine unit tests
engine/ai.go	Adds greedy AI player
engine/ai_test.go	Adds AI unit tests
engine/client.go	Adds GameClient + LocalClient implementation
engine/client_test.go	Adds LocalClient tests
cli/ui.go	Adds Bubble Tea entrypoint for the TUI
cli/model.go	Implements rolling/choosing/game-over TUI flow
p2p/protocol.go	Adds length-prefixed JSON protocol + codecs
p2p/protocol_test.go	Adds protocol round-trip tests
p2p/host.go	Adds host-authority P2P game runner
p2p/host_test.go	Adds P2P host tests
p2p/guest.go	Adds RemoteClient guest implementation
p2p/guest_test.go	Adds guest/RemoteClient tests
p2p/e2e_test.go	Adds P2P full-game E2E tests (skipped in -short)
mcp/server.go	Adds MCP server exposing game tools
mcp/server_test.go	Adds MCP tool handler tests
mcp/e2e_test.go	Adds MCP full-game E2E tests (skipped in -short)
match/client.go	Adds WebSocket matchmaking client
match/client_test.go	Adds matchmaking client tests
lambda/handler.go	Adds AWS Lambda matchmaking handler (DynamoDB + API GW)
lambda/handler_test.go	Adds Lambda handler unit tests with mocks
README.md	Updates installation/usage/architecture docs for v2
docs/superpowers/specs/2026-03-22-yatzcli-rebuild-design.md	Adds design spec for the rebuild
cmd/yatz/main.go	Adds cobra-based single-binary CLI entrypoint
CLAUDE.md	Adds repo usage + architecture notes for Claude Code
go.mod	Updates module path, Go version, and dependencies
go.sum	Updates dependency lockfile
.github/workflows/ci.yml	Adds unit + E2E GitHub Actions workflows
.github/workflows/release.yml	Adds tag-based GoReleaser workflow
.goreleaser.yml	Adds GoReleaser build/archive config
.gitignore	Ignores new built binary name (yatz)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Scorecard has only an unexported field (scores), so when engine.GameState is JSON-marshaled (e.g., in P2P state_update / game_start messages) the scorecard will serialize as {} and the guest will never see filled categories or totals. Consider making the field exported (and JSON-tagged) or implementing MarshalJSON/UnmarshalJSON so scorecards round-trip correctly over the wire.

[Copilot]

listen() writes to responseCh with a blocking send for every state_update. When the guest is waiting for turn_start (e.g., inside Score() after ending their turn), there may be no goroutine receiving from responseCh, so the listener can deadlock and stop processing further messages (including turn_start / game_over). Consider making these sends non-blocking (drop/overwrite when no waiter), or gating delivery to responseCh only when an action request is in-flight, while still updating lastState for UI.

[Copilot]

ReadMessage trusts the peer-provided length and allocates make([]byte, length) with no upper bound. A malicious/buggy peer can send a huge length prefix and trigger excessive memory allocation/DoS. Introduce a reasonable max message size (e.g., 1MB) and reject frames exceeding it before allocating/reading.

[Copilot]

Score() accepts any Category string and will happily fill unknown categories (since CalcScore returns 0 for unrecognized values). This allows clients (MCP/P2P) to send arbitrary category names, bypass normal scorecard filling, and potentially invalidate game results. Consider validating category is one of AllCategories (and ideally currently available for the player) before scoring.

[Copilot]

The hold_dice tool declares indices as a string parameter, but handleHoldDice reads it via GetIntSlice. This schema/handler mismatch can break real MCP clients that validate arguments against the tool schema. Define indices as an array of numbers/ints in the tool schema (instead of a stringified JSON array), or update the handler to parse a string.

[Copilot]

This spec says the guest receives turn_start at the beginning of every turn, but the current host implementation only sends turn_start when the guest’s turn begins (see Host.handleGuestTurn()). Either update the spec to match the implementation, or adjust the host to emit turn_start for host turns too.

[Copilot]

handleMessage trusts msg.Port from the client and blindly constructs endpoint := sourceIP + ":" + strconv.Itoa(msg.Port). This can produce invalid endpoints (0/negative/out-of-range ports) and could be abused to poison matchmaking results. Consider validating Port is within 1-65535 (and optionally rejecting empty sourceIP) before storing/notifying.

[edge2992]

Code review

Found 5 issues:

1. Scorecard unexported field prevents JSON serialization over P2P. The Scorecard struct uses an unexported scores field (map[Category]*int). Since encoding/json cannot access unexported fields, all scorecard data is silently lost when GameState is serialized over the P2P protocol. No custom MarshalJSON/UnmarshalJSON methods are defined. The P2P round-trip test only checks CurrentPlayer, Dice, and player count -- never scorecard contents -- masking this bug.

yatzcli/engine/scorecard.go

Lines 5 to 7 in 41904e1

	}
	func NewScorecard() Scorecard {

2. RemoteClient deadlocks during host's turn due to responseCh buffer overflow. responseCh has buffer size 1. During the host's turn, each HostGameClient.Roll(), Hold(), and Score() sends a state_update to the guest. After the guest's Score() returns, nothing consumes from responseCh. A typical host turn (Roll + Score = 2 state_updates) fills the buffer on the first, blocks the listener goroutine on the second, and prevents turn_start from ever being delivered to turnCh. The guest hangs forever. This deadlock occurs in every P2P game after the first guest turn. The E2E test avoids this because the host plays directly via game.Roll()/game.Score() (bypassing HostGameClient), so no state_updates are sent during host turns in the test.

yatzcli/p2p/guest.go

Lines 86 to 95 in 41904e1

	rc := &RemoteClient{
	conn: conn,
	lastState: &sp.State,
	playerID: "player-1",
	playerName: name,
	responseCh: make(chan responseResult, 1),
	turnCh: make(chan *engine.GameState, 1),
	gameOverCh: make(chan *engine.GameState, 1),
	doneCh: make(chan struct{}),
	}

yatzcli/p2p/host.go

Lines 30 to 50 in 41904e1

	func (h *HostGameClient) Roll() (*engine.GameState, error) {
	gs, err := h.local.Roll()
	if err != nil {
	return nil, err
	}
	if err := h.host.sendStateUpdate(*gs); err != nil {
	return nil, fmt.Errorf("send state update: %w", err)
	}
	return gs, nil
	}
	func (h *HostGameClient) Hold(indices []int) (*engine.GameState, error) {
	gs, err := h.local.Hold(indices)
	if err != nil {
	return nil, err
	}
	if err := h.host.sendStateUpdate(*gs); err != nil {
	return nil, fmt.Errorf("send state update: %w", err)
	}
	return gs, nil
	}

3. release.yml uses Go 1.22 but go.mod requires Go 1.24.2. The release workflow will fail to build because Go 1.22 cannot compile code requiring Go 1.24.2. ci.yml was updated to 1.24 but release.yml was not.

yatzcli/.github/workflows/release.yml

Lines 16 to 18 in 41904e1

	with:
	go-version: '1.22'
	- uses: goreleaser/goreleaser-action@v6

4. MCP hold_dice tool schema/handler type mismatch. The tool defines indices as mcp.WithString (string type) but the handler retrieves it with req.GetIntSlice(). Tests pass because the in-process client sends native Go slices that bypass JSON schema validation. A real MCP client (e.g., Claude Desktop) would send "[0,2,4]" as a string per the schema, and GetIntSlice would return nil, causing the tool to always error.

yatzcli/mcp/server.go

Lines 44 to 48 in 41904e1

	holdDiceTool := mcp.NewTool("hold_dice",
	mcp.WithDescription("Hold specified dice and reroll the others. Indices are 0-4."),
	mcp.WithString("indices", mcp.Required(), mcp.Description("JSON array of dice indices to hold, e.g. [0,2,4]")),
	)
	s.AddTool(holdDiceTool, gs.handleHoldDice)

yatzcli/mcp/server.go

Lines 114 to 118 in 41904e1

	}
	indices := req.GetIntSlice("indices", nil)
	if indices == nil {
	return mcp.NewToolResultError("indices parameter is required (JSON array of ints 0-4)"), nil
	}

5. No maximum message size in ReadMessage. ReadMessage reads a uint32 length prefix and allocates make([]byte, length) without an upper bound. A malicious peer can send a length up to ~4GB, causing OOM. This is on a TCP network boundary accepting untrusted input. (CLAUDE.md says "Validate inputs at system boundaries; trust internal code.")

yatzcli/p2p/protocol.go

Lines 67 to 78 in 41904e1

	func ReadMessage(r io.Reader) (*Message, error) {
	var length uint32
	if err := binary.Read(r, binary.BigEndian, &length); err != nil {
	return nil, fmt.Errorf("read length: %w", err)
	}
	data := make([]byte, length)
	if _, err := io.ReadFull(r, data); err != nil {
	return nil, fmt.Errorf("read payload: %w", err)
	}
	var msg Message
	if err := json.Unmarshal(data, &msg); err != nil {
	return nil, fmt.Errorf("unmarshal message: %w", err)

---

Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[edge2992]

Code review (re-review after fixes)

Previous 5 issues were all addressed. Found 2 remaining issues:

1. MsgError not guarded by expectResponse -- the deadlock fix for MsgStateUpdate (adding expectResponse check before sending to responseCh) was not applied to the MsgError case. If the host sends an error while expectResponse is false, the listener goroutine blocks on the buffered channel, causing deadlock. Same pattern as the original MsgStateUpdate bug that was just fixed.

yatzcli/p2p/guest.go

Lines 147 to 155 in adae23b

	case MsgError:
	ep, err := DecodeError(msg)
	if err != nil {
	rc.responseCh <- responseResult{err: fmt.Errorf("decode error response: %w", err)}
	continue
	}
	rc.responseCh <- responseResult{err: fmt.Errorf("%s", ep.Message)}

2. DynamoDB Scan uses Limit: 1 with FilterExpression: "PlayerID <> :self". DynamoDB applies Limit before the filter expression, so if the first scanned item happens to be the requesting player, the result is 0 items even when other waiting players exist. This causes matchmaking to intermittently fail.

yatzcli/lambda/handler.go

Lines 131 to 140 in adae23b

	// Scan for a waiting player (exclude self)
	scanOut, err := h.db.Scan(ctx, &dynamodb.ScanInput{
	TableName: aws.String(h.table),
	FilterExpression: aws.String("PlayerID <> :self"),
	ExpressionAttributeValues: map[string]types.AttributeValue{
	":self": &types.AttributeValueMemberS{Value: connectionID},
	},
	Limit: aws.Int32(1),
	})
	if err != nil {

---

Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[edge2992]

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}