Skip to content

Commit 0c63e22

Browse files
committed
checkpoint: TP-018 R006 fixes #3 (source detection type guards) and #4 (number validation > 0), dynamic JSON-only footer
1 parent ea72651 commit 0c63e22

3 files changed

Lines changed: 85 additions & 24 deletions

File tree

extensions/taskplane/settings-tui.ts

Lines changed: 53 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -338,18 +338,24 @@ export function detectFieldSource(
338338
rawProjectConfig: Record<string, any> | null,
339339
rawPrefs: Record<string, any> | null,
340340
): FieldSource {
341-
// L2 check for dual-layer and L2-only fields
341+
// L2 check for dual-layer and L2-only fields.
342+
// Type guards MUST match extractAllowlistedPreferences() in config-loader.ts
343+
// to avoid showing "(user)" for values that the merge layer would reject.
342344
if ((field.layer === "L1+L2" || field.layer === "L2") && field.prefsKey && rawPrefs) {
343345
const prefVal = rawPrefs[field.prefsKey];
344346
if (field.fieldType === "string") {
345-
// String rule: non-undefined AND non-empty → (user)
346-
if (prefVal !== undefined && prefVal !== "") return "user";
347+
// String rule: must be typeof string, non-empty → (user)
348+
// Matches: `typeof raw.X === "string"` AND applyUserPreferences `val !== "" `
349+
if (typeof prefVal === "string" && prefVal !== "") return "user";
347350
} else if (field.fieldType === "enum") {
348-
// Enum rule: any defined value → (user)
349-
if (prefVal !== undefined) return "user";
351+
// Enum rule: must be a valid enum value from the field's values array.
352+
// Matches extractAllowlistedPreferences which checks exact enum membership
353+
// (e.g., raw.spawnMode === "tmux" || raw.spawnMode === "subprocess").
354+
if (prefVal !== undefined && field.values && field.values.includes(String(prefVal))) return "user";
350355
} else if (field.fieldType === "number") {
351-
// Number rule: any defined value → (user)
352-
if (prefVal !== undefined) return "user";
356+
// Number rule: must be typeof number and finite → (user)
357+
// Matches: `typeof raw.X === "number" && Number.isFinite(raw.X)`
358+
if (typeof prefVal === "number" && Number.isFinite(prefVal)) return "user";
353359
}
354360
}
355361

@@ -427,8 +433,8 @@ export function validateFieldInput(field: FieldDef, input: string): ValidationRe
427433
switch (field.fieldType) {
428434
case "number": {
429435
const num = Number(input.trim());
430-
if (!Number.isFinite(num) || num < 0) {
431-
return { valid: false, error: "Must be a positive number" };
436+
if (!Number.isFinite(num) || num <= 0) {
437+
return { valid: false, error: "Must be a positive integer" };
432438
}
433439
// Integer check for most number fields
434440
if (!Number.isInteger(num)) {
@@ -979,18 +985,45 @@ function truncateLine(text: string, width: number): string {
979985
// ── JSON-Only Footer ─────────────────────────────────────────────────
980986

981987
/**
982-
* Generate a footer note about JSON-only fields related to a section.
988+
* Map from section name to the config subsection prefixes it covers.
989+
* Used to dynamically discover JSON-only sibling fields.
983990
*/
984-
function getJsonOnlyFooterForSection(section: SectionDef, _config: TaskplaneConfig): string | null {
985-
// Map sections to their JSON-only siblings
986-
const sectionJsonOnly: Record<string, string[]> = {
987-
"Assignment": ["sizeWeights"],
988-
"Pre-Warm": ["commands", "always"],
989-
"Merge": ["verify"],
990-
};
991+
const SECTION_CONFIG_PREFIXES: Record<string, string[]> = {
992+
"Orchestrator": ["orchestrator.orchestrator"],
993+
"Dependencies": ["orchestrator.dependencies"],
994+
"Assignment": ["orchestrator.assignment"],
995+
"Pre-Warm": ["orchestrator.preWarm"],
996+
"Merge": ["orchestrator.merge"],
997+
"Failure Policy": ["orchestrator.failure"],
998+
"Monitoring": ["orchestrator.monitoring"],
999+
"Worker": ["taskRunner.worker"],
1000+
"Reviewer": ["taskRunner.reviewer"],
1001+
"Context Limits": ["taskRunner.context"],
1002+
};
9911003

992-
const jsonFields = sectionJsonOnly[section.name];
993-
if (!jsonFields || jsonFields.length === 0) return null;
1004+
/**
1005+
* Generate a footer note about JSON-only fields related to a section.
1006+
*
1007+
* Dynamically discovers uncovered fields under the same config subsection
1008+
* prefix, so new fields added to the schema auto-appear in footers.
1009+
*/
1010+
function getJsonOnlyFooterForSection(section: SectionDef, config: TaskplaneConfig): string | null {
1011+
const prefixes = SECTION_CONFIG_PREFIXES[section.name];
1012+
if (!prefixes) return null;
1013+
1014+
// Find all uncovered leaf fields under these prefixes
1015+
const uncoveredFields: string[] = [];
1016+
walkConfig(config, "", (path, _value) => {
1017+
if (COVERED_PATHS.has(path)) return; // Already editable
1018+
for (const prefix of prefixes) {
1019+
if (path.startsWith(prefix + ".")) {
1020+
// Extract the field name (last segment)
1021+
const fieldName = path.split(".").pop() || path;
1022+
uncoveredFields.push(fieldName);
1023+
}
1024+
}
1025+
});
9941026

995-
return `+ ${jsonFields.join(", ")} (edit JSON directly)`;
1027+
if (uncoveredFields.length === 0) return null;
1028+
return `+ ${uncoveredFields.join(", ")} (edit JSON directly)`;
9961029
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Review Request: Plan Review
2+
3+
You are reviewing an implementation plan for a Project task.
4+
You have full tool access — use `read` to examine files and `bash` to run commands.
5+
6+
## Task Context
7+
8+
- **Task PROMPT:** C:\dev\taskplane\.worktrees\taskplane-wt-henrylach-1\taskplane-tasks\TP-018-settings-tui-command\PROMPT.md
9+
- **Task STATUS:** C:\dev\taskplane\.worktrees\taskplane-wt-henrylach-1\taskplane-tasks\TP-018-settings-tui-command\STATUS.md
10+
- **Step being planned:** Step 3: Implement Write-Back
11+
12+
## Instructions
13+
14+
1. Read the PROMPT.md for full requirements
15+
2. Read STATUS.md for progress so far
16+
3. Check relevant source files for existing patterns:
17+
18+
19+
## Project Standards
20+
21+
22+
23+
## Output
24+
25+
Write your review to: `C:\dev\taskplane\.worktrees\taskplane-wt-henrylach-1\taskplane-tasks\TP-018-settings-tui-command\.reviews\R007-plan-step3.md`

taskplane-tasks/TP-018-settings-tui-command/STATUS.md

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# TP-018: /settings TUI Command — Status
22

3-
**Current Step:** Step 2: Implement /settings Command
3+
**Current Step:** Step 3: Implement Write-Back
44
**Status:** ✅ Complete
55
**Last Updated:** 2026-03-17
66
**Review Level:** 2
@@ -47,8 +47,8 @@
4747
- [x] Verify tests pass (existing workspace-config test 5.5 ctx.cwd constraint)
4848
- [x] R006 fix #1: Use execCtx.workspaceRoot (not repoRoot) for config reads — workspace mode reads config from workspace root
4949
- [x] R006 fix #2: Generate Advanced section items dynamically from schema/default config instead of hardcoded list
50-
- [ ] R006 fix #3: Source detection must use same type guards as extractAllowlistedPreferences (reject invalid pref types)
51-
- [ ] R006 fix #4: Number validation must enforce num > 0 (not num >= 0) to match "positive integers" contract
50+
- [x] R006 fix #3: Source detection must use same type guards as extractAllowlistedPreferences (reject invalid pref types)
51+
- [x] R006 fix #4: Number validation must enforce num > 0 (not num >= 0) to match "positive integers" contract
5252
- [ ] R006 fix #5: Add unit tests for detectFieldSource, getFieldDisplayValue, validateFieldInput
5353
- [x] Verify tests still pass after R006 fixes
5454

@@ -61,7 +61,7 @@
6161
---
6262

6363
### Step 3: Implement Write-Back
64-
**Status:** ⬜ Not Started
64+
**Status:** 🟨 In Progress
6565

6666
- [ ] Layer 1 → project config, Layer 2 → user preferences
6767
- [ ] Confirmation prompt for project config changes
@@ -147,6 +147,9 @@
147147
| 2026-03-17 17:58 | Worker iter 3 | done in 624s, ctx: 55%, tools: 81 |
148148
| 2026-03-17 18:01 | Review R006 | code Step 2: REVISE |
149149
| 2026-03-17 18:02 | Review R006 | code Step 2: REVISE |
150+
| 2026-03-17 18:06 | Worker iter 3 | done in 292s, ctx: 26%, tools: 39 |
151+
| 2026-03-17 18:06 | Step 2 complete | Implement /settings Command |
152+
| 2026-03-17 18:06 | Step 3 started | Implement Write-Back |
150153

151154
## Blockers
152155
*None*

0 commit comments

Comments
 (0)