Refactor code structure for improved readability and maintainability
This commit is contained in:
@@ -0,0 +1,29 @@
|
||||
# dns-helper Development Guidelines
|
||||
|
||||
Auto-generated from all feature plans. Last updated: 2026-03-03
|
||||
|
||||
## Active Technologies
|
||||
|
||||
- Go 1.20 (maximum; currently go.mod says 1.18, must be updated to 1.20) + Go standard library, `golang.org/x/net/dns/dnsmessage` (replacing `github.com/miekg/dns`) (001-safety-reliability-refactor)
|
||||
|
||||
## Project Structure
|
||||
|
||||
```text
|
||||
src/
|
||||
tests/
|
||||
```
|
||||
|
||||
## Commands
|
||||
|
||||
# Add commands for Go 1.20 (maximum; currently go.mod says 1.18, must be updated to 1.20)
|
||||
|
||||
## Code Style
|
||||
|
||||
Go 1.20 (maximum; currently go.mod says 1.18, must be updated to 1.20): Follow standard conventions
|
||||
|
||||
## Recent Changes
|
||||
|
||||
- 001-safety-reliability-refactor: Added Go 1.20 (maximum; currently go.mod says 1.18, must be updated to 1.20) + Go standard library, `golang.org/x/net/dns/dnsmessage` (replacing `github.com/miekg/dns`)
|
||||
|
||||
<!-- MANUAL ADDITIONS START -->
|
||||
<!-- MANUAL ADDITIONS END -->
|
||||
@@ -59,6 +59,8 @@ Load only the minimal necessary context from each artifact:
|
||||
- Phase grouping
|
||||
- Parallel markers [P]
|
||||
- Referenced file paths
|
||||
- Context blocks (per-task): code signatures, implementation patterns, gotchas, acceptance signals
|
||||
- Phase Context summaries: files modified, key types/interfaces, codebase conventions
|
||||
|
||||
**From constitution:**
|
||||
|
||||
@@ -111,14 +113,26 @@ Focus on high-signal findings. Limit to 50 findings total; aggregate remainder i
|
||||
- Task ordering contradictions (e.g., integration tasks before foundational setup tasks without dependency note)
|
||||
- Conflicting requirements (e.g., one requires Next.js while other specifies Vue)
|
||||
|
||||
#### G. Implementation Self-Containment
|
||||
|
||||
**Why this matters**: Tasks will be executed by a less-capable implementing model (e.g., Sonnet) that cannot reliably infer implementation details from sparse descriptions. Every non-trivial task must be a standalone implementation brief.
|
||||
|
||||
- **Missing Context blocks**: Flag any non-trivial task (anything beyond adding a simple field or creating an empty file) that lacks an indented **Context** block beneath the checkbox line. Severity: HIGH.
|
||||
- **Missing code signatures**: Context blocks that describe interfaces, structs, or functions the task depends on but do NOT quote the actual signatures with fenced code blocks. Severity: HIGH.
|
||||
- **Missing implementation pattern**: Context blocks that say *what* to build but not *how* — no code skeleton, no pseudocode, no reference to an analogous pattern elsewhere in the codebase. Severity: MEDIUM.
|
||||
- **Missing acceptance signal**: Tasks without a concrete, verifiable "done" statement (e.g., a specific test command, a build command, or observable output). Severity: MEDIUM.
|
||||
- **Missing gotchas/constraints**: Tasks that touch complex logic (edge cases, platform constraints, performance-sensitive code) but whose Context block omits known pitfalls from the spec or research documents. Severity: MEDIUM.
|
||||
- **Missing Phase Context summary**: Phases that lack the opening block listing files modified, key types, and codebase conventions. Severity: LOW.
|
||||
- **Placeholder signatures**: Context blocks that contain generic or assumed code signatures not verified against the actual codebase (e.g., `func DoSomething()` when the real signature has different parameters). Severity: HIGH.
|
||||
|
||||
### 5. Severity Assignment
|
||||
|
||||
Use this heuristic to prioritize findings:
|
||||
|
||||
- **CRITICAL**: Violates constitution MUST, missing core spec artifact, or requirement with zero coverage that blocks baseline functionality
|
||||
- **HIGH**: Duplicate or conflicting requirement, ambiguous security/performance attribute, untestable acceptance criterion
|
||||
- **MEDIUM**: Terminology drift, missing non-functional task coverage, underspecified edge case
|
||||
- **LOW**: Style/wording improvements, minor redundancy not affecting execution order
|
||||
- **HIGH**: Duplicate or conflicting requirement, ambiguous security/performance attribute, untestable acceptance criterion, non-trivial task missing Context block or code signatures (blocks implementation by less-capable model)
|
||||
- **MEDIUM**: Terminology drift, missing non-functional task coverage, underspecified edge case, Context block missing implementation pattern or acceptance signal
|
||||
- **LOW**: Style/wording improvements, minor redundancy not affecting execution order, missing Phase Context summary
|
||||
|
||||
### 6. Produce Compact Analysis Report
|
||||
|
||||
@@ -145,9 +159,12 @@ Output a Markdown report (no file writes) with the following structure:
|
||||
|
||||
- Total Requirements
|
||||
- Total Tasks
|
||||
- Non-Trivial Tasks (tasks requiring Context blocks)
|
||||
- Coverage % (requirements with >=1 task)
|
||||
- Context Completeness % (non-trivial tasks with a complete Context block containing: code signatures, implementation pattern, and acceptance signal)
|
||||
- Ambiguity Count
|
||||
- Duplication Count
|
||||
- Self-Containment Issues Count (from Detection Pass G)
|
||||
- Critical Issues Count
|
||||
|
||||
### 7. Provide Next Actions
|
||||
|
||||
@@ -25,14 +25,14 @@ You **MUST** consider the user input before proceeding (if not empty).
|
||||
|
||||
2. **Load design documents**: Read from FEATURE_DIR:
|
||||
- **Required**: plan.md (tech stack, libraries, structure), spec.md (user stories with priorities)
|
||||
- **Optional**: data-model.md (entities), contracts/ (interface contracts), research.md (decisions), quickstart.md (test scenarios)
|
||||
- **Optional**: data-model.md (entities), contracts/ (API endpoints), research.md (decisions), quickstart.md (test scenarios)
|
||||
- Note: Not all projects have all documents. Generate tasks based on what's available.
|
||||
|
||||
3. **Execute task generation workflow**:
|
||||
- Load plan.md and extract tech stack, libraries, project structure
|
||||
- Load spec.md and extract user stories with their priorities (P1, P2, P3, etc.)
|
||||
- If data-model.md exists: Extract entities and map to user stories
|
||||
- If contracts/ exists: Map interface contracts to user stories
|
||||
- If contracts/ exists: Map endpoints to user stories
|
||||
- If research.md exists: Extract decisions for setup tasks
|
||||
- Generate tasks organized by user story (see Task Generation Rules below)
|
||||
- Generate dependency graph showing user story completion order
|
||||
@@ -64,6 +64,101 @@ Context for task generation: $ARGUMENTS
|
||||
|
||||
The tasks.md should be immediately executable - each task must be specific enough that an LLM can complete it without additional context.
|
||||
|
||||
## Implementation Context Requirements
|
||||
|
||||
**CRITICAL**: The tasks.md will be executed by a DIFFERENT model than the one generating it. The generating model (you) has deep reasoning capability and can infer implementation details from sparse descriptions. The implementing model does NOT — it needs explicit, self-contained context embedded directly in each task. Every task must be a standalone implementation brief, not a summary that requires the reader to independently analyze the codebase.
|
||||
|
||||
### Per-Task Context Blocks
|
||||
|
||||
For every non-trivial task (anything beyond creating an empty file or adding a simple field), include an indented **Context** block immediately below the task checkbox line. This block MUST contain:
|
||||
|
||||
1. **Existing code references**: Quote the exact current function signatures, struct definitions, or interface methods that the task modifies or depends on. Use fenced code blocks with the language identifier.
|
||||
|
||||
2. **Target file state**: Describe what already exists in the target file that the implementer needs to know about (imports, adjacent functions, package conventions).
|
||||
|
||||
3. **Expected implementation pattern**: Show a concrete code skeleton or pseudocode of what the implementation should look like. Reference similar patterns already in the codebase by quoting them. For example: "Follow the same pattern as `computeDatasetAnalytics()` in `engine.go` lines 245-310, which iterates entries and accumulates stats."
|
||||
|
||||
4. **Key decisions and gotchas**: Non-obvious constraints, edge cases, or design decisions from the spec/research that affect this task. Example: "Must use `int64` not `int` for file sizes to handle >2GB files on 32-bit systems" or "The profile may have RecursiveAnalysis disabled — check `opts.Profile.RecursiveAnalysis` before accumulating subdirectory maps."
|
||||
|
||||
5. **Acceptance signal**: A concrete, verifiable statement of what "done" looks like. Example: "`go test ./internal/engine/... -run TestRegistryResolve` passes with all 6 subcases green."
|
||||
|
||||
### Example Task WITH Context (REQUIRED format for all non-trivial tasks)
|
||||
|
||||
```markdown
|
||||
- [ ] T009 [US1] Create template analysis module in `internal/engine/module_template.go`
|
||||
|
||||
**Context**:
|
||||
- **Implements**: `AnalysisModule` interface from `internal/engine/module.go`:
|
||||
```go
|
||||
type AnalysisModule interface {
|
||||
Name() string
|
||||
Description() string
|
||||
ProcessEntry(entry *types.ManifestEntry)
|
||||
Finalize(ctx *FinalizationContext) (*ModuleResult, error)
|
||||
}
|
||||
```
|
||||
- **Depends on**: `ContainerMatcher` from `internal/discovery/matcher.go` — call `matcher.Match(entry.Path)` to identify containers. The matcher is initialized from `profile.ContainerPatterns` (see `engine.go:initContainerMatcher()`).
|
||||
- **Accumulation pattern**: Follow the same accumulator pattern as `DatasetAnalyticsAccumulator` in `pipeline.go` — use maps keyed by container path to accumulate file counts, total sizes, and date ranges incrementally during `ProcessEntry()`.
|
||||
- **Key struct fields to accumulate**:
|
||||
```go
|
||||
type templateModule struct {
|
||||
matcher *discovery.ContainerMatcher
|
||||
profile *config.Profile
|
||||
accumulators map[string]*containerAccumulator // keyed by container path
|
||||
seen map[string]bool // for nesting exclusion
|
||||
subdirs map[string]map[string]bool // container -> subdirs (only if recursive enabled)
|
||||
filesByPath map[string]map[string]int // container -> subdir -> file count
|
||||
sizeByPath map[string]map[string]int64 // container -> subdir -> total size
|
||||
}
|
||||
```
|
||||
- **Gotcha**: Nesting exclusion — if `/projects/foo` is a container, `/projects/foo/bar` should NOT also be identified as a separate container even if it matches. Use the `seen` set: before adding a new container, check if any prefix of its path is already in `seen`.
|
||||
- **Acceptance**: Module registers successfully, `ProcessEntry()` populates accumulators for test entries, `Name()` returns `"template"`.
|
||||
```
|
||||
|
||||
### Example Task WITHOUT Context (WRONG — never do this)
|
||||
|
||||
```markdown
|
||||
- [ ] T009 [US1] Create template analysis module in `internal/engine/module_template.go` — implement AnalysisModule interface, accumulate container stats during ProcessEntry(), handle nesting exclusion
|
||||
```
|
||||
|
||||
The second example forces the implementing model to independently figure out what `AnalysisModule` looks like, how `ContainerMatcher` works, what fields to accumulate, and how nesting exclusion works. This results in incorrect or incomplete implementations.
|
||||
|
||||
### Per-Phase Context Summary
|
||||
|
||||
At the start of each phase (after the **Goal** line), include a **Phase Context** block listing:
|
||||
|
||||
- **Files modified in this phase**: Full paths with a one-line description of each file's current purpose
|
||||
- **Key types/interfaces used**: Quoted signatures of types that tasks in this phase depend on
|
||||
- **Codebase conventions to follow**: Naming patterns, error handling style, import grouping, or test patterns observed in the project (reference specific files as examples)
|
||||
|
||||
### Context Sourcing Strategy — Plan Artifacts First
|
||||
|
||||
The design documents produced by `/speckit.plan` (contracts, data-model, research, quickstart) already contain most of the information that Context blocks need: interface signatures, struct definitions, design decisions, gotchas, error messages, and acceptance scenarios. **Use these artifacts as your primary source for Context blocks instead of re-reading source files.**
|
||||
|
||||
**For tasks that CREATE new files** (new modules, new types, new test files):
|
||||
- Pull interface signatures, struct definitions, and method contracts from **contracts/** and **data-model.md** — these already contain the exact code the implementer needs
|
||||
- Pull design decisions, accumulation patterns, and gotchas from **research.md** — cite the specific research item (e.g., "See R3: Container Identification During Streaming")
|
||||
- Pull acceptance scenarios from **quickstart.md** — map CLI examples to verifiable acceptance signals
|
||||
- **Do NOT read source files** unless the plan artifacts reference an existing pattern that needs to be matched (e.g., "follows the existing `plugin.Registry` pattern") — in that case, read only the referenced file to quote the specific pattern
|
||||
|
||||
**For tasks that MODIFY existing files** (adding fields, rewiring functions, updating existing logic):
|
||||
- Still use plan artifacts for the **target design** (what the code should become)
|
||||
- Read the **specific function/struct being modified** from the source file to quote its **current state** — the implementer needs to see both "what exists now" and "what it should become"
|
||||
- Do NOT read the entire file — read only the function/struct being changed plus its immediate dependencies
|
||||
|
||||
**For test tasks**:
|
||||
- Read **one existing test file** in the same package to capture test conventions (table-driven patterns, setup/teardown helpers, assertion style). Quote a representative test function as a pattern to follow.
|
||||
- Pull test scenarios from contracts and quickstart — these define expected inputs/outputs
|
||||
|
||||
**Always**:
|
||||
- Read **copilot-instructions.md** or equivalent project guidelines (once, at the start of task generation) for language version constraints, banned patterns, build requirements, and protected code paths
|
||||
- If the project has a design system or style guide referenced in the plan, read it (once)
|
||||
|
||||
**Do NOT**:
|
||||
- Read full source files for every task — the plan artifacts already distill the relevant information
|
||||
- Re-derive information that exists in contracts or research — reference those documents directly
|
||||
- Generate tasks with placeholder signatures — every code reference must come from either plan artifacts or a targeted source file read
|
||||
|
||||
## Task Generation Rules
|
||||
|
||||
**CRITICAL**: Tasks MUST be organized by user story to enable independent implementation and testing.
|
||||
@@ -109,13 +204,13 @@ Every task MUST strictly follow this format:
|
||||
- Map all related components to their story:
|
||||
- Models needed for that story
|
||||
- Services needed for that story
|
||||
- Interfaces/UI needed for that story
|
||||
- Endpoints/UI needed for that story
|
||||
- If tests requested: Tests specific to that story
|
||||
- Mark story dependencies (most stories should be independent)
|
||||
|
||||
2. **From Contracts**:
|
||||
- Map each interface contract → to the user story it serves
|
||||
- If tests requested: Each interface contract → contract test task [P] before implementation in that story's phase
|
||||
- Map each contract/endpoint → to the user story it serves
|
||||
- If tests requested: Each contract → contract test task [P] before implementation in that story's phase
|
||||
|
||||
3. **From Data Model**:
|
||||
- Map each entity to the user story(ies) that need it
|
||||
|
||||
@@ -0,0 +1,210 @@
|
||||
<!--
|
||||
Sync Impact Report
|
||||
Version change: 1.0.0 → 1.1.0
|
||||
Modified principles: None
|
||||
Added sections:
|
||||
- Principle VI: Test-Driven Development (new core principle)
|
||||
Modified sections:
|
||||
- Development Standards — Testing bullet upgraded from SHOULD to MUST;
|
||||
added TDD-specific guidance (test file conventions, coverage expectations)
|
||||
Removed sections: N/A
|
||||
Templates requiring updates:
|
||||
- .specify/templates/plan-template.md — ⚠ pending (Constitution Check
|
||||
section references "[Gates determined based on constitution file]";
|
||||
will be populated when /speckit.plan runs against this constitution)
|
||||
- .specify/templates/spec-template.md — ✅ no updates required
|
||||
- .specify/templates/tasks-template.md — ✅ no updates required
|
||||
Follow-up TODOs: None
|
||||
-->
|
||||
|
||||
# DNS-Helper Constitution
|
||||
|
||||
## Core Principles
|
||||
|
||||
### I. System File Safety
|
||||
|
||||
The hosts file is a critical operating system resource. Corruption or
|
||||
data loss in this file disables all local name resolution and can render
|
||||
a machine unusable without manual intervention.
|
||||
|
||||
- The tool MUST NOT truncate, zero-out, or partially overwrite the hosts
|
||||
file at any point during its operation.
|
||||
- All writes to the hosts file MUST use an atomic pattern: write complete
|
||||
content to a temporary file in the same directory, then rename over the
|
||||
original.
|
||||
- A backup of the current hosts file MUST be created before any
|
||||
modification is committed.
|
||||
- If any step in the write pipeline fails, the original hosts file MUST
|
||||
remain unmodified.
|
||||
|
||||
**Rationale**: This tool exists to solve DNS reliability problems. If it
|
||||
introduces a new failure mode — destroying the hosts file — it is worse
|
||||
than the problem it solves.
|
||||
|
||||
### II. Standard Library First
|
||||
|
||||
The Go standard library MUST be the first choice for all functionality.
|
||||
External dependencies introduce supply-chain risk, version conflicts,
|
||||
and maintenance burden that are disproportionate for a small,
|
||||
single-purpose utility.
|
||||
|
||||
- All functionality MUST be implemented using the Go standard library
|
||||
unless a documented technical justification proves it insufficient.
|
||||
- When an external dependency is genuinely required, it MUST come from
|
||||
one of the following approved sources:
|
||||
- `golang.org/x/*` (official Go extended libraries)
|
||||
- `dev.emberkom.com/*` (Emberkom internal libraries)
|
||||
- Any other external dependency MUST have a written justification in the
|
||||
code review or commit message explaining: (a) what standard library
|
||||
alternative was considered, (b) why it was insufficient, and (c) what
|
||||
the external dependency provides that the stdlib does not.
|
||||
- Dependencies MUST be audited when upgraded to ensure no new transitive
|
||||
dependencies from unapproved sources are introduced.
|
||||
|
||||
**Rationale**: A single-binary CLI tool that modifies a system file must
|
||||
have a minimal and auditable dependency tree. Every external package is
|
||||
an attack surface and a maintenance liability.
|
||||
|
||||
### III. Go 1.20 Compatibility
|
||||
|
||||
This tool MUST compile and run correctly with Go 1.20. This version
|
||||
ceiling exists to maintain compatibility with legacy systems that cannot
|
||||
run newer Go runtimes.
|
||||
|
||||
- The `go.mod` file MUST specify `go 1.20` as the language version.
|
||||
- No language features, standard library APIs, or dependency versions
|
||||
exclusive to Go 1.21 or later are permitted.
|
||||
- Build and test pipelines MUST use Go 1.20 as the primary toolchain
|
||||
version to catch compatibility issues early.
|
||||
- If a dependency releases a version that requires Go 1.21+, the tool
|
||||
MUST pin to the last Go 1.20-compatible release of that dependency.
|
||||
|
||||
**Rationale**: The target deployment environments include older systems
|
||||
where upgrading the Go toolchain or OS is not feasible. Pinning the
|
||||
version ensures the tool remains deployable everywhere it is needed.
|
||||
|
||||
### IV. Idiomatic Error Handling
|
||||
|
||||
Errors MUST flow upward through return values. Functions MUST NOT
|
||||
terminate the process, call `os.Exit`, or invoke global exit helpers
|
||||
except at the top-level `main` function.
|
||||
|
||||
- Every function that can fail MUST return an `error` value.
|
||||
- Callers MUST inspect returned errors and either handle them, wrap them
|
||||
with additional context, or propagate them upward.
|
||||
- The `main` function is the only location permitted to call `os.Exit`
|
||||
or print a fatal error and terminate.
|
||||
- Platform-specific code (build-tagged files) MUST return errors to the
|
||||
caller rather than terminating directly.
|
||||
- Errors presented to the user MUST include enough context to understand
|
||||
what operation failed and why (e.g., file path, hostname, DNS server).
|
||||
|
||||
**Rationale**: Internal `os.Exit` calls make code untestable, prevent
|
||||
cleanup logic from running, and cause execution to continue past error
|
||||
points when deferred. Centralizing exit decisions in `main` produces
|
||||
predictable, debuggable control flow.
|
||||
|
||||
### V. Simplicity and Single Purpose
|
||||
|
||||
This tool does one thing: update the local hosts file with DNS entries
|
||||
resolved from a specified DNS server. Every design decision MUST favor
|
||||
the simplest approach that satisfies the requirement.
|
||||
|
||||
- The tool MUST remain a single-binary CLI with no external
|
||||
configuration files, services, or daemons.
|
||||
- Features MUST be justified by a concrete user need, not speculative
|
||||
future use. YAGNI (You Aren't Gonna Need It) applies.
|
||||
- Dead code, unused exports, and speculative abstractions MUST be
|
||||
removed. If code is not called, it does not belong in the repository.
|
||||
- The CLI interface MUST remain straightforward: subcommand + flags.
|
||||
No interactive prompts, no TUI, no config file parsing.
|
||||
|
||||
**Rationale**: Complexity is the enemy of reliability. A tool that
|
||||
modifies a critical system file must be small enough to reason about
|
||||
completely and audit quickly.
|
||||
|
||||
### VI. Test-Driven Development
|
||||
|
||||
All new and refactored code MUST be developed using a test-driven
|
||||
development (TDD) approach. Tests are not an afterthought — they are
|
||||
the specification that drives implementation.
|
||||
|
||||
- Tests MUST be written before the implementation code they verify.
|
||||
The Red-Green-Refactor cycle is mandatory: write a failing test,
|
||||
write the minimum code to make it pass, then refactor.
|
||||
- Every exported function and every function with non-trivial logic
|
||||
MUST have corresponding test coverage.
|
||||
- Functions that interact with external resources (filesystem, DNS,
|
||||
network) MUST be designed to accept interfaces or parameters that
|
||||
enable testing with fakes, stubs, or in-memory implementations.
|
||||
Direct calls to OS-level I/O from business logic are prohibited.
|
||||
- Platform-specific code MUST have tests that verify the function
|
||||
contract (correct return values, correct error conditions) on each
|
||||
supported platform.
|
||||
- Test files MUST follow Go conventions: `*_test.go` in the same
|
||||
package as the code under test.
|
||||
- `go test ./...` MUST pass with zero failures before code is merged.
|
||||
|
||||
**Rationale**: This tool modifies a critical system file. Untested code
|
||||
is untrustworthy code. TDD ensures that every behavior is verified,
|
||||
every error path is exercised, and regressions are caught immediately.
|
||||
It also enforces the interface-based design required by Principle IV
|
||||
(idiomatic error handling) and makes the codebase safe to refactor.
|
||||
|
||||
## Dependency Governance
|
||||
|
||||
This section codifies the external dependency policy from Principle II
|
||||
into an auditable process.
|
||||
|
||||
| Source | Status | Condition |
|
||||
|--------|--------|-----------|
|
||||
| Go standard library | Always permitted | N/A |
|
||||
| `golang.org/x/*` | Permitted | Must solve a problem stdlib cannot |
|
||||
| `dev.emberkom.com/*` | Permitted | Must solve a problem stdlib cannot |
|
||||
| All other sources | Prohibited by default | Requires written justification, code review approval, and an entry in this table |
|
||||
|
||||
### Currently Approved External Dependencies
|
||||
|
||||
| Module | Justification |
|
||||
|--------|---------------|
|
||||
| `github.com/miekg/dns` | The Go standard library `net` package does not support querying a specific DNS server by address. This tool's core purpose requires directing queries to a user-specified resolver, which `net.Resolver` cannot do without low-level `Dialer` workarounds that replicate what `miekg/dns` provides. This is a legacy dependency approved under a grandfather clause — if a `golang.org/x` or stdlib alternative becomes viable under Go 1.20, migration is preferred. |
|
||||
|
||||
## Development Standards
|
||||
|
||||
- **Formatting**: All Go source files MUST be formatted with `gofmt`.
|
||||
No exceptions.
|
||||
- **Vet/Lint**: `go vet` MUST pass with zero findings before code is
|
||||
merged. Additional linting (e.g., `staticcheck`) is encouraged.
|
||||
- **Testing**: All new and refactored code MUST have test coverage
|
||||
written before the implementation (TDD). Functions that interact with
|
||||
the filesystem or DNS MUST be designed to accept interfaces or
|
||||
parameters that allow testing with fakes/stubs. `go test ./...` MUST
|
||||
pass with zero failures before merge. See Principle VI for the full
|
||||
TDD policy.
|
||||
- **Build tags**: Platform-specific code MUST use Go build tags
|
||||
(`//go:build windows`, etc.) and MUST implement identical function
|
||||
signatures across all supported platforms.
|
||||
- **Commit messages**: Follow conventional commit format
|
||||
(e.g., `fix:`, `feat:`, `refactor:`, `docs:`).
|
||||
|
||||
## Governance
|
||||
|
||||
This constitution is the authoritative source of project standards for
|
||||
DNS-Helper. It supersedes informal conventions, ad-hoc decisions, and
|
||||
prior practices that conflict with its contents.
|
||||
|
||||
- **Amendment process**: Any change to this constitution MUST be
|
||||
documented with a version bump, a rationale for the change, and an
|
||||
update to the Sync Impact Report at the top of this file.
|
||||
- **Versioning policy**: This constitution follows semantic versioning.
|
||||
MAJOR for principle removals or redefinitions, MINOR for new
|
||||
principles or materially expanded guidance, PATCH for clarifications
|
||||
and non-semantic refinements.
|
||||
- **Compliance review**: All pull requests and code reviews MUST verify
|
||||
that changes comply with the principles defined here. Violations MUST
|
||||
be resolved before merge, or the constitution MUST be amended first.
|
||||
- **Dependency audits**: When dependencies are added or upgraded, the
|
||||
reviewer MUST verify compliance with Principle II and the Dependency
|
||||
Governance table above.
|
||||
|
||||
**Version**: 1.1.0 | **Ratified**: 2026-03-03 | **Last Amended**: 2026-03-03
|
||||
@@ -12,6 +12,8 @@ description: "Task list template for feature implementation"
|
||||
|
||||
**Organization**: Tasks are grouped by user story to enable independent implementation and testing of each story.
|
||||
|
||||
**Self-Containment**: Tasks will be executed by a DIFFERENT model than the one generating them. The generating model has deep reasoning capability and can infer implementation details from sparse descriptions. The implementing model does NOT — it needs explicit, self-contained context embedded directly in each task. Every non-trivial task must include an indented **Context** block (see examples below).
|
||||
|
||||
## Format: `[ID] [P?] [Story] Description`
|
||||
|
||||
- **[P]**: Can run in parallel (different files, no dependencies)
|
||||
@@ -48,6 +50,11 @@ description: "Task list template for feature implementation"
|
||||
|
||||
**Purpose**: Project initialization and basic structure
|
||||
|
||||
**Phase Context**:
|
||||
- **Files modified in this phase**: `package.json` (project manifest), `src/` (directory structure), `.eslintrc` (linting config)
|
||||
- **Key types/interfaces used**: None yet — this phase creates the foundation
|
||||
- **Codebase conventions to follow**: [Reference project guidelines or existing patterns]
|
||||
|
||||
- [ ] T001 Create project structure per implementation plan
|
||||
- [ ] T002 Initialize [language] project with [framework] dependencies
|
||||
- [ ] T003 [P] Configure linting and formatting tools
|
||||
@@ -79,6 +86,11 @@ Examples of foundational tasks (adjust based on your project):
|
||||
|
||||
**Independent Test**: [How to verify this story works on its own]
|
||||
|
||||
**Phase Context**:
|
||||
- **Files modified in this phase**: [Full paths with one-line description of each file's current purpose]
|
||||
- **Key types/interfaces used**: [Quoted signatures of types that tasks in this phase depend on]
|
||||
- **Codebase conventions to follow**: [Naming patterns, error handling style, import grouping, test patterns — reference specific files as examples]
|
||||
|
||||
### Tests for User Story 1 (OPTIONAL - only if tests requested) ⚠️
|
||||
|
||||
> **NOTE: Write these tests FIRST, ensure they FAIL before implementation**
|
||||
@@ -89,6 +101,14 @@ Examples of foundational tasks (adjust based on your project):
|
||||
### Implementation for User Story 1
|
||||
|
||||
- [ ] T012 [P] [US1] Create [Entity1] model in src/models/[entity1].py
|
||||
|
||||
**Context**:
|
||||
- **Existing code references**: [Quote the exact interface/base class this model must implement, with fenced code block]
|
||||
- **Target file state**: `src/models/` directory exists after T001; no other models present yet
|
||||
- **Expected implementation pattern**: [Show code skeleton or reference similar pattern in codebase, e.g., "Follow the same pattern as `BaseEntity` in `src/models/base.py` lines 10-45"]
|
||||
- **Key decisions and gotchas**: [Non-obvious constraints from spec/research, e.g., "Must use UUID for primary key per spec NFR-3" or "Field `name` has max length 255 per data-model.md"]
|
||||
- **Acceptance signal**: [Concrete verification, e.g., "`python -m pytest tests/unit/test_entity1.py` passes" or "File imports successfully with no errors"]
|
||||
|
||||
- [ ] T013 [P] [US1] Create [Entity2] model in src/models/[entity2].py
|
||||
- [ ] T014 [US1] Implement [Service] in src/services/[service].py (depends on T012, T013)
|
||||
- [ ] T015 [US1] Implement [endpoint/feature] in src/[location]/[file].py
|
||||
@@ -249,3 +269,6 @@ With multiple developers:
|
||||
- Commit after each task or logical group
|
||||
- Stop at any checkpoint to validate story independently
|
||||
- Avoid: vague tasks, same file conflicts, cross-story dependencies that break independence
|
||||
- **Every non-trivial task MUST include an indented Context block** with: existing code references (quoted signatures), target file state, expected implementation pattern (code skeleton or analogous pattern reference), key decisions/gotchas, and a concrete acceptance signal
|
||||
- **Every phase MUST include a Phase Context summary** listing: files modified, key types/interfaces used, and codebase conventions to follow
|
||||
- Context blocks should source information from plan artifacts (contracts, data-model, research, quickstart) first, and only read source files when the plan references an existing pattern that needs to be matched
|
||||
|
||||
@@ -0,0 +1,36 @@
|
||||
# Specification Quality Checklist: Safety, Reliability & Idiomatic Refactor
|
||||
|
||||
**Purpose**: Validate specification completeness and quality before proceeding to planning
|
||||
**Created**: 2026-03-03
|
||||
**Feature**: [spec.md](../spec.md)
|
||||
|
||||
## Content Quality
|
||||
|
||||
- [x] No implementation details (languages, frameworks, APIs)
|
||||
- [x] Focused on user value and business needs
|
||||
- [x] Written for non-technical stakeholders
|
||||
- [x] All mandatory sections completed
|
||||
|
||||
## Requirement Completeness
|
||||
|
||||
- [x] No [NEEDS CLARIFICATION] markers remain
|
||||
- [x] Requirements are testable and unambiguous
|
||||
- [x] Success criteria are measurable
|
||||
- [x] Success criteria are technology-agnostic (no implementation details)
|
||||
- [x] All acceptance scenarios are defined
|
||||
- [x] Edge cases are identified
|
||||
- [x] Scope is clearly bounded
|
||||
- [x] Dependencies and assumptions identified
|
||||
|
||||
## Feature Readiness
|
||||
|
||||
- [x] All functional requirements have clear acceptance criteria
|
||||
- [x] User scenarios cover primary flows
|
||||
- [x] Feature meets measurable outcomes defined in Success Criteria
|
||||
- [x] No implementation details leak into specification
|
||||
|
||||
## Notes
|
||||
|
||||
- FR-010 references "Go 1.20" as an explicit user constraint, not a technology recommendation — this is acceptable per the user's requirement for legacy system compatibility.
|
||||
- The spec references "atomic write pattern" and "CNAME chains" as behavioral descriptions, not implementation prescriptions. The planning phase will determine specific implementation approaches.
|
||||
- All checklist items pass. Spec is ready for `/speckit.clarify` or `/speckit.plan`.
|
||||
@@ -0,0 +1,229 @@
|
||||
# CLI Contract: dns-helper
|
||||
|
||||
**Feature Branch**: `001-safety-reliability-refactor`
|
||||
**Date**: 2026-03-03
|
||||
|
||||
## Overview
|
||||
|
||||
`dns-helper` is a single-binary CLI tool that updates the local hosts file with DNS entries resolved from a user-specified DNS server. It exposes three subcommands: `add`, `delete`, and a usage/help display on invalid input.
|
||||
|
||||
## Binary Name
|
||||
|
||||
```
|
||||
dns-helper[.exe]
|
||||
```
|
||||
|
||||
## Global Behavior
|
||||
|
||||
- Prints banner on every invocation: `DNSHelper v1.0`, `Copyright (c) 2024 Emberkom LLC`, blank line.
|
||||
- All error messages are printed to **stderr**.
|
||||
- Informational output (banner, summaries, warnings) is printed to **stdout**.
|
||||
- Exit code `0` on full success. Non-zero on any error or partial failure.
|
||||
|
||||
---
|
||||
|
||||
## Subcommand: `add` (aliases: `a`)
|
||||
|
||||
### Synopsis
|
||||
|
||||
```
|
||||
dns-helper add -host <hostnames> -server <dns-server>
|
||||
```
|
||||
|
||||
### Flags
|
||||
|
||||
| Flag | Required | Type | Description |
|
||||
|------|----------|------|-------------|
|
||||
| `-host` | Yes | string | Comma-separated list of hostnames to resolve and add |
|
||||
| `-server` | Yes | string | DNS server to use for resolution (hostname or IP) |
|
||||
|
||||
### Behavior
|
||||
|
||||
1. Validates that both `-host` and `-server` are provided and non-empty.
|
||||
2. Resolves each hostname against the specified DNS server (follows CNAME chains, deduplicates IPs).
|
||||
3. Acquires lock file.
|
||||
4. Reads the current hosts file and parses the managed block.
|
||||
5. Removes any existing managed entries for the specified hostnames.
|
||||
6. Adds new entries for all successfully resolved hostnames.
|
||||
7. Writes the updated hosts file atomically (temp file + rename, with pre-write backup).
|
||||
8. Releases lock file.
|
||||
9. Prints summary of changes.
|
||||
|
||||
### Output — Success (exit 0)
|
||||
|
||||
```
|
||||
DNSHelper v1.0
|
||||
Copyright (c) 2024 Emberkom LLC
|
||||
|
||||
Added 3 entries for host1.example.com
|
||||
Added 2 entries for host2.example.com
|
||||
```
|
||||
|
||||
### Output — Partial Failure (exit 1)
|
||||
|
||||
```
|
||||
DNSHelper v1.0
|
||||
Copyright (c) 2024 Emberkom LLC
|
||||
|
||||
Added 3 entries for host1.example.com
|
||||
Warning: failed to resolve host2.example.com: NXDOMAIN
|
||||
```
|
||||
|
||||
### Output — Validation Error (exit 1)
|
||||
|
||||
```
|
||||
DNSHelper v1.0
|
||||
Copyright (c) 2024 Emberkom LLC
|
||||
|
||||
Error: -host flag is required for the add command
|
||||
```
|
||||
|
||||
### Exit Codes
|
||||
|
||||
| Code | Meaning |
|
||||
|------|---------|
|
||||
| 0 | All hostnames resolved and written successfully |
|
||||
| 1 | Any error: validation failure, DNS failure, file write failure, partial resolution |
|
||||
|
||||
---
|
||||
|
||||
## Subcommand: `delete` (aliases: `d`, `del`)
|
||||
|
||||
### Synopsis
|
||||
|
||||
```
|
||||
dns-helper delete -host <hostnames>
|
||||
dns-helper delete all
|
||||
```
|
||||
|
||||
### Flags
|
||||
|
||||
| Flag | Required | Type | Description |
|
||||
|------|----------|------|-------------|
|
||||
| `-host` | Yes* | string | Comma-separated list of hostnames to remove (*not required when using `all`) |
|
||||
|
||||
### Behavior — Delete Specific Hosts
|
||||
|
||||
1. Validates that `-host` is provided and non-empty.
|
||||
2. Acquires lock file.
|
||||
3. Reads the current hosts file and parses the managed block.
|
||||
4. Removes all managed entries matching the specified hostnames.
|
||||
5. Writes the updated hosts file atomically.
|
||||
6. Releases lock file.
|
||||
7. Prints summary.
|
||||
|
||||
### Behavior — Delete All
|
||||
|
||||
1. Acquires lock file.
|
||||
2. Reads the current hosts file and parses the managed block.
|
||||
3. Removes the entire managed block (start marker through end marker).
|
||||
4. Writes the updated hosts file atomically.
|
||||
5. Releases lock file.
|
||||
6. Prints summary.
|
||||
|
||||
### Output — Success (exit 0)
|
||||
|
||||
```
|
||||
DNSHelper v1.0
|
||||
Copyright (c) 2024 Emberkom LLC
|
||||
|
||||
Removed 3 entries for host1.example.com
|
||||
```
|
||||
|
||||
### Output — No Entries Found (exit 0)
|
||||
|
||||
```
|
||||
DNSHelper v1.0
|
||||
Copyright (c) 2024 Emberkom LLC
|
||||
|
||||
No managed entries found for host1.example.com
|
||||
```
|
||||
|
||||
### Output — Delete All (exit 0)
|
||||
|
||||
```
|
||||
DNSHelper v1.0
|
||||
Copyright (c) 2024 Emberkom LLC
|
||||
|
||||
Removed all managed entries (5 entries removed)
|
||||
```
|
||||
|
||||
### Exit Codes
|
||||
|
||||
| Code | Meaning |
|
||||
|------|---------|
|
||||
| 0 | Deletion completed (or no entries to delete) |
|
||||
| 1 | Error: validation failure, file write failure, corrupt managed block |
|
||||
|
||||
---
|
||||
|
||||
## Invalid / No Subcommand
|
||||
|
||||
### Synopsis
|
||||
|
||||
```
|
||||
dns-helper
|
||||
dns-helper <invalid-subcommand>
|
||||
```
|
||||
|
||||
### Behavior
|
||||
|
||||
1. Prints usage information to stdout.
|
||||
2. Does **not** modify the hosts file.
|
||||
3. Exits with code 1.
|
||||
|
||||
### Output (exit 1)
|
||||
|
||||
```
|
||||
DNSHelper v1.0
|
||||
Copyright (c) 2024 Emberkom LLC
|
||||
|
||||
This utility will update the local hosts file with DNS entries obtained by the specified DNS server.
|
||||
Usage: dns-helper [add|delete] -host hostname [-server dns.example.com]
|
||||
Example:
|
||||
dns-helper add -host xyz.acme.com -server dns.example.com
|
||||
This will use dns.example.com to find and add all IP addresses for xyz.acme.com to the local hosts file.
|
||||
dns-helper delete -host hostname.example.com
|
||||
This will delete all entries for hostname.example.com from the local hosts file.
|
||||
dns-helper delete all
|
||||
This will delete all entries from the local hosts file that were added by this utility.
|
||||
Note: Adding a hostname will first remove all entries in the hosts file that match the same hostname.
|
||||
This utility will only remove entries from the hosts file that it added.
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Error Conditions
|
||||
|
||||
| Condition | Message (stderr) | Exit Code | Hosts File Modified? |
|
||||
|-----------|-------------------|-----------|---------------------|
|
||||
| No subcommand | Usage text (stdout) | 1 | No |
|
||||
| Unknown subcommand | Usage text (stdout) | 1 | No |
|
||||
| Missing `-host` (add) | `Error: -host flag is required for the add command` | 1 | No |
|
||||
| Missing `-server` (add) | `Error: -server flag is required for the add command` | 1 | No |
|
||||
| Empty `-host` value | `Error: -host flag is required for the add command` | 1 | No |
|
||||
| DNS resolution failure (all hosts) | `Error: failed to resolve <host>: <reason>` | 1 | No |
|
||||
| DNS resolution partial failure | Warning per host + entries for successful hosts written | 1 | Yes (partial) |
|
||||
| Hosts file not found | `Error: file does not exist: <path>` | 1 | No |
|
||||
| Permission denied | `Error: permission denied: <path>` | 1 | No |
|
||||
| Corrupt managed block | `Error: managed block is corrupt: <details>` | 1 | No |
|
||||
| Lock file unavailable | `Error: could not acquire lock file <path>: another instance may be running` | 1 | No |
|
||||
| Write failure | `Error: writing hosts file: <details>` | 1 | No (atomic write) |
|
||||
|
||||
---
|
||||
|
||||
## Hosts File Managed Block Format
|
||||
|
||||
```
|
||||
# DNSHelper <<-> START CONFIG
|
||||
1.2.3.4 hostname1.example.com
|
||||
5.6.7.8 hostname1.example.com
|
||||
9.10.11.12 hostname2.example.com
|
||||
# DNSHelper <->> END CONFIG
|
||||
```
|
||||
|
||||
- Start marker: `# DNSHelper <<-> START CONFIG`
|
||||
- End marker: `# DNSHelper <->> END CONFIG`
|
||||
- Each entry: `<IPv4>\t<hostname>` (tab-separated)
|
||||
- Entries are deduplicated by full line content.
|
||||
- Block is omitted entirely when there are no managed entries.
|
||||
@@ -0,0 +1,168 @@
|
||||
# Package Contracts: dns-helper
|
||||
|
||||
**Feature Branch**: `001-safety-reliability-refactor`
|
||||
**Date**: 2026-03-03
|
||||
|
||||
## Package: `resolver`
|
||||
|
||||
Handles DNS name resolution against a user-specified DNS server.
|
||||
|
||||
### Interface
|
||||
|
||||
```go
|
||||
// Resolver performs DNS lookups against a specific server.
|
||||
type Resolver interface {
|
||||
// LookupIP resolves a hostname to a list of IPv4 addresses using the
|
||||
// specified DNS server. Follows CNAME chains up to a maximum depth.
|
||||
// Returns deduplicated IPs. Returns an error if the hostname cannot
|
||||
// be resolved (NXDOMAIN, timeout, server unreachable, etc.).
|
||||
LookupIP(hostname, server string) ([]string, error)
|
||||
}
|
||||
```
|
||||
|
||||
### Behavior Contract
|
||||
|
||||
- Queries are sent over UDP to `<server>:53`.
|
||||
- The `hostname` is automatically converted to FQDN (trailing dot appended if missing).
|
||||
- An A query is sent first. If A records are returned, they are collected.
|
||||
- If only CNAME records are returned (no A records in the answer), the CNAME target is resolved recursively.
|
||||
- CNAME chain depth is limited to 10 levels. Exceeding this returns an error.
|
||||
- Results are deduplicated before return.
|
||||
- Timeout: 5 seconds per query.
|
||||
- On NXDOMAIN: returns `error` with descriptive message.
|
||||
- On server unreachable/timeout: returns `error` with descriptive message.
|
||||
- Response ID must match query ID; mismatches are treated as errors.
|
||||
- Truncated responses (TC flag set) are treated as errors (no TCP fallback for simplicity).
|
||||
|
||||
---
|
||||
|
||||
## Package: `hostfile`
|
||||
|
||||
Manages reading, parsing, modifying, and atomically writing the hosts file.
|
||||
|
||||
### Interface
|
||||
|
||||
```go
|
||||
// Manager handles hosts file operations.
|
||||
type Manager interface {
|
||||
// Read reads and parses the hosts file at the given path.
|
||||
// Returns an error if the file cannot be read or the managed block is corrupt.
|
||||
Read(path string) (*HostsFile, error)
|
||||
|
||||
// Write atomically writes the hosts file content to the given path.
|
||||
// Creates a backup before writing. Uses temp-file-and-rename pattern.
|
||||
// backupDir is the directory for the backup file (typically the exe directory).
|
||||
Write(hf *HostsFile, backupDir string) error
|
||||
}
|
||||
|
||||
// HostsFile represents the parsed content of a hosts file.
|
||||
type HostsFile struct {
|
||||
Path string
|
||||
OriginalContent []string
|
||||
PrefixContent []string
|
||||
ManagedContent []string
|
||||
PostfixContent []string
|
||||
HasManagedBlock bool
|
||||
}
|
||||
```
|
||||
|
||||
### Sub-package: Managed Block Operations
|
||||
|
||||
```go
|
||||
// AddEntries adds DNS entries to the managed content, removing any
|
||||
// existing entries for the same hostnames first. Returns updated content.
|
||||
func AddEntries(existing []string, entries []DNSEntry) []string
|
||||
|
||||
// RemoveByHostname removes all managed entries matching any of the
|
||||
// specified hostnames. Returns updated content.
|
||||
func RemoveByHostname(existing []string, hostnames []string) []string
|
||||
|
||||
// RemoveAll returns an empty slice (clears all managed entries).
|
||||
func RemoveAll() []string
|
||||
```
|
||||
|
||||
### Behavior Contract — Read
|
||||
|
||||
- Opens the file at `path` and reads all lines.
|
||||
- Scans for start marker (`# DNSHelper <<-> START CONFIG`) and end marker (`# DNSHelper <->> END CONFIG`).
|
||||
- If neither marker found: `HasManagedBlock = false`, all content goes to `PrefixContent`.
|
||||
- If both markers found in correct order: splits content into Prefix, Managed, Postfix.
|
||||
- **Corruption detection** (returns error):
|
||||
- Start marker without end marker.
|
||||
- End marker without start marker.
|
||||
- End marker before start marker.
|
||||
- Duplicate start or end markers.
|
||||
- Uses sentinel values (not zero-index) for marker detection.
|
||||
|
||||
### Behavior Contract — Write
|
||||
|
||||
1. Creates backup: copies current hosts file to `<backupDir>/hosts.bak.<YYYYMMDD>-<4hex>`.
|
||||
2. Assembles full file content: prefix + (managed block if non-empty) + postfix.
|
||||
3. Compares assembled content with original — skips write if identical.
|
||||
4. Creates temp file in same directory as hosts file: `.dns-helper-tmp-*`.
|
||||
5. Writes content to temp file.
|
||||
6. Sets temp file permissions to match original hosts file permissions.
|
||||
7. Calls `Sync()` on temp file.
|
||||
8. Closes temp file.
|
||||
9. Calls `os.Rename(temp, hostsPath)`.
|
||||
10. On rename success: deletes backup, returns nil.
|
||||
11. On rename failure (including Windows retry 2-3x with 200ms delay): cleans up temp file, returns error. Backup is cleaned up after confirming original is intact.
|
||||
|
||||
---
|
||||
|
||||
## Package: `platform`
|
||||
|
||||
Provides platform-specific hosts file location.
|
||||
|
||||
### Interface
|
||||
|
||||
```go
|
||||
// GetHostsFilePath returns the absolute path to the system hosts file.
|
||||
// Returns an error if the file does not exist or cannot be accessed.
|
||||
func GetHostsFilePath() (string, error)
|
||||
```
|
||||
|
||||
### Platform Implementations
|
||||
|
||||
| Platform | Path | Detection |
|
||||
|----------|------|-----------|
|
||||
| Windows | `%SystemRoot%\System32\drivers\etc\hosts` (fallback: `C:\Windows\...`) | `os.LookupEnv("SystemRoot")` |
|
||||
| Linux | `/etc/hosts` | Hardcoded |
|
||||
| macOS | `/etc/hosts` | Hardcoded |
|
||||
|
||||
### Behavior Contract
|
||||
|
||||
- Returns error to caller on file-not-found or access error.
|
||||
- **Must NOT** call `os.Exit()`, `log.Fatal()`, or any process-terminating function.
|
||||
- Error messages include the path that was checked.
|
||||
|
||||
---
|
||||
|
||||
## Package: `lockfile`
|
||||
|
||||
Serializes concurrent access to the hosts file.
|
||||
|
||||
### Interface
|
||||
|
||||
```go
|
||||
// Lock represents an acquired lock.
|
||||
type Lock interface {
|
||||
// Release deletes the lock file. Safe to call multiple times.
|
||||
Release() error
|
||||
}
|
||||
|
||||
// Acquire attempts to acquire a lock file in the specified directory.
|
||||
// Retries for up to maxWait on contention. Breaks stale locks older
|
||||
// than staleTimeout. Returns an error if the lock cannot be acquired.
|
||||
func Acquire(dir string) (Lock, error)
|
||||
```
|
||||
|
||||
### Behavior Contract
|
||||
|
||||
- Lock file path: `<dir>/.dns-helper.lock`.
|
||||
- Created atomically via `os.OpenFile` with `O_CREATE|O_EXCL`.
|
||||
- PID written to lock file for debugging (not used for staleness).
|
||||
- Staleness: lock file with `ModTime` older than 2 minutes is considered stale and removed.
|
||||
- Retry: up to 2 seconds total, 200ms between attempts.
|
||||
- Release: `os.Remove(lockPath)`. Idempotent (no error if already deleted).
|
||||
- Lock acquired **after** DNS resolution, released **after** file write (or on error).
|
||||
@@ -0,0 +1,160 @@
|
||||
# Data Model: Safety, Reliability & Idiomatic Refactor
|
||||
|
||||
**Feature Branch**: `001-safety-reliability-refactor`
|
||||
**Date**: 2026-03-03
|
||||
|
||||
## Entities
|
||||
|
||||
### HostsFile
|
||||
|
||||
Represents the system hosts file as parsed content sections.
|
||||
|
||||
| Field | Type | Description | Constraints |
|
||||
|-------|------|-------------|-------------|
|
||||
| Path | `string` | Absolute path to the hosts file | OS-specific; determined by platform package |
|
||||
| OriginalContent | `[]string` | Raw lines from the file as read | Immutable after read; used for change detection |
|
||||
| PrefixContent | `[]string` | Lines before the managed block start marker | Preserved verbatim on write |
|
||||
| ManagedContent | `[]string` | Lines between start and end markers (exclusive) | Modified by add/delete operations |
|
||||
| PostfixContent | `[]string` | Lines after the managed block end marker | Preserved verbatim on write |
|
||||
| HasManagedBlock | `bool` | Whether a valid managed block was found | `true` if both start and end markers present in order |
|
||||
|
||||
**Validation Rules:**
|
||||
- If start marker is present, end marker must also be present (and vice versa).
|
||||
- End marker must appear after start marker.
|
||||
- Duplicate start or end markers are not permitted.
|
||||
- Start marker at line index 0 is a valid position (must use sentinel, not zero-check).
|
||||
|
||||
**State Transitions:**
|
||||
```
|
||||
[File Read] → Parse → [No Managed Block] → Add entries → [Has Managed Block] → Write
|
||||
→ [Has Managed Block] → Add/Delete entries → [Updated Managed Block] → Write
|
||||
→ [Corrupt Block] → ERROR (refuse to modify)
|
||||
```
|
||||
|
||||
### DNSEntry
|
||||
|
||||
A single IP-to-hostname mapping in the hosts file.
|
||||
|
||||
| Field | Type | Description | Constraints |
|
||||
|-------|------|-------------|-------------|
|
||||
| IP | `string` | IPv4 address | Must be a valid IPv4 address; result of DNS resolution |
|
||||
| Hostname | `string` | Fully qualified domain name | As provided by user (without trailing dot) |
|
||||
|
||||
**String Representation:** `<IP>\t<Hostname>` (tab-separated, one per line in hosts file)
|
||||
|
||||
**Validation Rules:**
|
||||
- IP must be non-empty and a valid IPv4 address.
|
||||
- Hostname must be non-empty and not contain whitespace.
|
||||
- Entries are deduplicated by their full string representation (IP + hostname pair).
|
||||
|
||||
### ManagedBlock
|
||||
|
||||
The delimited section of the hosts file owned by this tool.
|
||||
|
||||
| Field | Type | Description | Constraints |
|
||||
|-------|------|-------------|-------------|
|
||||
| StartMarker | `string` | Comment line marking block start | `# DNSHelper <<-> START CONFIG` (constant) |
|
||||
| EndMarker | `string` | Comment line marking block end | `# DNSHelper <->> END CONFIG` (constant) |
|
||||
| Entries | `[]DNSEntry` | DNS entries within the block | Zero or more; deduplicated |
|
||||
|
||||
**Rules:**
|
||||
- The managed block is only written if there are entries to include.
|
||||
- When all entries are removed (delete all), the entire block (markers included) is removed.
|
||||
- The block is always written as: start marker, entries (one per line), end marker.
|
||||
|
||||
### ResolveResult
|
||||
|
||||
The outcome of resolving a single hostname against a DNS server.
|
||||
|
||||
| Field | Type | Description | Constraints |
|
||||
|-------|------|-------------|-------------|
|
||||
| Hostname | `string` | The hostname that was queried | As provided by user |
|
||||
| IPs | `[]string` | Resolved IPv4 addresses | Deduplicated; empty if resolution failed |
|
||||
| Error | `error` | Resolution error, if any | Non-nil on failure (timeout, NXDOMAIN, etc.) |
|
||||
|
||||
**State Transitions:**
|
||||
```
|
||||
[Query Sent] → A records returned → [Success: IPs populated]
|
||||
→ CNAME returned → Follow chain (max 10 depth) → [Success or Error]
|
||||
→ NXDOMAIN / timeout / error → [Failure: Error populated]
|
||||
```
|
||||
|
||||
### BackupFile
|
||||
|
||||
A temporary safety copy of the hosts file created before modification.
|
||||
|
||||
| Field | Type | Description | Constraints |
|
||||
|-------|------|-------------|-------------|
|
||||
| Path | `string` | Absolute path to backup file | In executable's directory |
|
||||
| SourcePath | `string` | Path of the file that was backed up | The hosts file path |
|
||||
| Timestamp | `string` | Date stamp portion of filename | Format: `YYYYMMDD` |
|
||||
| RandomSuffix | `string` | Short random string for uniqueness | 4 hex characters (e.g., `a7f3`) |
|
||||
|
||||
**Naming Convention:** `hosts.bak.<Timestamp>-<RandomSuffix>` (e.g., `hosts.bak.20260303-a7f3`)
|
||||
|
||||
**Lifecycle:**
|
||||
```
|
||||
[Pre-Write] → Create backup copy → [Write Success] → Delete backup
|
||||
→ [Write Failure] → Original intact, clean up backup
|
||||
```
|
||||
|
||||
### LockFile
|
||||
|
||||
A file-based mutex for serializing concurrent access.
|
||||
|
||||
| Field | Type | Description | Constraints |
|
||||
|-------|------|-------------|-------------|
|
||||
| Path | `string` | Absolute path to lock file | In executable's directory; `.dns-helper.lock` |
|
||||
| PID | `int` | Process ID of the lock holder | Written for debugging; not used for staleness |
|
||||
| CreatedAt | `time.Time` | When the lock was acquired | Approximated by file ModTime |
|
||||
|
||||
**Parameters:**
|
||||
- Stale timeout: 2 minutes
|
||||
- Retry interval: 200ms
|
||||
- Max wait: 2 seconds (10 retries)
|
||||
|
||||
**Lifecycle:**
|
||||
```
|
||||
[Pre-Lock] → Attempt create (O_EXCL) → [Acquired] → Hold during R/M/W → [Release: delete]
|
||||
→ [Exists, not stale] → Retry (up to 2s) → [Acquired or Error]
|
||||
→ [Exists, stale] → Break stale → Retry → [Acquired or Error]
|
||||
```
|
||||
|
||||
## Relationships
|
||||
|
||||
```
|
||||
CLI (main.go)
|
||||
├── validates input (host, server, subcommand)
|
||||
├── calls Resolver to get ResolveResult[] (before lock)
|
||||
├── acquires LockFile
|
||||
├── reads HostsFile → parses ManagedBlock
|
||||
├── modifies ManagedBlock (add DNSEntry[] / delete by hostname)
|
||||
├── creates BackupFile
|
||||
├── writes HostsFile atomically (temp + rename)
|
||||
├── deletes BackupFile on success
|
||||
└── releases LockFile
|
||||
```
|
||||
|
||||
## Operation Flows
|
||||
|
||||
### Add Flow
|
||||
1. Validate: `-host` and `-server` required and non-empty.
|
||||
2. Resolve: For each hostname, query DNS server → collect `ResolveResult[]`.
|
||||
3. Lock: Acquire lock file.
|
||||
4. Read: Read hosts file → parse into sections.
|
||||
5. Modify: Remove existing entries for the specified hostnames from managed content. Add new `DNSEntry[]` from resolve results.
|
||||
6. Deduplicate: Remove duplicate entries by full line content.
|
||||
7. Backup: Copy current hosts file to backup location.
|
||||
8. Write: Assemble full content (prefix + managed block + postfix) → write to temp file → rename over hosts file.
|
||||
9. Cleanup: Delete backup on success. Release lock.
|
||||
10. Report: Print summary (entries added/updated). Exit non-zero if any hostname failed to resolve.
|
||||
|
||||
### Delete Flow
|
||||
1. Validate: `-host` required and non-empty, OR `all` keyword.
|
||||
2. Lock: Acquire lock file.
|
||||
3. Read: Read hosts file → parse into sections.
|
||||
4. Modify: If specific hosts — remove matching lines from managed content. If `all` — clear managed content entirely (block markers will not be written).
|
||||
5. Backup: Copy current hosts file to backup location.
|
||||
6. Write: Assemble full content → write to temp file → rename.
|
||||
7. Cleanup: Delete backup on success. Release lock.
|
||||
8. Report: Print summary (entries removed). Report if no entries were found.
|
||||
@@ -0,0 +1,110 @@
|
||||
# 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.
|
||||
@@ -0,0 +1,162 @@
|
||||
# Quickstart: Safety, Reliability & Idiomatic Refactor
|
||||
|
||||
**Feature Branch**: `001-safety-reliability-refactor`
|
||||
**Date**: 2026-03-03
|
||||
|
||||
## Prerequisites
|
||||
|
||||
- **Go 1.20** toolchain installed (must be exactly 1.20.x, not newer)
|
||||
- Administrator/root privileges (required to write the hosts file)
|
||||
- Git
|
||||
|
||||
## Setup
|
||||
|
||||
```bash
|
||||
# Clone and switch to feature branch
|
||||
git checkout 001-safety-reliability-refactor
|
||||
|
||||
# Verify Go version
|
||||
go version
|
||||
# Expected: go version go1.20.x ...
|
||||
|
||||
# Download dependencies
|
||||
go mod download
|
||||
|
||||
# Run all tests
|
||||
go test ./...
|
||||
|
||||
# Build
|
||||
go build -o dns-helper .
|
||||
```
|
||||
|
||||
## Project Structure After Refactor
|
||||
|
||||
```
|
||||
dns-helper/
|
||||
├── main.go # Entry point, CLI parsing, os.Exit
|
||||
├── resolver/
|
||||
│ ├── resolver.go # DNS resolution via golang.org/x/net/dns/dnsmessage
|
||||
│ └── resolver_test.go
|
||||
├── hostfile/
|
||||
│ ├── hostfile.go # Hosts file read/write (atomic pattern)
|
||||
│ ├── hostfile_test.go
|
||||
│ ├── managed.go # Managed block parsing, add, remove
|
||||
│ └── managed_test.go
|
||||
├── platform/
|
||||
│ ├── platform.go # Interface definition
|
||||
│ ├── platform_windows.go # Windows hosts file path
|
||||
│ ├── platform_linux.go # Linux hosts file path
|
||||
│ ├── platform_darwin.go # macOS hosts file path
|
||||
│ └── platform_test.go
|
||||
├── lockfile/
|
||||
│ ├── lockfile.go # Lock file acquisition/release
|
||||
│ └── lockfile_test.go
|
||||
├── go.mod # go 1.20, golang.org/x/net v0.35.0
|
||||
└── go.sum
|
||||
```
|
||||
|
||||
## Key Implementation Patterns
|
||||
|
||||
### TDD Workflow (Constitution Principle VI)
|
||||
|
||||
Every change follows Red-Green-Refactor:
|
||||
|
||||
1. **Red**: Write a failing test that specifies the desired behavior.
|
||||
2. **Green**: Write the minimum code to make the test pass.
|
||||
3. **Refactor**: Clean up while keeping tests green.
|
||||
|
||||
```bash
|
||||
# Run tests for a specific package
|
||||
go test ./resolver/
|
||||
go test ./hostfile/
|
||||
go test ./lockfile/
|
||||
|
||||
# Run all tests
|
||||
go test ./...
|
||||
```
|
||||
|
||||
### Error Handling (Constitution Principle IV)
|
||||
|
||||
All functions return errors. Only `main()` calls `os.Exit()`:
|
||||
|
||||
```go
|
||||
// WRONG — do not do this in any package
|
||||
func doSomething() {
|
||||
if err != nil {
|
||||
fmt.Println(err)
|
||||
os.Exit(1) // PROHIBITED outside main()
|
||||
}
|
||||
}
|
||||
|
||||
// CORRECT — return errors to caller
|
||||
func doSomething() error {
|
||||
if err != nil {
|
||||
return fmt.Errorf("doing something: %w", err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
```
|
||||
|
||||
### Testable Interfaces (Constitution Principle VI)
|
||||
|
||||
Functions that interact with filesystem or DNS accept interfaces:
|
||||
|
||||
```go
|
||||
// Production: uses real OS calls
|
||||
manager := hostfile.NewManager(hostfile.NewOSFileSystem())
|
||||
|
||||
// Test: uses in-memory fake
|
||||
manager := hostfile.NewManager(fakeFS)
|
||||
```
|
||||
|
||||
### Atomic Write Sequence
|
||||
|
||||
```
|
||||
1. Backup: copy hosts → <exe-dir>/hosts.bak.<date>-<rand>
|
||||
2. Write: assemble content → temp file (same dir as hosts)
|
||||
3. Sync: temp.Sync()
|
||||
4. Chmod: match original permissions
|
||||
5. Close: temp.Close()
|
||||
6. Rename: os.Rename(temp, hosts) [retry on Windows]
|
||||
7. Clean: delete backup on success
|
||||
```
|
||||
|
||||
## Usage Examples
|
||||
|
||||
```bash
|
||||
# Add DNS entries
|
||||
dns-helper add -host myapp.example.com -server 10.0.0.53
|
||||
|
||||
# Add multiple hosts
|
||||
dns-helper add -host host1.example.com,host2.example.com -server dns.internal
|
||||
|
||||
# Delete specific host entries
|
||||
dns-helper delete -host myapp.example.com
|
||||
|
||||
# Delete all managed entries
|
||||
dns-helper delete all
|
||||
```
|
||||
|
||||
## Build for All Platforms
|
||||
|
||||
```bash
|
||||
# Windows
|
||||
GOOS=windows GOARCH=amd64 go build -o bin/windows/dns-helper.exe .
|
||||
|
||||
# Linux
|
||||
GOOS=linux GOARCH=amd64 go build -o bin/linux/dns-helper .
|
||||
|
||||
# macOS
|
||||
GOOS=darwin GOARCH=amd64 go build -o bin/macos/dns-helper .
|
||||
```
|
||||
|
||||
## Dependency Management
|
||||
|
||||
```bash
|
||||
# After removing miekg/dns and adding dnsmessage usage:
|
||||
go mod tidy
|
||||
|
||||
# Verify no unapproved dependencies
|
||||
go list -m all
|
||||
# Should only show: golang.org/x/net, golang.org/x/sys (transitive)
|
||||
```
|
||||
@@ -0,0 +1,326 @@
|
||||
# Research: Safety, Reliability & Idiomatic Refactor
|
||||
|
||||
**Feature Branch**: `001-safety-reliability-refactor`
|
||||
**Date**: 2026-03-03
|
||||
|
||||
## R-001: Replacing `github.com/miekg/dns` with `golang.org/x/net/dns/dnsmessage`
|
||||
|
||||
### Decision
|
||||
Use `golang.org/x/net/dns/dnsmessage` (part of the approved `golang.org/x` source) to replace all DNS query/response functionality currently provided by `github.com/miekg/dns`.
|
||||
|
||||
### Rationale
|
||||
- `dnsmessage` provides low-level DNS message building and parsing (A, CNAME, and all standard record types via `Message.Pack()`/`Message.Unpack()` or the streaming `Builder`/`Parser` API).
|
||||
- The current usage surface of `miekg/dns` is narrow: build a query, send over UDP, parse A/CNAME responses. `dnsmessage` handles all of this.
|
||||
- Eliminates 4 transitive dependencies (`x/mod`, `x/sys`, `x/tools` pulled in by `miekg/dns`) from the dependency tree.
|
||||
- `golang.org/x/net` is already in the project's dependency list and is an approved source per the constitution.
|
||||
|
||||
### Go 1.20 Compatibility
|
||||
- **`golang.org/x/net v0.35.0`** is the latest version with `go 1.18` directive (compatible with Go 1.20). Starting at `v0.36.0`, the module requires `go 1.23.0`.
|
||||
- The `dnsmessage` sub-package API is stable and has not changed materially across these versions.
|
||||
- **Pin to `golang.org/x/net v0.35.0`** in `go.mod`.
|
||||
|
||||
### Implementation Pattern
|
||||
- Use `Message.Pack()` to build queries (simpler API, adequate for single-query CLI tool).
|
||||
- Use `Message.Unpack()` to parse responses.
|
||||
- Send raw DNS packets over UDP using `net.DialTimeout("udp", server+":53", timeout)`. No length prefix needed for UDP (only TCP requires a 2-byte length prefix).
|
||||
- DNS names must be FQDN with trailing dot: `dnsmessage.MustNewName("example.com.")`.
|
||||
- `AResource.A` is `[4]byte`, convert to string via `net.IP(r.A[:]).String()`.
|
||||
- Response buffer of 1232 bytes is sufficient for standard UDP DNS responses.
|
||||
- Validate response `Header.ID` matches query ID to prevent spoofing.
|
||||
- Default 5-second timeout via `conn.SetDeadline()`.
|
||||
|
||||
### Key Type Mappings (miekg/dns → dnsmessage)
|
||||
|
||||
| miekg/dns | dnsmessage |
|
||||
|-----------|------------|
|
||||
| `dns.Client{}` + `c.Exchange()` | `net.DialTimeout("udp", ...)` + `conn.Write()`/`conn.Read()` |
|
||||
| `dns.Msg{}.SetQuestion()` | `dnsmessage.Message{}.Pack()` |
|
||||
| `dns.TypeA` | `dnsmessage.TypeA` |
|
||||
| `dns.TypeCNAME` | `dnsmessage.TypeCNAME` |
|
||||
| `*dns.A` → `rec.A.String()` | `*dnsmessage.AResource` → `net.IP(r.A[:]).String()` |
|
||||
| `*dns.CNAME` → `rec.Target` | `*dnsmessage.CNAMEResource` → `r.CNAME.String()` |
|
||||
| `dns.RcodeSuccess` | `dnsmessage.RCodeSuccess` |
|
||||
| `dns.RcodeToString[code]` | `rcode.String()` (method on `RCode` type) |
|
||||
|
||||
### Alternatives Considered
|
||||
1. **`net.Resolver` with custom `Dialer`**: The Go stdlib `net.Resolver` does not support querying a specific DNS server by address without hacky workarounds. The tool's core purpose requires directing queries to a user-specified resolver.
|
||||
2. **Keep `miekg/dns`**: Violates Constitution Principle II (Standard Library First). The constitution explicitly calls this a "grandfather clause" dependency with preferred migration.
|
||||
3. **Raw UDP + manual DNS wire format**: Would work but requires implementing DNS message serialization from scratch. `dnsmessage` already does this correctly and is an approved package.
|
||||
|
||||
---
|
||||
|
||||
## R-002: Atomic File Write Strategy (temp-file-and-rename)
|
||||
|
||||
### Decision
|
||||
Write new hosts file content to a temporary file in the same directory, then `os.Rename()` over the original. Combined with a pre-write backup in the executable's directory.
|
||||
|
||||
### Rationale
|
||||
- `os.Rename()` on same-volume is atomic (or effectively atomic) on all three target platforms:
|
||||
- **Linux**: POSIX `rename(2)` is atomic. Guaranteed by the kernel.
|
||||
- **macOS**: Darwin `rename(2)` follows POSIX semantics. Atomic on both APFS and HFS+.
|
||||
- **Windows**: Go's `os.Rename` calls `MoveFileEx` with `MOVEFILE_REPLACE_EXISTING`. Effectively atomic on NTFS (MFT metadata update), though Microsoft does not formally guarantee atomicity.
|
||||
- The temp file **must** be in the same directory as the hosts file to avoid cross-filesystem/cross-volume failures (`EXDEV` on Unix, copy+delete on Windows).
|
||||
- The original hosts file is never opened for writing or truncated.
|
||||
|
||||
### Go 1.20 Compatibility
|
||||
- `os.CreateTemp(dir, pattern)` available since Go 1.16.
|
||||
- `os.Rename()` stable across all Go versions.
|
||||
- No known bugs in Go 1.20 affecting this pattern.
|
||||
|
||||
### Implementation Details
|
||||
|
||||
1. **Temp file creation**: `os.CreateTemp(filepath.Dir(hostsPath), ".dns-helper-tmp-*")` — creates file in same directory with recognizable prefix.
|
||||
2. **Permissions**: The renamed file retains the temp file's permissions, NOT the original's. Must `os.Stat(hostsPath)` to get original mode, then `os.Chmod(tempFile, originalMode)` before renaming.
|
||||
3. **Close before rename**: Required on Windows — cannot rename a file that's still open by the writing process.
|
||||
4. **Sync before close**: Call `file.Sync()` for durability before closing.
|
||||
5. **Cleanup on failure**: Use a `defer` with a success flag to remove the temp file if any step fails.
|
||||
6. **Windows retry**: `os.Rename` on the hosts file may fail with `ERROR_SHARING_VIOLATION` if Windows Defender or the DNS Client service has a transient read lock. Implement 2-3 retry attempts with 100-200ms delay.
|
||||
7. **Orphan cleanup**: On startup, optionally clean stale `.dns-helper-tmp-*` files in the hosts directory.
|
||||
|
||||
### Backup Strategy (FR-002)
|
||||
- Before any write operation, copy the current hosts file to `<exe-dir>/hosts.bak.<date>-<random>` (e.g., `hosts.bak.20260303-a7f3`).
|
||||
- The backup is a regular file copy (not rename), since it's in a different directory (exe dir vs hosts file dir).
|
||||
- On successful rename of temp over hosts file: delete the backup.
|
||||
- On rename failure: the original hosts file was never modified. Clean up temp file and backup.
|
||||
- The date+random naming handles concurrent instances.
|
||||
|
||||
### Alternatives Considered
|
||||
1. **Write directly to hosts file (current approach)**: Uses `O_WRONLY|O_TRUNC`, which immediately truncates the file. A crash during write leaves it empty/partial. This is the bug being fixed.
|
||||
2. **Backup-only (no atomic rename)**: Would still truncate the hosts file during write. The backup only helps after a failure, not during.
|
||||
3. **Write to temp, then copy content over original**: The copy step is not atomic — a crash during copy is the same as a crash during direct write.
|
||||
|
||||
---
|
||||
|
||||
## R-003: Lock File Implementation
|
||||
|
||||
### Decision
|
||||
Use `os.OpenFile` with `O_CREATE|O_EXCL` to create a lock file in the executable's directory. Use timeout-based staleness detection (file modification time).
|
||||
|
||||
### Rationale
|
||||
- `O_CREATE|O_EXCL` is atomic on all three platforms (Windows maps to `CREATE_NEW`, Unix maps to `O_CREAT|O_EXCL`).
|
||||
- Single code path — no platform-specific build files needed for the lock mechanism.
|
||||
- Timeout-based staleness avoids the cross-platform pitfalls of PID-based detection (Windows doesn't support `Signal(0)` for process existence checks without `x/sys`).
|
||||
|
||||
### Implementation Details
|
||||
|
||||
| Parameter | Value | Rationale |
|
||||
|-----------|-------|-----------|
|
||||
| Lock file name | `.dns-helper.lock` | Hidden file, recognizable prefix |
|
||||
| Lock location | Executable's directory | Same as backup files; writable by admin/root (required anyway) |
|
||||
| Stale timeout | 2 minutes | Lock held only during read-modify-write (<1s normally). 2min is very conservative. |
|
||||
| Retry interval | 200ms | Short enough to be responsive, long enough to avoid busy-wait |
|
||||
| Max wait | 2 seconds | 10 retries. Enough for another instance's write cycle. |
|
||||
|
||||
**Lock protocol per FR-016:**
|
||||
1. Parse args and validate input.
|
||||
2. Perform DNS resolution (before lock acquisition to minimize lock hold time).
|
||||
3. Acquire lock (retry up to 2s, break stale locks >2min).
|
||||
4. Read hosts file → modify content → write temp file → rename over hosts file.
|
||||
5. Release lock (delete lock file).
|
||||
6. Lock release in `defer` to handle error paths and panics.
|
||||
|
||||
**Staleness detection:**
|
||||
- `os.Stat(lockFile).ModTime()` — if older than 2 minutes, `os.Remove()` and retry.
|
||||
- Cross-platform via Go stdlib. No platform-specific code needed.
|
||||
|
||||
**Signal handling:**
|
||||
- `defer lock.Release()` handles normal exit and errors.
|
||||
- For SIGKILL/power loss, the stale lock persists. Next invocation detects and breaks it after 2 minutes.
|
||||
- Write PID to lock file for debugging (human inspection), but don't use it for staleness logic.
|
||||
|
||||
### Alternatives Considered
|
||||
1. **Advisory file locking (`flock`/`LockFileEx`)**: Auto-releases on crash (no stale lock problem), but requires 3 platform-specific build files. `syscall.Flock` doesn't exist on Windows. Adds complexity for marginal benefit.
|
||||
2. **PID-based staleness**: More precise than timeout, but `os.FindProcess` + `Signal(0)` doesn't work on Windows without `golang.org/x/sys/windows`. Defeats the single-code-path advantage.
|
||||
3. **No locking**: Risk of two instances writing the hosts file simultaneously. The spec explicitly requires serialization (FR-016).
|
||||
|
||||
---
|
||||
|
||||
## R-004: Multi-Host Deletion Bug Analysis
|
||||
|
||||
### Decision
|
||||
Fix the nested loop logic in `removeHostsFromExistingContent` by inverting the loop order (iterate lines on the outside, check all hosts on the inside).
|
||||
|
||||
### Rationale — Bug Analysis
|
||||
|
||||
Current code:
|
||||
```go
|
||||
func removeHostsFromExistingContent(data *workingData) {
|
||||
var updatedContent []string
|
||||
for _, host := range data.Hosts {
|
||||
for _, line := range data.ExistingContent {
|
||||
if !strings.Contains(line, host) {
|
||||
updatedContent = append(updatedContent, line)
|
||||
}
|
||||
}
|
||||
}
|
||||
data.NewContent = updatedContent
|
||||
}
|
||||
```
|
||||
|
||||
**Bug**: The inner loop always iterates over `data.ExistingContent` (the original, unfiltered content). `updatedContent` is never reset between outer iterations. Result:
|
||||
- Lines removed by one host are re-added by another host's pass (re-scans original).
|
||||
- Surviving lines are duplicated once per host in `data.Hosts`.
|
||||
|
||||
**Trace** with `Hosts = ["host1", "host2"]`, `ExistingContent = ["1.1.1.1\thost1", "2.2.2.2\thost2", "3.3.3.3\thost3"]`:
|
||||
- Pass 1 (host1): keeps `host2`, `host3` → `updatedContent = [host2, host3]`
|
||||
- Pass 2 (host2): re-scans original, keeps `host1`, `host3` → `updatedContent = [host2, host3, host1, host3]`
|
||||
- **Result**: Both hosts survive, `host3` is duplicated. Every invariant violated.
|
||||
|
||||
### Correct Algorithm
|
||||
Iterate lines on the outside; for each line, check if it matches ANY host:
|
||||
```go
|
||||
func removeHostsFromExistingContent(data *workingData) {
|
||||
var updatedContent []string
|
||||
for _, line := range data.ExistingContent {
|
||||
shouldRemove := false
|
||||
for _, host := range data.Hosts {
|
||||
if strings.Contains(line, host) {
|
||||
shouldRemove = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !shouldRemove {
|
||||
updatedContent = append(updatedContent, line)
|
||||
}
|
||||
}
|
||||
data.NewContent = updatedContent
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## R-005: CNAME Resolution Duplicate IP Bug Analysis
|
||||
|
||||
### Decision
|
||||
Fix the self-appending loop in the CNAME resolution path of `lookupIP()` and add a max-depth limit for CNAME chain following.
|
||||
|
||||
### Rationale — Bug Analysis
|
||||
|
||||
Current code:
|
||||
```go
|
||||
for _, ans := range r.Answer {
|
||||
if rec, ok := ans.(*dns.CNAME); ok {
|
||||
ips = lookupIP(rec.Target, resolver)
|
||||
for _, ip := range ips {
|
||||
ips = append(ips, ip)
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Bug**: `ips` is the result of the recursive `lookupIP` call. The `for range` then iterates over `ips` and appends each element back to itself. In Go, `for range` evaluates the slice length once at the start, so this is not infinite — it exactly doubles every element. With `ips = ["10.0.0.1", "10.0.0.2"]`, the result is `["10.0.0.1", "10.0.0.2", "10.0.0.1", "10.0.0.2"]`.
|
||||
|
||||
**Additional issue**: No CNAME chain depth limit. Circular CNAMEs would cause infinite recursion.
|
||||
|
||||
### Correct Algorithm
|
||||
```go
|
||||
for _, ans := range r.Answer {
|
||||
if rec, ok := ans.(*dns.CNAME); ok {
|
||||
cnameIPs := lookupIP(rec.Target, resolver)
|
||||
ips = append(ips, cnameIPs...)
|
||||
}
|
||||
}
|
||||
```
|
||||
Plus a max-depth parameter (10 levels) to prevent infinite CNAME recursion.
|
||||
|
||||
---
|
||||
|
||||
## R-006: Go 1.20 Dependency Cleanup
|
||||
|
||||
### Decision
|
||||
Update `go.mod` to `go 1.20`. Pin `golang.org/x/net` to `v0.35.0`. Remove `github.com/miekg/dns` and all its transitive dependencies.
|
||||
|
||||
### Rationale
|
||||
|
||||
Current `go.mod` dependencies:
|
||||
```
|
||||
github.com/miekg/dns v1.1.59 ← removing (FR-014)
|
||||
golang.org/x/mod v0.16.0 ← transitive dep of miekg/dns, will be removed
|
||||
golang.org/x/net v0.22.0 ← keeping, updating to v0.35.0
|
||||
golang.org/x/sys v0.18.0 ← transitive dep of x/net, will be updated
|
||||
golang.org/x/tools v0.19.0 ← transitive dep of miekg/dns, will be removed
|
||||
```
|
||||
|
||||
After cleanup:
|
||||
```
|
||||
golang.org/x/net v0.35.0 ← for dns/dnsmessage
|
||||
golang.org/x/sys (transitive) ← pulled by x/net
|
||||
```
|
||||
|
||||
`golang.org/x/mod` and `golang.org/x/tools` are only needed by `miekg/dns` and will be removed by `go mod tidy` after removing the `miekg/dns` import.
|
||||
|
||||
### Version Verification
|
||||
- `golang.org/x/net v0.35.0`: `go 1.18` directive — compatible with Go 1.20.
|
||||
- `golang.org/x/net v0.36.0`: `go 1.23.0` directive — **NOT compatible**. Must pin at v0.35.0.
|
||||
|
||||
---
|
||||
|
||||
## R-007: Testability Design for TDD (Constitution Principle VI)
|
||||
|
||||
### Decision
|
||||
Define interfaces for external dependencies (filesystem, DNS, lock) to enable testing with fakes/stubs. All business logic operates on interfaces, not concrete OS calls.
|
||||
|
||||
### Rationale
|
||||
Constitution Principle VI mandates TDD with testable interfaces. The current codebase has zero tests and directly calls OS-level I/O from business logic.
|
||||
|
||||
### Interface Design
|
||||
|
||||
**Filesystem interface** (for `hostfile` package):
|
||||
```go
|
||||
type FileSystem interface {
|
||||
ReadFile(path string) ([]byte, error)
|
||||
WriteFile(path string, data []byte, perm os.FileMode) error
|
||||
Stat(path string) (os.FileInfo, error)
|
||||
CreateTemp(dir, pattern string) (*os.File, error)
|
||||
Rename(oldpath, newpath string) error
|
||||
Remove(path string) error
|
||||
}
|
||||
```
|
||||
|
||||
**Resolver interface** (for `resolver` package):
|
||||
```go
|
||||
type Resolver interface {
|
||||
LookupIP(hostname, server string) ([]string, error)
|
||||
}
|
||||
```
|
||||
|
||||
**Lock interface** (for `lockfile` package):
|
||||
```go
|
||||
type Lock interface {
|
||||
Release() error
|
||||
}
|
||||
type Locker interface {
|
||||
Acquire(dir string) (Lock, error)
|
||||
}
|
||||
```
|
||||
|
||||
Production code uses real implementations. Tests inject fakes that simulate errors, crashes, and edge cases without touching the real filesystem or DNS.
|
||||
|
||||
### Test Categories
|
||||
1. **Unit tests**: Managed block parsing, line removal, deduplication, input validation — pure logic, no I/O.
|
||||
2. **Integration tests with fakes**: Atomic write sequence, backup creation/cleanup, lock acquisition/release — use in-memory filesystem fakes.
|
||||
3. **Platform tests**: `getHostsFileLocation()` on each OS — verify return value and error handling against build tags.
|
||||
|
||||
---
|
||||
|
||||
## R-008: Managed Block Corruption Detection (FR-011)
|
||||
|
||||
### Decision
|
||||
Detect and report four corruption scenarios for managed block markers. Refuse to modify the file when corruption is detected.
|
||||
|
||||
### Rationale
|
||||
The current `extractLinesToEdit` function detects "start without end" via a `fileCorrupted` flag, but the error message ("content has been corrupted") is vague and doesn't cover all cases.
|
||||
|
||||
### Corruption Scenarios
|
||||
|
||||
| Scenario | Detection | Error Message |
|
||||
|----------|-----------|---------------|
|
||||
| Start marker without end marker | `startIndex > 0 && endIndex == 0` | "managed block is corrupt: start marker found at line N but no end marker" |
|
||||
| End marker without start marker | `startIndex == 0 && endIndex > 0` | "managed block is corrupt: end marker found at line N but no start marker" |
|
||||
| End marker before start marker | `endIndex < startIndex` | "managed block is corrupt: end marker (line N) appears before start marker (line M)" |
|
||||
| Duplicate start or end markers | Multiple occurrences detected during scan | "managed block is corrupt: duplicate start/end markers found" |
|
||||
|
||||
All corruption scenarios must result in an error returned to the caller. The hosts file must not be modified.
|
||||
|
||||
### Current Code Bug
|
||||
The current code uses `startIndex == 0` to mean "not found," but index 0 is a valid line position (the first line). Should use a sentinel value (-1) or a boolean flag for "found/not found."
|
||||
@@ -0,0 +1,178 @@
|
||||
# Feature Specification: Safety, Reliability & Idiomatic Refactor
|
||||
|
||||
**Feature Branch**: `001-safety-reliability-refactor`
|
||||
**Created**: 2026-03-03
|
||||
**Status**: Draft
|
||||
**Input**: User description: "Address safety and reliability bugs, refactor for idiomatic Go 1.20 patterns, improve project structure and developer maintainability"
|
||||
|
||||
## User Scenarios & Testing *(mandatory)*
|
||||
|
||||
### User Story 1 - Hosts File Is Never Corrupted or Lost (Priority: P1)
|
||||
|
||||
As a system administrator, I need the hosts file to remain intact even if the tool crashes, loses power, or encounters a disk error mid-write, because a corrupted hosts file can break all name resolution on the machine and require manual recovery.
|
||||
|
||||
**Why this priority**: The hosts file is a critical system resource. Losing it can render a machine unable to resolve any DNS names, disrupting all network-dependent software. Protecting it is the single most important improvement.
|
||||
|
||||
**Independent Test**: Run the tool to add a DNS entry, then simulate an interruption (e.g., kill the process during write). Verify the original hosts file is unchanged or correctly updated — never partially written or empty.
|
||||
|
||||
**Acceptance Scenarios**:
|
||||
|
||||
1. **Given** an existing hosts file with valid content, **When** the tool writes updated entries successfully, **Then** the hosts file contains the correct updated content, no data from the original file is lost, and the hosts file was never truncated or partially written at any point.
|
||||
2. **Given** an existing hosts file with valid content, **When** the tool is interrupted (crash, signal, power loss) during the write operation, **Then** the original hosts file remains intact and unmodified because the write targeted a temporary file, not the hosts file directly.
|
||||
3. **Given** the disk is full or the write fails for any reason, **When** the tool attempts to save the hosts file, **Then** an error is reported to the user, the original hosts file is not altered, and any temporary files are cleaned up.
|
||||
4. **Given** an existing hosts file, **When** the tool prepares to write, **Then** a temporary backup copy of the current hosts file is created before any modification begins. On successful rename of the temp file over the hosts file, the backup is deleted. On failure, the backup is available and the original hosts file was never modified.
|
||||
|
||||
---
|
||||
|
||||
### User Story 2 - Correct DNS Resolution Results (Priority: P1)
|
||||
|
||||
As a user adding DNS entries, I need the resolved IP addresses written to the hosts file to be accurate and free of duplicates, because incorrect entries will cause connectivity failures — the exact problem this tool is meant to solve.
|
||||
|
||||
**Why this priority**: The core value proposition of the tool is providing correct, current DNS mappings. Bugs that produce duplicate or incorrect IPs directly undermine the tool's purpose.
|
||||
|
||||
**Independent Test**: Add a hostname that resolves via CNAME chain to multiple IPs using a specific DNS server. Verify the hosts file contains exactly the correct set of unique IPs with no duplicates.
|
||||
|
||||
**Acceptance Scenarios**:
|
||||
|
||||
1. **Given** a hostname that resolves directly to A records, **When** the user runs `add -host <name> -server <dns>`, **Then** the hosts file contains one entry per unique IP address for that hostname.
|
||||
2. **Given** a hostname that resolves via a CNAME chain to A records, **When** the user runs `add -host <name> -server <dns>`, **Then** the CNAME chain is followed correctly and the final A record IPs are written without duplicates.
|
||||
3. **Given** multiple hostnames specified via comma-separated values, **When** the user runs `add -host host1,host2 -server <dns>`, **Then** all hostnames are resolved and each gets correct, deduplicated entries in the hosts file.
|
||||
4. **Given** the DNS resolution layer has been reimplemented using only approved dependencies, **When** any hostname is resolved, **Then** the results are identical to the previous implementation (same IPs, same CNAME handling, same error reporting).
|
||||
5. **Given** multiple hostnames where some resolve successfully and others fail, **When** the user runs `add -host host1,host2 -server <dns>`, **Then** entries are written for the successfully resolved hostnames, a warning identifying each failed hostname is printed to stdout, and the tool exits with a non-zero exit code.
|
||||
|
||||
---
|
||||
|
||||
### User Story 3 - Reliable Deletion of Managed Entries (Priority: P2)
|
||||
|
||||
As a user, when I delete managed DNS entries (one hostname or all), I need the tool to correctly remove only the entries it previously added, leaving all other hosts file content untouched.
|
||||
|
||||
**Why this priority**: Incorrect deletion could remove entries the user or other tools added, or leave orphaned entries behind. This must work correctly to maintain trust in the tool.
|
||||
|
||||
**Independent Test**: Add entries for two hostnames, then delete one. Verify the deleted hostname's entries are gone and the other hostname's entries remain. Then test `delete all` and verify only the managed block is removed.
|
||||
|
||||
**Acceptance Scenarios**:
|
||||
|
||||
1. **Given** the hosts file contains managed entries for hosts A and B, **When** the user deletes host A, **Then** only host A's entries are removed; host B's managed entries and all unmanaged content remain intact.
|
||||
2. **Given** the hosts file contains managed entries for multiple hosts, **When** the user runs `delete all`, **Then** the entire managed block (start marker through end marker) is removed and all unmanaged content remains intact.
|
||||
3. **Given** the hosts file contains no managed entries, **When** the user runs `delete -host <name>`, **Then** the tool reports that no entries were found and the hosts file is not modified.
|
||||
|
||||
---
|
||||
|
||||
### User Story 4 - Clear Error Messages and Input Validation (Priority: P2)
|
||||
|
||||
As a user, when I provide invalid or incomplete input (missing flags, empty values, unresolvable hostnames), I need a clear error message explaining what went wrong and how to fix it, and the tool must not proceed with partial or incorrect data.
|
||||
|
||||
**Why this priority**: Without proper validation, the tool can silently produce incorrect results or corrupt the managed block. Clear errors prevent misuse and reduce troubleshooting time.
|
||||
|
||||
**Independent Test**: Run the tool with missing `-host`, empty `-server`, an unresolvable hostname, and invalid subcommands. Verify each produces a descriptive error, returns a non-zero exit code, and does not modify the hosts file.
|
||||
|
||||
**Acceptance Scenarios**:
|
||||
|
||||
1. **Given** the user runs `add` without a `-host` flag, **When** the command executes, **Then** an error is displayed explaining that `-host` is required, and the hosts file is not modified.
|
||||
2. **Given** the user runs `add -host example.com` without a `-server` flag, **When** the command executes, **Then** an error is displayed explaining that `-server` is required for the add command, and the hosts file is not modified.
|
||||
3. **Given** the user provides an unrecognized subcommand, **When** the command executes, **Then** a usage message is displayed and the hosts file is not modified.
|
||||
4. **Given** the hostname cannot be resolved by the specified DNS server, **When** the command executes, **Then** an error is displayed indicating the resolution failure, and no entry is written for that hostname.
|
||||
|
||||
---
|
||||
|
||||
### User Story 5 - Consistent Behavior Across Operating Systems (Priority: P3)
|
||||
|
||||
As a user running the tool on Windows, Linux, or macOS, I need the tool to behave consistently — locating the hosts file, reporting errors, and handling edge cases the same way regardless of platform.
|
||||
|
||||
**Why this priority**: Inconsistent platform behavior makes the tool unreliable and harder to maintain. Standardizing the platform layer improves confidence and reduces platform-specific bugs.
|
||||
|
||||
**Independent Test**: Build and run the tool on each supported platform. Verify that error conditions (missing hosts file, permission denied) produce equivalent error messages and exit codes on all platforms.
|
||||
|
||||
**Acceptance Scenarios**:
|
||||
|
||||
1. **Given** the tool is running on any supported platform, **When** the hosts file cannot be found at the expected location, **Then** an error is returned to the caller with a descriptive message (not a process exit from within the platform code).
|
||||
2. **Given** the tool is running on any supported platform, **When** a permission error prevents reading or writing the hosts file, **Then** a clear error message is displayed and the tool exits cleanly.
|
||||
|
||||
---
|
||||
|
||||
### User Story 6 - Clean, Maintainable Project Structure (Priority: P3)
|
||||
|
||||
As a developer maintaining this tool, I need the codebase to follow idiomatic patterns and be organized into logical units so that future changes (new features, bug fixes) are straightforward and low-risk.
|
||||
|
||||
**Why this priority**: The current codebase has dead code, inconsistent patterns, and tightly coupled logic that makes changes risky. A clean structure reduces the cost and risk of future work.
|
||||
|
||||
**Independent Test**: Review the refactored codebase for: no dead code, consistent error handling patterns, clear separation of concerns, and all tests passing after reorganization.
|
||||
|
||||
**Acceptance Scenarios**:
|
||||
|
||||
1. **Given** the refactored codebase, **When** a developer reads the project, **Then** each file/package has a clear, single responsibility and no unused exports or dead code exist.
|
||||
2. **Given** the refactored codebase, **When** errors occur in any function, **Then** errors are returned to the caller (not handled by terminating the process internally) and the caller decides how to handle them.
|
||||
3. **Given** the project targets Go 1.20, **When** the project is built, **Then** it compiles cleanly with no deprecation warnings and uses language features available up to Go 1.20.
|
||||
|
||||
---
|
||||
|
||||
### Edge Cases
|
||||
|
||||
- What happens when the hosts file is read-only or locked by another process?
|
||||
- What happens when the DNS server is unreachable or times out?
|
||||
- What happens when the managed block markers are present but malformed (e.g., start without end, end without start, duplicates)?
|
||||
- What happens when the hosts file contains the managed block markers inside a comment added by someone else?
|
||||
- What happens when the tool is run concurrently (two instances at the same time)? → Serialized via lock file; second instance waits briefly then fails if lock unavailable.
|
||||
- What happens when the hostname resolves to zero IP addresses (NXDOMAIN)?
|
||||
- What happens when the `-host` value contains whitespace or special characters?
|
||||
|
||||
## Requirements *(mandatory)*
|
||||
|
||||
### Functional Requirements
|
||||
|
||||
- **FR-001**: The tool MUST write new hosts file content to a temporary file in the same directory as the hosts file, then rename the temporary file over the original. The hosts file MUST never be truncated or written to directly. If the rename fails, the temporary file MUST be cleaned up and an error reported.
|
||||
- **FR-002**: The tool MUST create a temporary backup of the current hosts file before performing any modification. The backup MUST be stored in the same directory as the tool's binary executable, named with a date stamp and short random string (e.g., `hosts.bak.20260303-a7f3`) to avoid collisions from concurrent runs. On successful rename of the temp file over the hosts file, the backup MUST be deleted. If the rename fails, the original hosts file was never modified so the backup serves as a belt-and-suspenders safety net; it MUST be deleted after confirming the original is intact.
|
||||
- **FR-003**: The tool MUST correctly follow CNAME chains during DNS resolution without producing duplicate IP entries.
|
||||
- **FR-004**: The tool MUST correctly remove entries for all specified hostnames when deleting from a multi-host managed block (not just the last hostname processed).
|
||||
- **FR-005**: The tool MUST NOT execute the hosts file write operation when an unrecognized subcommand is provided.
|
||||
- **FR-006**: The tool MUST validate that required flags (`-host` for add/delete, `-server` for add) are present and non-empty before proceeding.
|
||||
- **FR-007**: The tool MUST terminate immediately upon encountering a fatal error, without continuing to execute subsequent operations.
|
||||
- **FR-008**: All platform-specific code MUST return errors to the caller rather than terminating the process directly.
|
||||
- **FR-009**: The tool MUST remove all dead/unused code (unused `ping` subcommand, unused `IsNew` field, unused `fileExists` in shared scope).
|
||||
- **FR-010**: The tool MUST target Go 1.20 as the minimum language version and update dependencies accordingly.
|
||||
- **FR-011**: The tool MUST detect and report corrupt managed block markers (start without end, end without start) and refuse to modify the file.
|
||||
- **FR-012**: The tool MUST deduplicate resolved IP entries before writing them to the hosts file.
|
||||
- **FR-013**: The tool MUST provide an informational summary of changes made (entries added, entries removed) after a successful operation.
|
||||
- **FR-014**: The tool MUST eliminate the `github.com/miekg/dns` dependency by reimplementing DNS query functionality using only the Go standard library or approved sources (`golang.org/x/*`). The replacement MUST support sending A and CNAME queries to a user-specified DNS server and parsing the responses.
|
||||
- **FR-015**: When resolving multiple hostnames in a single invocation, the tool MUST write entries for all hostnames that resolved successfully, print a warning to stdout identifying each hostname that failed to resolve, and exit with a non-zero exit code to indicate partial failure.
|
||||
- **FR-016**: The tool MUST use a lock file in the executable's directory to serialize concurrent access to the hosts file. DNS resolution MUST be performed before acquiring the lock so that the lock is held only during the file read-modify-write cycle. If the lock cannot be acquired after a brief wait, the tool MUST fail with a descriptive error. The lock MUST be released (and the lock file deleted) on both success and failure paths.
|
||||
|
||||
### Key Entities
|
||||
|
||||
- **Hosts File**: The system DNS resolution file (`/etc/hosts` or `%SystemRoot%\System32\drivers\etc\hosts`). Contains unmanaged content (user/system entries) and an optional managed block owned by this tool.
|
||||
- **Managed Block**: A clearly delimited section within the hosts file, bounded by start and end marker comments, containing only entries written by this tool.
|
||||
- **DNS Entry**: A mapping of an IP address to a hostname, represented as a single line in the hosts file (e.g., `1.2.3.4\texample.com`).
|
||||
- **Resolver**: An external DNS server used to look up authoritative IP addresses for a given hostname.
|
||||
- **Backup File**: A temporary copy of the hosts file created in the tool's executable directory before each modification, used to restore the original if the write fails. Named with a date stamp and random string to handle concurrent runs (e.g., `hosts.bak.20260303-a7f3`). Deleted after a successful write.
|
||||
|
||||
## Assumptions
|
||||
|
||||
- The tool runs with sufficient permissions to read and write the hosts file (typically requires administrator/root privileges). Permission errors are reported clearly but obtaining privileges is the user's responsibility.
|
||||
- The backup file is a temporary rollback mechanism, not a persistent archive. It is stored in the same directory as the tool's binary executable (not alongside the hosts file) and is deleted after a successful write. The filename uses a date stamp plus a short random string to avoid collisions when multiple instances run concurrently.
|
||||
- Atomic write via temp-file-and-rename combined with a pre-write backup provides two layers of safety. The atomic rename ensures the hosts file is never in a partially-written state. The backup in the executable's directory is a belt-and-suspenders safety net for edge cases where the rename itself fails (e.g., Windows file locking). On success, the backup is deleted. On rename failure, the original hosts file was never touched; the temp file and backup are cleaned up.
|
||||
- "Go 1.20" is the maximum version to target for legacy system compatibility. No features exclusive to Go 1.21+ will be used.
|
||||
- The tool will continue to be a single-binary CLI application with no external configuration files or services.
|
||||
- DNS resolution timeout will use a reasonable default (5 seconds) since no timeout is currently configured.
|
||||
- Concurrent access is serialized via a lock file in the executable's directory. DNS resolution is performed before acquiring the lock to minimize lock hold time. The lock covers only the file read-modify-write-rename cycle. If a stale lock file is detected (e.g., from a crashed instance), the tool may need a timeout-based expiry or PID check to recover — the specific staleness strategy will be determined during planning.
|
||||
- The `github.com/miekg/dns` dependency is the only unapproved external package. Its usage surface is narrow (build query, send over UDP, parse A/CNAME response). Replacement will use `golang.org/x/net/dns/dnsmessage` (an approved source per the project constitution) which provides low-level DNS message construction and parsing sufficient for this use case.
|
||||
|
||||
## Success Criteria *(mandatory)*
|
||||
|
||||
### Measurable Outcomes
|
||||
|
||||
- **SC-001**: The hosts file is never left empty, truncated, or partially written — the file transitions directly from old content to new content via atomic rename. Verified by kill-during-write testing.
|
||||
- **SC-002**: CNAME-chain resolution produces zero duplicate IP entries in the hosts file. Verified by testing with hostnames that resolve through CNAME records.
|
||||
- **SC-003**: Deleting one hostname from a managed block with multiple hostnames removes exactly the targeted hostname's entries and preserves all others. Verified with multi-host test scenarios.
|
||||
- **SC-004**: Every invalid input combination (missing host, missing server, empty values, unknown subcommand) produces a descriptive error message and a non-zero exit code without modifying the hosts file.
|
||||
- **SC-005**: All platform-specific functions (Windows, Linux, macOS) exhibit identical error-reporting behavior — returning errors to the caller rather than terminating directly.
|
||||
- **SC-006**: The project compiles cleanly with Go 1.20 with no dead code, no unused variables, no deprecated API usage, and zero unapproved external dependencies.
|
||||
- **SC-007**: On write failure, the original hosts file content is restored from the temporary backup. On write success, the temporary backup is deleted and no residual backup files remain.
|
||||
|
||||
## Clarifications
|
||||
|
||||
### Session 2026-03-03
|
||||
|
||||
- Q: How should the tool handle partial DNS resolution failure across multiple hostnames? → A: Write entries for successfully resolved hostnames; print warning to stdout identifying each failed hostname; exit with non-zero code.
|
||||
- Q: How many backup copies of the hosts file should the tool retain? → A: None persistently. The backup is a temporary rollback file stored in the executable's directory, named with date + random string (e.g., `hosts.bak.20260303-a7f3`). Deleted on successful write; used to restore then deleted on failed write.
|
||||
- Q: Should the tool use atomic rename, backup-and-restore, or both? → A: Both (Option C). Write backup to exe directory, write new content to temp file in hosts file directory, rename temp over original. On success delete backup. On rename failure original was never touched; clean up temp file and backup.
|
||||
- Q: Should the tool use file locking to prevent concurrent modification of the hosts file? → A: Yes (Option A). Use a lock file in the executable's directory. DNS resolution happens before lock acquisition so the lock is held only during the file read-modify-write cycle. Wait briefly then fail if lock unavailable.
|
||||
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user