Skip to content

Feature/#108#109

Merged
vkehfdl1 merged 13 commits intodevfrom
feature/#108
Apr 12, 2026
Merged

Feature/#108#109
vkehfdl1 merged 13 commits intodevfrom
feature/#108

Conversation

@vkehfdl1
Copy link
Copy Markdown
Contributor

Summary

  • add a new geeknews-search skill for GeekNews RSS/Atom list/search/detail workflows
  • add a fixture-backed Python helper plus unit/docs regressions for the public feed shape
  • wire README/install/source docs so the new skill is discoverable and documented

Verification

  • PYTHONPATH=.:scripts python3 -m unittest scripts.test_geeknews_search
  • node --test scripts/skill-docs.test.js
  • python3 scripts/geeknews_search.py list --limit 3
  • python3 scripts/geeknews_search.py search --query Claude --limit 3
  • python3 scripts/geeknews_search.py detail --id 28439
  • npm run ci

hyeongr and others added 12 commits April 7, 2026 19:35
생활쓰레기 조회 스킬 문서와 기능 가이드를 추가하고, 프록시 라우트를 구현해 조회 흐름을 완성했다. 설치/설정 문서도 스킬 사용 흐름에 맞게 정리했다.

Made-with: Cursor
…x SKILL.md newline

- Drop user-supplied returnType and force "json" upstream so the cache key
  (which omits returnType) cannot be poisoned by alternate response shapes.
- Add server tests covering: missing SGG_NM (400), missing API key (503),
  serviceKey injection + cache hit on second call, returnType=xml override
  ignored, upstream non-200 surfaced as 502.
- Add trailing newline to household-waste-info/SKILL.md.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
vkehfdl1's review on PR #82: skill/docs claimed support for
cond[DAT_CRTR_YMD::*] / cond[DAT_UPDT_PNT::*] filters and an optional
returnType, but the proxy only forwards pageNo, numOfRows, and
cond[SGG_NM::LIKE], and forces returnType=json. Typical user queries
("강남구 쓰레기 배출 요일") only need 시군구 검색, so shrink the
documented contract to match the proxy instead of widening pass-through.

- household-waste-info/SKILL.md: list only proxy-supported params, note
  returnType is server-forced, fix failure modes.
- docs/features/household-waste-info.md: switch base example to the
  proxy route, drop the bare upstream curl, call out unsupported
  filters explicitly.
- docs/install.md, docs/security-and-secrets.md, k-skill-setup/SKILL.md:
  describe the skill as calling the proxy /v1/household-waste/info
  route rather than the raw upstream endpoint.
The 인증/시크릿 column mixed user-side credentials, proxy URL hints,
and "use this fallback" notes — confusing for end users who only need
to know "do I have to log in or not?". Operator-managed secrets that
ship in k-skill-proxy are not the user's problem.

- Rename column to "사용자 로그인" with a one-line preface explaining
  the new contract.
- Reclassify proxy-fronted skills (서울 지하철, 한강 수위, 부동산,
  생활쓰레기, 가장 싼 주유소, 한국 법령 remote endpoint) to 불필요.
- Only SRT, KTX, 토스증권 keep 필요 (real per-user account login).
- Tighten the household-waste-info row to use the proxy-route phrasing
  consistent with the rest of the docs in this PR.
- Update skill-docs tests to assert the new binary classification for
  서울 지하철 and 한국 법령 rows.
The 설명 column was leaking implementation details — k-skill-proxy
routing notes, upstream package names, anti-bot helper mentions —
that don't help a user decide whether the skill does what they want.

Rewrite each row to state only "what this skill does for the user",
dropping references to k-skill-proxy, upstream library names
(real-estate-mcp, kakaocli, daiso-mcp, coupang-mcp, tossctl,
korean-law-mcp, Dynapath helper, Kakao Map anchor, Opinet, etc.) and
proxy route paths. The 사용자 로그인 column already captures the
"do I need credentials?" question, so the description is free to
focus on the capability itself.
- KEDU_INFO_KEY로 /v1/neis/school-search, /v1/neis/school-meal 중계
- 시도교육청 자연어 해석(neis-office-codes.js)
- k-schoollunch-menu 스킬, README·설치/설정/보안·프록시 문서 반영
- docs/adding-a-skill.md 스킬 추가 가이드

Made-with: Cursor
- upstream 생활쓰레기 프록시/스킬·skill-docs 변경 반영
- README 표에 학교 급식 행 복원, security에 KEDU_INFO_KEY·household 라우트 문구 정리
- NEIS 프록시 단위 테스트 블록 복원

Made-with: Cursor
- /v1/household-waste/info에 pageNo·numOfRows 필수, 값은 1·100만 허용(미충족·비정수 문자열은 400)
- validateHouseholdWastePaginationQuery 오동작 수정
- validate-skills.sh에서 .cursor·.vscode 디렉터리 제외
- household-waste·k-skill-proxy 문서, 스킬, 패키지 README에 NEIS·생활쓰레기 curl 예시 정리

Made-with: Cursor
- setup.md·k-skill-setup: 생활쓰레기/급식을 기본 hosted·unset base URL 문구에 포함, 서버 키(DATA_GO_KR·KEDU) 명시
- 표·다음 문서: 생활쓰레기 행·가이드 링크, pageNo/numOfRows 필수
- proxy README·feature doc·household-waste 스킬: 키·페이지네이션·400 계약 보강
- skill-docs.test.js: hosted 생활쓰레기/급식 흐름 회귀 테스트, 예시 secrets에 KEDU_INFO_KEY 금지

Made-with: Cursor
Issue #108 needs a dedicated read-only skill that can browse,
search, and inspect GeekNews posts using the public feed alone.
The implementation adds a fixture-backed Atom helper, skill/docs
surfaces, and install/README wiring, then verifies the helper
against the live GeekNews feed.

Constraint: Must stay RSS-first and avoid new dependencies or unofficial APIs
Constraint: Skill development requires syncing the skill into ~/.claude/skills and ~/.agents/skills during verification
Rejected: Fetch article pages directly for v1 | expands scope beyond the approved RSS-driven workflow
Rejected: Use XML parser modules | current python3 environment has expat issues, so regex + HTML parsing is safer here
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Keep the root helper and geeknews-search/scripts copy behaviorally identical because the installed skill must remain self-contained
Tested: PYTHONPATH=.:scripts python3 -m unittest scripts.test_geeknews_search; node --test scripts/skill-docs.test.js; python3 scripts/geeknews_search.py list --limit 3; python3 scripts/geeknews_search.py search --query Claude --limit 3; python3 scripts/geeknews_search.py detail --id 28439; npm run ci
Not-tested: Non-default feed mirrors or future Atom schema changes beyond the current public GeekNews feed shape
Related: Issue #108
@vkehfdl1
Copy link
Copy Markdown
Contributor Author

vkehfdl1 commented Apr 12, 2026

REQUEST CHANGES

I ran $code-review and did real local verification on the PR branch.

Findings

  1. Medium — new NEIS proxy routes return 500 on upstream fetch failures instead of a controlled proxy error
    packages/k-skill-proxy/src/server.js:806-808, 844-846, 1788-1794, 1888-1896

    proxyNeisSchoolInfoRequest() / proxyNeisSchoolMealRequest() call fetch() directly, and the route handlers await them without a surrounding try/catch. When the upstream request rejects (timeout/DNS/reset), Fastify surfaces a 500 instead of the 502-style proxy response used by the other proxy routes in this file.

    Real repro:

    node - <<'NODE'
    const { buildServer } = require('./packages/k-skill-proxy/src/server');
    (async () => {
      const originalFetch = global.fetch;
      global.fetch = async () => { throw new Error('boom'); };
      const app = buildServer({ env: { KEDU_INFO_KEY: 'k' } });
      try {
        const search = await app.inject({ method: 'GET', url: '/v1/neis/school-search?educationOffice=%EC%84%9C%EC%9A%B8&schoolName=%EB%AF%B8%EB%9E%98%EC%B4%88%EB%93%B1%ED%95%99%EA%B5%90' });
        const meal = await app.inject({ method: 'GET', url: '/v1/neis/school-meal?educationOfficeCode=B10&schoolCode=7010123&mealDate=20260410' });
        console.log({ searchStatus: search.statusCode, mealStatus: meal.statusCode, searchBody: search.body, mealBody: meal.body });
      } finally { global.fetch = originalFetch; await app.close(); }
    })();
    NODE

    Observed result: searchStatus: 500, mealStatus: 500, body {"error":"proxy_error","message":"boom"}.

    Please normalize these failures the same way the other proxy endpoints do and add regression coverage for rejected fetch() calls.

  2. Medium — the selective-install command in docs/install.md is broken
    docs/install.md:80-81

    The newly added --skill k-schoollunch-menu line is missing its trailing \, so the next line becomes a separate shell command. Anyone copy/pasting this block will fail before korean-character-count is installed.

    Real repro:

    bash -lc 'echo npx --yes skills add repo \
      --skill k-schoollunch-menu
      --skill korean-character-count'

    Observed result:

    npx --yes skills add repo --skill k-schoollunch-menu
    bash: line 2: --skill: command not found
    

Real Result

I also ran the PR’s advertised checks locally:

  • PYTHONPATH=.:scripts python3 -m unittest scripts.test_geeknews_search
  • node --test scripts/skill-docs.test.js
  • python3 scripts/geeknews_search.py list --limit 3
  • python3 scripts/geeknews_search.py search --query Claude --limit 3
  • python3 scripts/geeknews_search.py detail --id 28439
  • node --test packages/k-skill-proxy/test/server.test.js
  • npm run ci

So the branch is close, but the two issues above should be fixed before merge.

The GeekNews follow-up review found two merge blockers outside the new helper itself: rejected NEIS upstream fetches escaped as Fastify 500s, and the documented selective install block split into two shell commands. This change adds regression coverage first, catches rejected NEIS fetches at the route boundary with the established proxy_error/502 contract, and locks the multiline install snippet so copy-paste works.

Constraint: Keep NEIS proxy failures aligned with existing public proxy error handling
Constraint: Preserve the published GeekNews verification commands and current PR scope
Rejected: Wrapping fetch inside the NEIS helper functions | route-level handling matches the surrounding Fastify endpoint pattern
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Keep public proxy routes fail-closed on rejected upstream fetches and keep multiline install snippets copy-pasteable
Tested: PYTHONPATH=.:scripts python3 -m unittest scripts.test_geeknews_search; node --test scripts/skill-docs.test.js; node --test packages/k-skill-proxy/test/server.test.js; python3 scripts/geeknews_search.py list --limit 3; python3 scripts/geeknews_search.py search --query Claude --limit 3; python3 scripts/geeknews_search.py detail --id 28439; node NEIS rejected-fetch repro; npm run ci
Not-tested: Live NEIS upstream success responses against the real public API in this follow-up
@vkehfdl1
Copy link
Copy Markdown
Contributor Author

Applied the requested follow-up fixes on feature/#108 and pushed commit 5e34ee0.

  • normalized /v1/neis/school-search and /v1/neis/school-meal rejected upstream fetches to the existing 502 + proxy_error contract
  • added proxy regressions that reproduce the rejected-fetch() failure path for both NEIS routes
  • fixed the missing trailing \ in docs/install.md and added a docs regression to keep k-schoollunch-menu + korean-character-count in the same continued command

Verification run:

  • PYTHONPATH=.:scripts python3 -m unittest scripts.test_geeknews_search
  • node --test scripts/skill-docs.test.js
  • node --test packages/k-skill-proxy/test/server.test.js
  • python3 scripts/geeknews_search.py list --limit 3
  • python3 scripts/geeknews_search.py search --query Claude --limit 3
  • python3 scripts/geeknews_search.py detail --id 28439
  • rejected-fetch repro now returns searchStatus: 502 and mealStatus: 502
  • npm run ci

@vkehfdl1
Copy link
Copy Markdown
Contributor Author

APPROVE

I ran $code-review again on the updated branch and did fresh local verification.

Findings

No new blocking issues found in round 2.

Real Result

  • packages/k-skill-proxy/src/server.js:1788-1803 and 1897-1914 now catch rejected NEIS upstream fetches at the route boundary and normalize them to the existing 502 + proxy_error contract.
  • packages/k-skill-proxy/test/server.test.js:1790-1837 locks both rejected-fetch regressions.
  • docs/install.md:80-81 now keeps k-schoollunch-menu and korean-character-count in the same continued shell command, and scripts/skill-docs.test.js:1898-1901 guards that regression.
  • scripts/skill-docs.test.js:518-540 covers the new GeekNews docs surfaces/workflow, and the GeekNews helper/tests still behave correctly on the live feed.

Verification

  • PYTHONPATH=.:scripts python3 -m unittest scripts.test_geeknews_search
  • node --test scripts/skill-docs.test.js
  • node --test packages/k-skill-proxy/test/server.test.js
  • python3 scripts/geeknews_search.py list --limit 3
  • python3 scripts/geeknews_search.py search --query Claude --limit 3
  • python3 scripts/geeknews_search.py detail --id 28439
  • Manual rejected-fetch repro now returns searchStatus: 502 and mealStatus: 502 with {"error":"proxy_error","message":"boom"}
  • Copy/paste shell repro for the selective install snippet now stays one command: npx --yes skills add repo --skill k-schoollunch-menu --skill korean-character-count
  • npm run ci

RECOMMENDATION: APPROVE

@vkehfdl1
Copy link
Copy Markdown
Contributor Author

Implemented follow-up verification for issue #108 on feature/#108 and confirmed the approved fixes are on the PR branch at 5e34ee0.

  • GeekNews skill/docs/helper flow is present and verified via the fixture-backed Python tests plus live list, search, and detail CLI runs
  • /v1/neis/school-search and /v1/neis/school-meal now normalize rejected upstream fetches to 502 with the existing proxy_error contract, and the rejected-fetch regressions pass
  • docs/install.md keeps k-schoollunch-menu and korean-character-count on the same continued install command, and the docs regression locks that formatting
  • npm run ci passes on the branch, and PR Feature/#108 #109 already targets dev

Verification run:

  • PYTHONPATH=.:scripts python3 -m unittest scripts.test_geeknews_search
  • node --test scripts/skill-docs.test.js
  • node --test packages/k-skill-proxy/test/server.test.js
  • python3 scripts/geeknews_search.py list --limit 3
  • python3 scripts/geeknews_search.py search --query Claude --limit 3
  • python3 scripts/geeknews_search.py detail --id 28439
  • rejected-fetch repro now returns searchStatus: 502 and mealStatus: 502
  • copy/paste install snippet repro now stays one command
  • npm run ci

@vkehfdl1
Copy link
Copy Markdown
Contributor Author

APPROVE

I ran $code-review again on the updated PR branch and did fresh local verification for round 3.

Findings

No new blocking issues found in round 3.

Real Result

  • packages/k-skill-proxy/src/server.js:1788-1803 and 1897-1914 now catch rejected NEIS upstream fetches at the route boundary and normalize them to the existing 502 + proxy_error contract.
  • packages/k-skill-proxy/test/server.test.js:1790-1837 locks the rejected-fetch regressions for both /v1/neis/school-search and /v1/neis/school-meal.
  • scripts/geeknews_search.py:143-244 still parses the public GeekNews feed into stable list/search/detail JSON payloads, and the live helper runs returned valid current feed data on April 13, 2026 (Asia/Seoul).
  • README.md:21-35, docs/features/geeknews-search.md:1-68, and scripts/skill-docs.test.js:518-542 keep the new GeekNews skill discoverable and lock the RSS-first/read-only workflow in docs.
  • docs/install.md:80-81 keeps k-schoollunch-menu and korean-character-count in one continued shell command, and scripts/skill-docs.test.js:1898-1901 guards that regression.

Verification

  • PYTHONPATH=.:scripts python3 -m unittest scripts.test_geeknews_search
  • node --test scripts/skill-docs.test.js
  • node --test packages/k-skill-proxy/test/server.test.js
  • python3 scripts/geeknews_search.py list --limit 3 ✅ returned count: 3 with current GeekNews items
  • python3 scripts/geeknews_search.py search --query Claude --limit 3 ✅ returned count: 3 on the live feed
  • python3 scripts/geeknews_search.py detail --id 28439 ✅ returned the expected live entry with author_name: "workdriver"
  • Manual rejected-fetch repro ✅ returned searchStatus: 502 and mealStatus: 502 with {"error":"proxy_error","message":"boom"}
  • Copy/paste install snippet repro ✅ stayed one command: npx --yes skills add repo --skill k-schoollunch-menu --skill korean-character-count
  • npm run ci

RECOMMENDATION: APPROVE

@vkehfdl1
Copy link
Copy Markdown
Contributor Author

APPROVE

I ran $code-review again on the updated PR branch and did fresh local verification for round 3.

Findings

No new blocking issues found in round 3.

Real Result

  • packages/k-skill-proxy/src/server.js:1788-1803 and 1897-1914 catch rejected NEIS upstream fetches at the route boundary and normalize them to the existing 502 + proxy_error contract.
  • packages/k-skill-proxy/test/server.test.js:1790-1837 locks the rejected-fetch regressions for both /v1/neis/school-search and /v1/neis/school-meal.
  • scripts/geeknews_search.py:143-244 still parses the public GeekNews feed into stable list/search/detail JSON payloads, and the live helper runs returned valid current feed data on April 13, 2026 (Asia/Seoul).
  • README.md:21-35, docs/features/geeknews-search.md:1-68, and scripts/skill-docs.test.js:518-542 keep the new GeekNews skill discoverable and lock the RSS-first/read-only workflow in docs.
  • docs/install.md:80-81 keeps k-schoollunch-menu and korean-character-count in one continued shell command, and scripts/skill-docs.test.js:1898-1901 guards that regression.

Verification

  • PYTHONPATH=.:scripts python3 -m unittest scripts.test_geeknews_search
  • node --test scripts/skill-docs.test.js
  • node --test packages/k-skill-proxy/test/server.test.js
  • python3 scripts/geeknews_search.py list --limit 3 ✅ returned count: 3 with current GeekNews items
  • python3 scripts/geeknews_search.py search --query Claude --limit 3 ✅ returned count: 3 on the live feed
  • python3 scripts/geeknews_search.py detail --id 28439 ✅ returned the expected live entry with author_name: "workdriver"
  • Manual rejected-fetch repro ✅ returned searchStatus: 502 and mealStatus: 502 with {"error":"proxy_error","message":"boom"}
  • Copy/paste install snippet repro ✅ stayed one command: npx --yes skills add repo --skill k-schoollunch-menu --skill korean-character-count
  • npm run ci

RECOMMENDATION: APPROVE

@vkehfdl1
Copy link
Copy Markdown
Contributor Author

Re-verified the approved issue #108 follow-up on feature/#108 and confirmed no additional code changes were needed beyond the already-pushed fixes at 5e34ee0.

  • GeekNews skill/docs/helper flow is present and working: fixture-backed unit tests pass, docs regressions pass, and live list, search, and detail runs returned valid current feed data.
  • /v1/neis/school-search and /v1/neis/school-meal normalize rejected upstream fetches to the existing 502 + proxy_error contract, and both automated regressions plus the manual rejected-fetch repro pass.
  • docs/install.md keeps the selective install snippet on one continued shell command, and the copy/paste repro plus docs regression confirm it stays intact.
  • npm run ci passes on the branch, PR Feature/#108 #109 remains open, and it still targets dev from feature/#108.

Verification re-run:

  • PYTHONPATH=.:scripts python3 -m unittest scripts.test_geeknews_search
  • node --test scripts/skill-docs.test.js
  • node --test packages/k-skill-proxy/test/server.test.js
  • python3 scripts/geeknews_search.py list --limit 3
  • python3 scripts/geeknews_search.py search --query Claude --limit 3
  • python3 scripts/geeknews_search.py detail --id 28439
  • manual rejected-fetch repro for both NEIS routes (502 / proxy_error)
  • copy/paste install snippet repro
  • npm run ci

@vkehfdl1
Copy link
Copy Markdown
Contributor Author

APPROVE

Short reason: I re-verified the current PR head (5e34ee0) on April 13, 2026 (Asia/Seoul) and found no blocking issues. The previously requested NEIS proxy error-handling fix and the broken install snippet are both fixed, and the new GeekNews skill/helper/docs behave as advertised.

Real Result

  • packages/k-skill-proxy/src/server.js:1788-1803 and 1897-1914 now catch rejected NEIS upstream fetches at the route boundary and normalize them to the existing 502 + proxy_error contract.
  • packages/k-skill-proxy/test/server.test.js:1790-1837 locks both rejected-fetch regressions.
  • README.md:28,97, docs/features/geeknews-search.md:17-40, and scripts/skill-docs.test.js:518-542 keep the GeekNews skill discoverable and enforce the RSS-first/read-only list-search-detail workflow.
  • docs/install.md:80-81 plus scripts/skill-docs.test.js:1898-1901 keep k-schoollunch-menu and korean-character-count in one continued selective-install command.

Verification

  • PYTHONPATH=.:scripts python3 -m unittest scripts.test_geeknews_search
  • node --test scripts/skill-docs.test.js
  • node --test packages/k-skill-proxy/test/server.test.js
  • python3 scripts/geeknews_search.py list --limit 3 ✅ returned count: 3; top item was https://news.hada.io/topic?id=28443
  • python3 scripts/geeknews_search.py search --query Claude --limit 3 ✅ returned count: 3; included https://news.hada.io/topic?id=28439
  • python3 scripts/geeknews_search.py detail --id 28439 ✅ returned author_name: "workdriver"
  • Manual rejected-fetch repro ✅ returned searchStatus: 502 and mealStatus: 502 with {"error":"proxy_error","message":"boom"} for both routes
  • Copy/paste install-snippet repro ✅ stayed one command: npx --yes skills add repo --skill k-schoollunch-menu --skill korean-character-count
  • npm run ci

RECOMMENDATION: APPROVE

@vkehfdl1 vkehfdl1 merged commit 77001d7 into dev Apr 12, 2026
1 check passed
@vkehfdl1 vkehfdl1 deleted the feature/#108 branch April 12, 2026 15:54
@vkehfdl1
Copy link
Copy Markdown
Contributor Author

APPROVE

Short reason: I re-verified the updated PR branch at 5e34ee0 on April 13, 2026 (Asia/Seoul) and found no new blocking issues. The previously requested NEIS proxy error-handling fix and the selective-install docs fix are both present, covered by regression tests, and behaving correctly in local repros.

Real Result

  • packages/k-skill-proxy/src/server.js:1738-1848 and 1851-1914 now catch rejected NEIS upstream fetches at the route boundary and normalize them to the existing 502 + proxy_error contract.
  • packages/k-skill-proxy/test/server.test.js:1790-1837 locks the rejected-fetch regressions for both /v1/neis/school-search and /v1/neis/school-meal.
  • docs/install.md:80-81 now keeps k-schoollunch-menu and korean-character-count in the same continued shell command, and scripts/skill-docs.test.js:1898-1901 guards that regression.
  • scripts/geeknews_search.py:143-246 still parses the public GeekNews feed into stable list/search/detail payloads; README.md:21-35, docs/features/geeknews-search.md:1-68, and scripts/skill-docs.test.js:518-542 keep the feature discoverable and lock the RSS-first/read-only workflow.

Verification

  • PYTHONPATH=.:scripts python3 -m unittest scripts.test_geeknews_searchRan 7 tests ... OK
  • node --test scripts/skill-docs.test.js92 tests passed
  • node --test packages/k-skill-proxy/test/server.test.js58 tests passed
  • python3 scripts/geeknews_search.py list --limit 3 ✅ returned count: 3; live feed top item was topic?id=28443
  • python3 scripts/geeknews_search.py search --query Claude --limit 3 ✅ returned count: 3; included topic?id=28439
  • python3 scripts/geeknews_search.py detail --id 28439 ✅ returned the expected live entry with author_name: "workdriver"
  • Manual rejected-fetch repro ✅ returned searchStatus: 502 and mealStatus: 502 with {"error":"proxy_error","message":"boom"}
  • Copy/paste install-snippet repro ✅ stayed one command: npx --yes skills add repo --skill k-schoollunch-menu --skill korean-character-count
  • cmp -s scripts/geeknews_search.py geeknews-search/scripts/geeknews_search.py ✅ helper copies are identical
  • npm run ci ✅ passed (lint, typecheck, test, pack:dry-run)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants