path: root/ISSUES.md

# Known Issues

A living catalog of identified issues in this extension. New findings should be added below; resolved items are kept for reference (move to **Fixed** under the matching severity, do not delete).

## Conventions

- Severity reflects user impact and likelihood. **High** items can hang the editor, drop work, or expose risks. **Medium** items degrade reliability or maintainability. **Low** items are polish or edge cases.
- Each entry has a status, a location (function or file path with optional line), and a one-paragraph description. Add resolution notes when fixing.
- Status markers:
  - `OPEN` — needs work
  - `FIXED` — addressed
  - `DOCUMENTED` — accepted as a limitation, called out in user docs
  - `WONT FIX` — declined with reasoning

---

## High

### FIXED — `launchInTerminal` could hang on synchronous start failure

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `launchInTerminal`
- **Description**: `process.start()` was called outside any try/catch. A synchronous throw left the returned `Promise` unresolved, so the `runInTerminal` command would hang forever.
- **Resolution**: Wrapped in try/catch; on throw, resolves with `{status: -1, stderr: String(error)}` so the existing caller's `result.status !== 0` branch surfaces the failure.

### FIXED — Fire-and-forget `pushConfiguration` swallowed rejections

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `ZigLanguageServer.start` and `ZigLanguageServer.observeConfig` (three call sites)
- **Description**: All three sites used `void this.pushConfiguration()`. Any rejection became an unhandled promise rejection with no log trace.
- **Resolution**: Replaced with `.catch((error) => console.error(...))`.

### FIXED — `runProcess` had no rejection path for stuck child processes

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `runProcess`
- **Description**: The promise resolved only on `onDidExit`. A child process that started but never exited (e.g. a misbehaving `build.zig`) would stall any caller indefinitely. `provideTasks` was the most exposed.
- **Resolution**: Added optional `timeoutMs` option. On timeout, sends `SIGTERM` (guarded for missing `process.signal`) and resolves with `{status: -1, stderr: "...[timeout after Nms]"}`. Applied 60s timeout to the `zig build --list-steps` call in `stepCache.fetch`.

---

## Medium

### Stability

#### FIXED — Dispose loops have no per-item try/catch

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `deactivate` and `ZigLanguageServer.dispose`
- **Description**: Both iterate disposables and call `.dispose()` without protection. One throwing disposable aborted the rest of the loop, leaking later registrations and listeners.
- **Resolution**: Wrapped each `disposable.dispose()` call in a try/catch that logs the error and continues iterating.

#### FIXED — ZLS crashed immediately on startup when zig was discovered automatically

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `ZigLanguageServer.start` (~L980)
- **Description**: `syncWorkspaceZlsConfiguration` was called immediately after
  `client.start()`. Writing `zls.zig_exe_path` to `nova.workspace.config` while the
  LanguageClient is newly registered triggers Nova to fire workspace config change events.
  On the first activation those keys did not yet exist (set = add, no event); on the
  second activation the keys persisted from the previous session (set = update, event
  fires), which caused Nova to tear down the freshly opened ZLS stdio pipe before the
  LSP handshake completed — ZLS exits with code 1 (`EndOfStream`). This was masked when
  `findOnPath` failed (see next entry) because `zig_exe_path` was never set, so the write
  was always a no-op remove rather than a set or update.
- **Resolution**: Removed the `syncWorkspaceZlsConfiguration` call from `start()`. Added
  a cleanup call with empty settings in `dispose()` (after observers are removed) so that
  each new activation always starts with absent `zls.*` keys.

#### FIXED — `findOnPath` always returned null when no explicit path was configured

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `findOnPath` (~L597)
- **Description**: Nova's `Process` constructor does not inherit the parent's environment
  automatically. The bare `new Process("/usr/bin/env", { args: ["which", name] })` call
  received a stripped environment with no `PATH`, so `which` could not locate any binary
  — including those in `/opt/homebrew/bin` that are clearly present in `nova.environment.PATH`.
- **Resolution**: Read `nova.environment.PATH` and pass it explicitly via the `env` option
  so the `which` subprocess searches the same PATH Nova itself sees.

#### OPEN — `pushConfiguration` re-resolves Zig path on every observation

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `ZigLanguageServer.pushConfiguration` (~L1040)
- **Description**: On rapid config-change storms, each observation calls `resolveSettings → resolveExecutable → findOnPath`, fanning out N parallel `which zig` subprocesses. The generation guard discards stale results but the work still runs.
- **Suggested fix**: Debounce the call (small `setTimeout` consolidator) or cache the resolved zig path for the lifetime of a generation.

#### OPEN — `start()` cannot distinguish handshake failure from runtime crash

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `ZigLanguageServer.start` (~L1000)
- **Description**: The try/catch around `client.start()` only fires when `start()` itself throws synchronously. If the language client starts successfully but the LSP fails handshake, the user only sees the generic "stopped unexpectedly" warning (via `onDidStop`), never the more informative "Unable to start" message.
- **Suggested fix**: Listen for an early failure on `onDidStop` within a brief window after start and emit the more informative warning.

#### OPEN — `stop()` removes from subscriptions before `client.stop()` settles

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `ZigLanguageServer.stop` (~L1058)
- **Description**: If `client.stop()` is async in Nova's API, removal happens before the stop completes. Not observed to cause issues in practice; flagging for verification.
- **Action**: Confirm whether Nova's `LanguageClient.stop()` is sync or returns a Promise; document the assumption either way.

### Logic

#### FIXED — `parseProjectName` matched commented-out `.name` lines

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `parseProjectName`
- **Description**: The regex did not strip line comments. A `build.zig.zon` with `// .name = "evil"` above the real name field would match the comment first.
- **Resolution**: Strip `//`-to-EOL comments via `content.replace(/\/\/[^\n]*/g, "")` before matching.

#### FIXED — Step discovery failures were invisible

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `stepCache.fetch`
- **Description**: When `--list-steps` failed (non-zero exit, timeout), the user saw a sparse task list with no signal that discovery was broken.
- **Resolution**: Added a per-cwd `lastWarnedAt` map that emits a `console.warn` with the exit status and a 500-char stderr excerpt at most once per 5 minutes. `invalidate()` clears the throttle so manual cache reset re-enables warnings.

#### OPEN — `resolveCleanAction` skips uninstall on cold cache

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `resolveCleanAction` (~L1202)
- **Description**: The `zig build uninstall` step only runs when `stepCache.entries` already contains the cwd and includes "uninstall". A first-time clean (cold cache) silently skips uninstall and only removes the cache directories.
- **Suggested fix**: Either accept the asymmetry and document it in [Zig.novaextension/MANUAL.md](Zig.novaextension/MANUAL.md), or warm the cache before deciding.

#### DOCUMENTED — `getConfigValue` treats empty string as missing

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `getConfigValue` (~L62)
- **Description**: A user cannot explicitly *unset* a global value by setting an empty workspace override; they must use Nova's "remove from workspace" UI. Acceptable but worth a note for power users.
- **Action**: Document in [Zig.novaextension/MANUAL.md](Zig.novaextension/MANUAL.md) settings reference.

#### OPEN — Step name regex rejects names starting with a digit

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `stepCache.fetch` filter (~L457) and `resolveBuildStepAction` (~L1290)
- **Description**: `/^[A-Za-z_][\w-]*$/` is stricter than Zig itself, which accepts step names beginning with a digit.
- **Action**: Either relax to allow leading digits, or document as an extension-imposed constraint.

### Security

#### FIXED — `resolveCleanAction` built its shell string by hand

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `resolveCleanAction`
- **Description**: The uninstall+rm command was assembled with manual `quoteShellArgument` calls. Safe today but fragile: a future contributor adding a new arg could forget to quote.
- **Resolution**: Compose the command from two `buildShellCommand` calls so all quoting routes through one helper. The intentional `;` (rather than `&&`) between the two is preserved and now commented — rm should run even if uninstall fails.

### Documentation

#### OPEN — Class methods lack JSDoc

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `ZigLanguageServer` (~L860) and `ZigTaskAssistant` (~L1080)
- **Description**: ~50% of utility helpers have JSDoc; 0% of class methods do. The prior review plan tracks this work; see [~/.claude/plans/i-want-to-perform-idiopotent-crystal.md](file:///Users/david/.claude/plans/i-want-to-perform-idiopotent-crystal.md) for the backlog.
- **Action**: Resume Groups 1, 5, 5a, 6, 7 from the JSDoc plan.

#### DOCUMENTED — Auto-discovered tasks cannot open the report automatically

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `ZigTaskAssistant.provideTasks`
- **Description**: Tasks returned from `provideTasks()` do not support the `openLogOnRun` setting. The property exists on task JSON files for user-created tasks but is not honoured on `Task` objects constructed in JS — setting `task.openLogOnRun` has no effect. Nova's extension API simply does not expose this for programmatic tasks.
- **Action**: Documented in [Zig.novaextension/README.md](Zig.novaextension/README.md) Known Issues.

#### OPEN — Issue matcher pattern not documented

- **Location**: [Zig.novaextension/extension.json](Zig.novaextension/extension.json) issue matcher block, [Zig.novaextension/README.md](Zig.novaextension/README.md), [Zig.novaextension/MANUAL.md](Zig.novaextension/MANUAL.md)
- **Description**: Users authoring custom tasks have no reference for what fields the `zig.compiler` matcher captures.
- **Suggested fix**: Add a short "Issue matcher" section to [Zig.novaextension/MANUAL.md](Zig.novaextension/MANUAL.md) documenting the regex, captures, and how to use it from custom tasks.

### Structure

#### OPEN — `Scripts/main.js` is a 1,398-line monolith

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js)
- **Description**: Section dividers help, but two ~300-line classes plus utilities in one file impede navigation and review.
- **Suggested split**:
  - `main.js` — activate/deactivate/registerCommands
  - `config.js` — config helpers and `CONFIG_KEYS`
  - `paths.js` — path resolution and project name parsing
  - `exec.js` — `runProcess`, executable resolvers, shell utilities, `launchInTerminal`
  - `stepCache.js` — the stepCache object
  - `languageServer.js` — `ZigLanguageServer` and `syncWorkspaceZlsConfiguration`
  - `taskAssistant.js` — `ZigTaskAssistant` and `buildZigArgv`/`resolveStep`/`resolveRunStep`
- **Action**: Defer until before the next major feature.

---

## Low

### Stability

#### OPEN — Dead `warnedMissing.delete("zig")`

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `ZigLanguageServer.start` (~L1010)
- **Description**: `warnMissingTool` is only ever called with `"zls"`; the matching `delete("zig")` line never has anything to remove.
- **Action**: Either delete the line, or wire up a missing-zig warning path through `warnMissingTool`.

#### FIXED — `buildShellCommand` uses `;` between cd and command



- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `buildShellCommand`
- **Description**: If `cd` fails (e.g. cwd was deleted between resolution and execution), the command runs in the wrong directory silently.
- **Resolution**: Changed `segments.join("; ")` to `segments.join(" && ")` so a failed `cd` aborts the command.

### Logic

#### FIXED — `localizeText` ignored its `fallback` parameter

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `localizeText` (~L542)
- **Description**: When a localization key was missing, the function returned `"Localization missing for ${key}"` instead of the caller-supplied English fallback. All callers pass real English strings as fallbacks.
- **Resolution**: Changed the missing-key branch to `return fallback !== undefined ? String(fallback) : key`.

#### FIXED — Dead `normalized === ""` branch in `resolveCleanPaths`

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `resolveCleanPaths` (~L307)
- **Description**: `nova.path.normalize(cwd)` can never return `""` when `cwd` starts with `"/"` (already guaranteed by the preceding guard), making the empty-string check dead code.
- **Resolution**: Removed `|| normalized === ""` from the condition.

#### FIXED — Step validated after `await` in `resolveBuildStepAction`

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `resolveBuildStepAction` (~L1420)
- **Description**: `resolveZigExecutable()` (async, runs `which zig`) was called before validating the `step` argument, wasting a subprocess on invalid input.
- **Resolution**: Moved the step null/regex guard to the top of the function, before the `await`.

#### FIXED — `USER_OPTION_REGEX` permitted trailing `=` with empty value

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `USER_OPTION_REGEX` (~L9)
- **Description**: `foo=` was accepted and produced `-Dfoo=`. Zig errors on empty option values.
- **Resolution**: Changed `=.*` to `=.+` so at least one character is required after `=`.

#### OPEN — Build step name has no length cap

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `resolveBuildStepAction` (~L1290)
- **Description**: A 1MB step name would pass the regex check.
- **Suggested fix**: Add a length anchor like `{1,128}`.

#### FIXED — `debounceMs` had no upper bound

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `resolveWatchAction`; [Zig.novaextension/extension.json](Zig.novaextension/extension.json) `debounceMs` schema
- **Description**: `Number.isFinite` and `>= 0` were checked, but `1e18` would be accepted.
- **Resolution**: Added `&& n <= 60000` guard in code; added `"min": 0, "max": 60000` to the manifest schema.

#### OPEN — `runProcess` joins stdout/stderr lines without separator

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `runProcess` (~L511)
- **Description**: `stdout.push(line)` then `stdout.join("")`. If Nova's `onStdout` strips trailing newlines, two lines `"foo"` and `"bar"` concatenate to `"foobar"`.
- **Action**: Verify Nova's behavior; if newlines are stripped, join with `"\n"`.

### Security

#### OPEN — `build-parser.sh` lacks explicit `xcrun` failure check

- **Location**: [Zig.novaextension/Scripts/build-parser.sh](Zig.novaextension/Scripts/build-parser.sh) `SDKROOT` line
- **Description**: An empty `SDKROOT` from a failed `xcrun` produces cryptic downstream errors.
- **Suggested fix**: Check `xcrun --show-sdk-path` exit status explicitly and abort with a clear message.

#### OPEN — Hardening tip for untrusted workspaces missing from MANUAL

- **Location**: [Zig.novaextension/MANUAL.md](Zig.novaextension/MANUAL.md)
- **Description**: Login-shell PATH discovery means a workspace (or a shell profile that reads
  workspace-local `.envrc` / `direnv` / `.env` files) that injects `.` early in PATH could
  run a malicious local `zig`. The same surface area exists for `createAction` (tasks). Already
  mitigated by the per-workspace `zig.toolchain.zig-path` setting; just not flagged for users.
- **Suggested fix**: Add a brief security note recommending pinned executable paths in untrusted workspaces.

### Documentation

#### OPEN — README troubleshooting omits common tool-not-found cases

- **Location**: [Zig.novaextension/README.md](Zig.novaextension/README.md) Troubleshooting
- **Description**: Mentions parser issues but not "ZLS not found" or "lldb-dap not found" recovery paths, which are the most common user-reported pain points.
- **Suggested fix**: Add brief recovery steps for both.

#### OPEN — CHANGELOG entries don't link to MANUAL sections

- **Location**: [Zig.novaextension/CHANGELOG.md](Zig.novaextension/CHANGELOG.md)
- **Description**: Polish: feature entries could deep-link to relevant [Zig.novaextension/MANUAL.md](Zig.novaextension/MANUAL.md) sections so users can discover usage.
- **Action**: Optional.

### Structure

#### OPEN — Dual escape paths for shell strings

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `buildShellCommand` and `buildIssueNormalizedCommand`
- **Description**: Both functions quote arguments but live separately. Future changes risk drift.
- **Suggested fix**: Compose `buildIssueNormalizedCommand` on top of `buildShellCommand`.

#### OPEN — `createAction` defined mid-class

- **Location**: [Zig.novaextension/Scripts/main.js](Zig.novaextension/Scripts/main.js), `ZigTaskAssistant.createAction`
- **Description**: Central helper used by every `resolve*Action`; defined between two of its callers. Discoverability nit.
- **Action**: Move to top of class above `resolveTaskAction`.