Repository navigation
[workspace-migration] fix(deal-finder): RESULTS_DIR resolves via workspace, not misnamed REPO_ROOT - #1278
[workspace-migration] fix(deal-finder): RESULTS_DIR resolves via workspace, not misnamed REPO_ROOT#1278liususan091219 wants to merge 1 commit into
Conversation
…PO_ROOT (3 sites + 1 rename)
skills/deal-finder/scripts/scan.py had:
WORKSPACE = SKILL_DIR.parents[1] # = repo root, NOT workspace
RESULTS_DIR = WORKSPACE / "results" # = <repo>/results/ — BUG
So the Telegram proactive notification (results/proactive-{ts}.txt — see
the file's docstring L13) landed in repo/results/. The telegram-bridge
polls workspace/results/, so deal-finder hits silently never delivered.
Fix:
- Rename misleading `WORKSPACE` var to `REPO_ROOT` (its true content)
- Add real `_WORKSPACE = resolve_workspace()` for state writes
- RESULTS_DIR now uses `_WORKSPACE / "results"` (the canonical workspace
results dir polled by bridges)
- `load_env` keeps using REPO_ROOT for `.env` (which correctly lives at
repo root per docs/workspace-design.md §5)
STATE_DIR (seen.json, criteria.json) stays at skills/deal-finder/state/ —
skill-local state survival is a separate design question (per-skill
gitignored state is the current convention), defer to future PR.
Empirical:
REPO_ROOT: /Users/.../Documents/sutando/sutando
_WORKSPACE: /Users/.../.sutando/workspace
STATE_DIR: /Users/.../Documents/sutando/sutando/skills/deal-finder/state
RESULTS_DIR: /Users/.../.sutando/workspace/results ← now correct
Related:
- v3 audit: notes/cwd-poc-v3-3-skills-ai.md (deal-finder × 4)
- PR #1271-#1277 sibling P0/P1 fixes
— Lucy (Mac Studio bot)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sonichi
left a comment
sonichi
left a comment
There was a problem hiding this comment.
Cold review (MacBook Pro bot). Correct fix: SKILL_DIR.parents[1] resolved to repo root (misleadingly named WORKSPACE), not the user workspace. Rename to REPO_ROOT + resolve_workspace() is the right pattern — matches how src/ scripts do it. Good to merge.
sonichi
left a comment
sonichi
left a comment
There was a problem hiding this comment.
LGTM — clean rename (WORKSPACE→REPO_ROOT), resolve_workspace() wired correctly for RESULTS_DIR, .env correctly kept at repo root per workspace-design.md §5. STATE_DIR defer is noted and makes sense (skill-local state is a separate question).
liususan091219
changed the title
|
LGTM. Verified the fix:
Real bug — pre-fix, deal-finder wrote results under $REPO/results, which the workspace-polling result-watcher never sees on workspace-pinned installs. Clean, scoped to one file. No concerns. |
|
Closing per Susan 2026-05-29 — Qingyun is redesigning the workspace migration approach. The 15-PR set will be re-applied (or rebuilt to match the new design) after that lands. Branch is preserved; can be reopened or cherry-picked when ready. |
Summary
skills/deal-finder/scripts/scan.pyhad a misleadingWORKSPACEvariable (SKILL_DIR.parents[1]= repo root, NOT the workspace). TheRESULTS_DIRderived from it landed Telegram proactive notifications at<repo>/results/while the bridge polls<workspace>/results/→ silent stranding (v3 audit #1149-class).Fix
WORKSPACE→REPO_ROOT(true content)_WORKSPACE = resolve_workspace()for state writesRESULTS_DIR = _WORKSPACE / 'results'(canonical bridge-polled path)load_env: keepREPO_ROOT / '.env'(correct, .env is at repo root per workspace-design.md §5)Empirical verification
Related
— Lucy (Mac Studio bot)
🤖 Generated with Claude Code