dnshelper/specs/001-safety-reliability-refactor/plan.md

# Implementation Plan: Safety, Reliability & Idiomatic Refactor

**Branch**: `001-safety-reliability-refactor` | **Date**: 2026-03-03 | **Spec**: [spec.md](spec.md)
**Input**: Feature specification from `/specs/001-safety-reliability-refactor/spec.md`

## Summary

Refactor the dns-helper CLI tool to address critical safety and reliability bugs — primarily the unsafe direct-write to the hosts file — and to modernize the codebase for idiomatic Go 1.20 patterns. Key changes: implement atomic write (temp file + rename) with pre-write backup, replace the `github.com/miekg/dns` dependency with `golang.org/x/net/dns/dnsmessage`, fix CNAME-chain duplicate IP bug, fix multi-host deletion bug, add input validation and proper error handling, add lock file for concurrency safety, remove dead code, and restructure the project into clear packages with testable interfaces per TDD.

## Technical Context

**Language/Version**: Go 1.20 (maximum; currently go.mod says 1.18, must be updated to 1.20)  
**Primary Dependencies**: Go standard library, `golang.org/x/net/dns/dnsmessage` (replacing `github.com/miekg/dns`)  
**Storage**: Local filesystem (system hosts file, backup files, lock file)  
**Testing**: `go test ./...` with TDD (Red-Green-Refactor); interfaces for filesystem and DNS to enable fakes/stubs  
**Target Platform**: Windows, Linux, macOS (cross-compiled single binary)  
**Project Type**: CLI tool (single binary, no config files)  
**Performance Goals**: N/A (single-shot CLI; DNS timeout default 5s)  
**Constraints**: Go 1.20 ceiling (no 1.21+ features); zero unapproved external dependencies; hosts file must never be corrupted  
**Scale/Scope**: ~500 LOC current, single-purpose utility; 6 source files + 3 platform-specific files

## Constitution Check

*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.*

### Principle I: System File Safety
- **Status**: VIOLATION IN CURRENT CODE — `writeOverContentInFile()` truncates the hosts file directly (`O_WRONLY|O_TRUNC`). No backup is created. A crash during write leaves the file corrupted.
- **Plan compliance**: FR-001 (atomic write via temp+rename) and FR-002 (pre-write backup) directly address this. PASS after implementation.

### Principle II: Standard Library First
- **Status**: VIOLATION IN CURRENT CODE — `github.com/miekg/dns` is an external dependency outside approved sources. The constitution notes it as a "grandfather clause" legacy dependency with preferred migration.
- **Plan compliance**: FR-014 replaces `miekg/dns` with `golang.org/x/net/dns/dnsmessage` (approved source). PASS after implementation.

### Principle III: Go 1.20 Compatibility
- **Status**: VIOLATION IN CURRENT CODE — `go.mod` specifies `go 1.18`. While this is compatible, the spec requires updating to `go 1.20` (FR-010).
- **Plan compliance**: Update `go.mod` to `go 1.20`, verify all code and dependencies are compatible. Verify `golang.org/x/net/dns/dnsmessage` is available in a Go 1.20-compatible version. PASS after implementation.

### Principle IV: Idiomatic Error Handling
- **Status**: VIOLATION IN CURRENT CODE — `fileoperations_linux.go` and `fileoperations_darwin.go` call `term()` (which calls `os.Exit`) instead of returning errors. `dns.go` prints errors directly via `fmt.Println` instead of returning them. `main.go` uses `defer os.Exit(1)` in `init()`.
- **Plan compliance**: FR-008 requires all platform code to return errors. All functions will return errors to callers; only `main()` calls `os.Exit`. PASS after implementation.

### Principle V: Simplicity and Single Purpose
- **Status**: VIOLATION IN CURRENT CODE — Dead code exists: unused `ping` subcommand FlagSet, unused `IsNew` field in `workingData`, `fileExists` in shared scope duplicated/unused, `slicesEqual` may be unused, commented-out debug code in `dataprep.go`.
- **Plan compliance**: FR-009 removes all dead code. Tool remains a single-binary CLI. PASS after implementation.

### Principle VI: Test-Driven Development
- **Status**: VIOLATION IN CURRENT CODE — Zero test files exist. No `*_test.go` files anywhere.
- **Plan compliance**: All new/refactored code will be developed with TDD. Functions interacting with filesystem/DNS will accept interfaces for testability. PASS after implementation.

### Gate Evaluation
All six constitution principles have violations in the current code. All violations are explicitly addressed by the feature spec requirements (FR-001 through FR-016). No unjustified violations exist. **GATE: PASS** — proceed to Phase 0.

## Project Structure

### Documentation (this feature)

```text
specs/001-safety-reliability-refactor/
├── plan.md              # This file
├── research.md          # Phase 0 output
├── data-model.md        # Phase 1 output
├── quickstart.md        # Phase 1 output
├── contracts/           # Phase 1 output
└── tasks.md             # Phase 2 output (created by /speckit.tasks)
```

### Source Code (repository root)

```text
dns-helper/
├── main.go                    # Entry point, CLI parsing, os.Exit
├── resolver/
│   ├── resolver.go            # DNS resolution interface + implementation
│   └── resolver_test.go       # Tests for DNS resolution
├── hostfile/
│   ├── hostfile.go            # Hosts file read/parse/write (atomic + backup)
│   ├── hostfile_test.go       # Tests for hosts file operations
│   ├── managed.go             # Managed block parsing, insertion, removal
│   └── managed_test.go        # Tests for managed block logic
├── platform/
│   ├── platform.go            # Interface for platform-specific operations
│   ├── platform_windows.go    # Windows hosts file location
│   ├── platform_linux.go      # Linux hosts file location
│   ├── platform_darwin.go     # macOS hosts file location
│   └── platform_test.go       # Platform tests
├── lockfile/
│   ├── lockfile.go            # Lock file acquisition/release
│   └── lockfile_test.go       # Lock file tests
├── go.mod
└── go.sum
```

**Structure Decision**: Reorganize from flat single-package into logical sub-packages (`resolver`, `hostfile`, `platform`, `lockfile`). Each package has a single responsibility and testable interfaces. `main.go` remains at root as the CLI entry point orchestrating the packages. This follows Go conventions for small-to-medium CLI tools and satisfies Constitution Principles V (simplicity) and VI (TDD/testability).

## Constitution Re-Check (Post Phase 1 Design)

*All six principles re-evaluated after design phase. No new violations introduced.*

| Principle | Status | Evidence |
|-----------|--------|----------|
| I. System File Safety | PASS | Atomic temp+rename in same directory (R-002). Pre-write backup in exe dir. Windows retry logic for sharing violations. Hosts file never opened for writing. |
| II. Standard Library First | PASS | `miekg/dns` replaced by `golang.org/x/net/dns/dnsmessage` (approved source, R-001). Lock file uses stdlib `os.OpenFile`. No unapproved dependencies remain. |
| III. Go 1.20 Compatibility | PASS | `golang.org/x/net` pinned to v0.35.0 (last Go 1.20-compatible release, R-006). `os.CreateTemp` available since Go 1.16. All APIs verified against Go 1.20. |
| IV. Idiomatic Error Handling | PASS | All package functions return errors. Platform implementations return errors to caller. Only `main()` calls `os.Exit()`. Errors include context (path, hostname, server). |
| V. Simplicity and Single Purpose | PASS | Four focused packages with clear single responsibilities. No speculative abstractions. Dead code eliminated. Remains single-binary CLI with subcommand+flags interface. |
| VI. Test-Driven Development | PASS | All packages have `_test.go` files in structure. Filesystem and DNS accessed through interfaces (R-007) enabling fakes/stubs. TDD workflow documented in quickstart. |

## Complexity Tracking

> No constitution violations remain unjustified. All violations are addressed by spec requirements.