Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ lastmod: 2025-10-18
status: active
---

This is the htmltest project - a fast HTML validation and link checker written in Go.
This is the htmltest project - a fast HTML validation and link checker written
in Go.

## Key Files to Reference

Expand All @@ -24,15 +25,16 @@ When starting a session or before making changes, review:

## Current Work

Branch: dev/main
Current feature: CacheAllExternal (see @docs/tasks/cache-unchecked-external-links.md)
- Branch: dev/main
- Current feature: CacheAllExternal (see
`@docs/tasks/cache-unchecked-external-links.md`)

## Testing Commands

Examples:

```bash
make test-tdd TEST_RUN=TestName # Run specific test with clean cache
make test-tdd TEST_RUN='.*Cache.*' # Run all cache tests
make test-tdd-cache TEST_RUN=TestName # Same but shows cache state
```

13 changes: 8 additions & 5 deletions docs/ops/session-start.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ cSpell:ignore: oneline

# Session Start Checklist

At the beginning of each work session, reference these key files to provide context:
At the beginning of each work session, reference these key files to provide
context:

## Essential Context Files

Expand All @@ -20,7 +21,8 @@ At the beginning of each work session, reference these key files to provide cont

## Current Work

- `@docs/tasks/cache-unchecked-external-links.md` - CacheAllExternal feature (in progress)
- `@docs/tasks/cache-unchecked-external-links.md` - CacheAllExternal feature (in
progress)
- `@docs/tasks/migrate-status-codes.md` - Status code migration (completed)

## How to Use
Expand All @@ -31,7 +33,8 @@ At the start of a session, say:
Review @AGENTS.md
```

Cursor auto-loads `docs/AGENTS.md`, but explicitly reviewing ensures full context.
Cursor auto-loads `docs/AGENTS.md`, but explicitly reviewing ensures full
context.

Then mention the specific task you're working on:

Expand All @@ -54,5 +57,5 @@ make help # Available commands

1. **Status codes**: 0 = unchecked, -10 = timeout, >0 = HTTP codes
2. **TDD workflow**: Use `make test-tdd` and `make test-tdd-cache`
3. **Three config dimensions**: CheckExternal, RetryCachedErrors, CacheAllExternal

3. **Three config dimensions**: CheckExternal, RetryCachedErrors,
CacheAllExternal
58 changes: 31 additions & 27 deletions docs/tasks/cache-unchecked-external-links.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@ links to the cache file. This allows you to:
separately

Additionally, when checking is enabled (`CheckExternal: true`), this option also
caches timeout errors (408), which were previously not cached.
caches timeout errors (as `StatusTimeout`), which were previously not cached.

Example workflow:

- Run htmltest with `CheckExternal: false` and `CacheAllExternal: true` during
development
- External links are saved to refcache with status code 0 (unchecked)
- External links are saved to refcache with `StatusUnchecked` (not checked)
- Later, extract the list of unchecked links from the cache for validation or
reporting
- Optionally run htmltest with `CheckExternal: true` to validate cached links
Expand All @@ -46,24 +46,27 @@ Example workflow:

### Complete Behavior Matrix

| CheckExternal | CacheAllExternal | RetryCachedErrors | Links Checked? | Errors Retried? | What Gets Cached | Use Case |
| ------------- | ---------------- | ----------------- | -------------- | --------------- | ------------------ | ------------------------------------ |
| `true` | `false` | `true` | ✓ | ✓ | 200, 404 (NOT 408) | **Default/Legacy** |
| `true` | `false` | `false` | ✓ | ✗ | 200, 404 (NOT 408) | Reuse errors, retry timeouts |
| `true` | `true` | `true` | ✓ | ✓ | 200, 404, 408 | Cache timeouts, but retry |
| `true` | `true` | `false` | ✓ | ✗ | 200, 404, 408 | **Fast re-runs** - cache & reuse all |
| `false` | `false` | (N/A) | ✗ | N/A | Nothing | **Default skip** - no cache |
| `false` | `true` | (N/A) | ✗ | N/A | 0 (unchecked) | **Link discovery** |
| CheckExternal | CacheAllExternal | RetryCachedErrors | Links Checked? | Errors Retried? | What Gets Cached | Use Case |
| ------------- | ---------------- | ----------------- | -------------- | --------------- | ----------------- | ------------------------------------ |
| `true` | `false` | `true` | ✓ | ✓ | 200, 4XX | **Default/Legacy** |
| `true` | `false` | `false` | ✓ | ✗ | 200, 4XX | Reuse errors, retry timeouts |
| `true` | `true` | `true` | ✓ | ✓ | 200, 4XX, TSC[^1] | Cache timeouts, but retry |
| `true` | `true` | `false` | ✓ | ✗ | 200, 404, TSC[^1] | **Fast re-runs** - cache & reuse all |
| `false` | `false` | (N/A) | ✗ | N/A | Nothing | **Default skip** - no cache |
| `false` | `true` | (N/A) | ✗ | N/A | unchecked links | **Link discovery** |

[^1]:
TSC = Tool-specific status code used by htmltest. See
`@docs/tasks/design.md` for details.

### Key Insights

- `CacheAllExternal` extends caching behavior in **both** modes:
- When `CheckExternal: true` → Also caches timeouts (408)
- When `CheckExternal: false` → Caches discovered links (0)
- When `CheckExternal: true` → Also caches timeouts (as `StatusTimeout`)
- When `CheckExternal: false` → Caches discovered links (as `StatusUnchecked`)
- When `CheckExternal: false`, `RetryCachedErrors` has no effect (nothing to
retry)
- Status code `0` indicates "unchecked/unknown" status
- Status code `408` indicates timeout error
- See `@docs/tasks/design.md` for status code details

## Implementation Steps (TDD Approach)

Expand All @@ -73,14 +76,14 @@ Example workflow:

Add test cases to verify:

- **Discovery mode**: Links cached with status 0 when `CheckExternal: false` and
`CacheAllExternal: true`
- **Discovery mode**: Links cached with `StatusUnchecked` when
`CheckExternal: false` and `CacheAllExternal: true`
- **Default behavior**: Links NOT cached when `CacheAllExternal: false`
- **Ignored URLs**: Ignored URLs still not cached even with
`CacheAllExternal: true`
- **Query string handling**: Query string stripping applied correctly
- **Timeout caching**: Timeouts cached as 408 when `CheckExternal: true` and
`CacheAllExternal: true`
- **Timeout caching**: Timeouts cached with `StatusTimeout` when
`CheckExternal: true` and `CacheAllExternal: true`

### 2. Add Configuration Option

Expand All @@ -102,7 +105,7 @@ Two modifications needed:
- Modify `checkExternal()` function (starting at line 129)
- When `!hT.opts.CheckExternal` is true:
- Check if `hT.opts.CacheAllExternal` is enabled
- If so, cache discovered links with status 0
- If so, cache discovered links with `StatusUnchecked`
- Apply URL processing (strip query string if configured)
- Skip ignored URLs (respect `isURLIgnored()` check)

Expand All @@ -111,17 +114,18 @@ Two modifications needed:
- In timeout handling code (around line 201-210)
- Change condition from `if !hT.opts.RetryCachedErrors` to
`if hT.opts.CacheAllExternal`
- This caches timeouts when CacheAllExternal is enabled
- Save with `StatusTimeout` instead of not caching

### 4. Update Documentation

**File**: `README.md`

- Add new row in the configuration options table (around line 145, after
`CheckExternal`)
- Format: `| \`CacheAllExternal\` | Cache all external links including timeouts
(when checking) and unchecked links (when not checking). Useful for fast
re-runs and link discovery. | \`false\` |`
- Description: Cache all external links including timeouts (when checking is
enabled) and unchecked links (when checking is disabled). Useful for fast
re-runs and link discovery.
- Default: `false`

## Key Design Decisions

Expand All @@ -131,10 +135,10 @@ Two modifications needed:
naturally
- **Clear semantics**: "CacheAll" clearly means "cache everything, not just
successes"
- **Status codes**:
- `0` = unchecked/unknown (discovery mode)
- `408` = timeout error (check mode)
- Other codes = actual HTTP responses
- **Status codes**: See `@docs/tasks/design.md` for details
- `StatusUnchecked` = unchecked/unknown (discovery mode)
- `StatusTimeout` = timeout error (check mode)
- Positive codes = actual HTTP responses
- **Respects existing patterns**:
- URL ignore patterns (`IgnoreURLs`)
- Query string stripping (`StripQueryString`)
Expand Down
49 changes: 37 additions & 12 deletions docs/tasks/migrate-status-codes.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,29 @@
title: Migrate Status Codes from 408 to -10
date: 2025-10-18
lastmod: 2025-10-18
status: in-progress
status: completed
---

## Progress Summary

**✅ COMPLETED** - All phases of TDD cycle complete:

**GREEN Phase (using hard-coded -10):**

- ✅ Updated test to expect -10 instead of 408
- ✅ Updated cache writing to save -10 for timeouts
- ✅ Updated cache reading to handle both 408 (legacy) and -10 (new)
- ✅ All tests passing

**REFACTOR Phase (named constants):**

- ✅ Created `htmltest/statuscodes.go` with `StatusTimeout` and
`StatusUnchecked` constants
- ✅ Replaced magic number `-10` with `StatusTimeout` in code and tests
- ✅ Added helper functions: `IsHTTPStatus()`, `IsUnchecked()`, `IsToolError()`
- ✅ All tests pass (7 cache tests + full suite)
- ✅ Cache file verified to contain -10

# Migrate Status Codes from 408 to -10

## Overview
Expand All @@ -17,7 +37,7 @@ client-side errors.

- **Old**: `408` - Ambiguous (is it from the server or our timeout?)
- **New**: `-10` - Clearly a tool-specific timeout error
- Aligns with new status code conventions (see `tasks/design.md`)
- Aligns with new status code conventions (see `@docs/tasks/design.md`)

## Implementation Steps (TDD Approach)

Expand Down Expand Up @@ -210,16 +230,21 @@ All tests should pass. Verify:

## Checklist (TDD Order)

- [ ] Write tests for constants/helpers in `htmltest/statuscodes_test.go` (RED)
- [ ] Update `htmltest/check-link-cache_test.go` to expect -10 (RED)
- [ ] Run tests → should FAIL
- [ ] Create `htmltest/statuscodes.go` with constants and helpers (GREEN)
- [ ] Run tests → should PASS
- [ ] Update `htmltest/check-link.go` to write -10 for timeouts (GREEN)
- [ ] Update `htmltest/check-link.go` to read both 408 and -10 (GREEN)
- [ ] Update `statusCodeValid` function if needed (GREEN)
- [ ] Run full test suite → should PASS
- [ ] Manual verification: check cache file contains -10
**All Steps Completed:**

- [x] Update `htmltest/check-link-cache_test.go` to expect -10 (RED)
- [x] Run tests → FAILED as expected
- [x] Update `htmltest/check-link.go` to write -10 for timeouts (GREEN)
- [x] Update `htmltest/check-link.go` to read both 408 and -10 (GREEN)
- [x] Run tests → PASS
- [x] Manual verification: cache file contains -10
- [x] Create `htmltest/statuscodes.go` with constants and helpers (REFACTOR)
- [x] Replace hard-coded `-10` in `check-link.go` with `StatusTimeout`
(REFACTOR)
- [x] Replace hard-coded `-10` in `check-link-cache_test.go` with
`StatusTimeout` (REFACTOR)
- [x] Run all cache tests → PASS
- [x] Run full test suite → PASS

## Backward Compatibility

Expand Down
55 changes: 46 additions & 9 deletions docs/tasks/summary.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,30 @@ without retrying them on subsequent runs.
retrying
- **Backward compatibility**: Default `true` maintains existing behavior

### 2. Timeout Caching (#2)
### 2. Timeout Caching

**Status**: Completed

Extended caching behavior to save timeout errors (HTTP 408 status) to the
refcache when `RetryCachedErrors: false`.
Extended caching behavior to save timeout errors to the refcache when
`RetryCachedErrors: false`.

- Timeouts are cached as status code 408 (`http.StatusRequestTimeout`)
- Timeouts are cached as status code `-10` (tool-specific timeout code)
- Prevents repeated timeout attempts on unreachable URLs
- Works in conjunction with `RetryCachedErrors` option

### 3. URL Encoding in Cache (#4)
### 3. Status Code Migration

**Status**: Completed

Migrated timeout status codes from `408` to `-10` for clarity.

- Created `htmltest/statuscodes.go` with status code constants
- `StatusTimeout = -10` (tool-specific timeout)
- `StatusUnchecked = 0` (for future use)
- Helper functions: `IsHTTPStatus()`, `IsUnchecked()`, `IsToolError()`
- See `@docs/tasks/design.md` for status code conventions

### 4. URL Encoding in Cache

**Status**: Completed

Expand All @@ -63,11 +75,36 @@ Added repository-specific suffix to version IDs for better build tracking.
- Added `Makefile` for build automation
- Added `CONTRIBUTING.md` with development guidelines

## Infrastructure & Tooling (continued)

### TDD Makefile Targets

**Status**: Completed

Added Makefile targets for TDD workflow:

- `make test-tdd TEST_RUN=TestName` - Run test with clean cache
- `make test-tdd-cache TEST_RUN=TestName` - Run test and show cache state
- `make clean-cache` - Remove refcache file
- Supports pattern matching for running multiple tests

### Documentation Structure

**Status**: Completed

Organized documentation under `docs/`:

- `docs/ops/` - Operational documentation (session-start, agent-guidance)
- `docs/tasks/` - Task and feature documentation
- `AGENTS.md` - AI agent guidance (Cursor auto-loads)
- `.github/copilot-instructions.md` - GitHub Copilot support

## Planned Features

Most features are documented under the `tasks/` directory.
Most features are documented under the `docs/tasks/` directory.

### CacheUncheckedExternal (In Progress)
### CacheAllExternal (In Progress)

See `tasks/cache-unchecked-external-links.md` for details. This feature will
allow caching external links without checking them when `CheckExternal: false`.
See `@docs/tasks/cache-unchecked-external-links.md` for details. This feature
will allow caching external links without checking them when
`CheckExternal: false`.
6 changes: 3 additions & 3 deletions htmltest/check-link-cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,13 @@ func TestTimeoutIsCached(t *testing.T) {
tExpectIssueCount(t, hT, 1)
tExpectIssue(t, hT, "request exceeded our ExternalTimeout", 1)

// Verify the timeout was saved to cache with -10 (tool-specific timeout code)
// Verify the timeout was saved to cache with StatusTimeout
cR, ok := hT.refCache.Get("http://5.6.7.8")
if !ok {
t.Error("expected timeout to be cached when RetryCachedErrors is false, but it wasn't")
}
if cR.StatusCode != -10 {
t.Errorf("expected status code -10 (timeout), got %d", cR.StatusCode)
if cR.StatusCode != StatusTimeout {
t.Errorf("expected status code %d (StatusTimeout), got %d", StatusTimeout, cR.StatusCode)
}
}

Expand Down
6 changes: 2 additions & 4 deletions htmltest/check-link.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ func (hT *HTMLTest) checkExternal(ref *htmldoc.Reference) {
if err != nil {
if strings.Contains(err.Error(), "Client.Timeout") {
if !hT.opts.RetryCachedErrors {
hT.refCache.Save(urlStr, -10) // Tool-specific timeout code
hT.refCache.Save(urlStr, StatusTimeout)
}
hT.issueStore.AddIssue(issues.Issue{
Level: issueLevel,
Expand Down Expand Up @@ -263,9 +263,7 @@ func (hT *HTMLTest) checkExternal(ref *htmldoc.Reference) {
Message: http.StatusText(statusCode),
Reference: ref,
})
case http.StatusRequestTimeout: // Legacy: old cache files may have 408
fallthrough
case -10: // Tool-specific timeout code
case StatusTimeout:
hT.issueStore.AddIssue(issues.Issue{
Level: issueLevel,
Message: "request exceeded our ExternalTimeout (cached)",
Expand Down
Loading
Loading