Feature/#0 by vkehfdl1 · Pull Request #103 · NomaDamas/k-skill

vkehfdl1 · 2026-04-10T04:49:43Z

Summary

carries over the k-schoollunch-menu / NEIS proxy feature from the existing forked PR into feature/#0
fixes the review blocker by validating repeated /v1/household-waste/info scalar query params before any upstream call
keeps malformed household-waste pagination and duplicated cond[SGG_NM::LIKE] requests on the documented 400 path
removes KEDU_INFO_KEY from the user-facing secrets example and updates proxy/operator docs for DATA_GO_KR_API_KEY + household-waste routing
makes scripts/validate-skills.sh ignore hidden metadata directories so the requested validation command stays green in this repo checkout

Why

public proxy routes should reject malformed pagination and repeated scalar filters instead of forwarding ambiguous values upstream
KEDU_INFO_KEY belongs on the proxy server, not in the default client secrets example
proxy deployment docs need to advertise the shipped household-waste route and required server env
this branch provides an upstream-owned PR path for the existing PR Feature/k-schoollunch-menu : 급식정보 조회기능 추가 #102 follow-up

Test plan

node --test packages/k-skill-proxy/test/server.test.js
npm run lint
npm run typecheck
npm test
local proxy smoke: duplicated cond[SGG_NM::LIKE] returns 400 before upstream config is required

Other

live household-waste success-path smoke was not run because no real local DATA_GO_KR_API_KEY was used in this environment
this PR follows up on the review feedback captured on PR Feature/k-schoollunch-menu : 급식정보 조회기능 추가 #102 and PR fix: #102, #103 수정사항 반영 #105

생활쓰레기 조회 스킬 문서와 기능 가이드를 추가하고, 프록시 라우트를 구현해 조회 흐름을 완성했다. 설치/설정 문서도 스킬 사용 흐름에 맞게 정리했다. 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

The existing school lunch feature branch was mergeable in shape, but review found that the public household-waste route still forwarded unchecked pagination inputs, user-facing secrets docs suggested storing a server-only KEDU key locally, and the proxy/operator docs omitted the new household-waste env requirements. This follow-up validates the public query surface before fetch, aligns docs with the proxy-only secret policy, and keeps the requested validation workflow green by ignoring hidden metadata folders in the skill-layout checker. Constraint: Public proxy routes must stay narrow and keep upstream keys server-only Rejected: Forward arbitrary pageNo values | violates the documented public-route contract Rejected: Leave validate-skills unchanged | hidden metadata directories break the requested verification flow Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep household-waste parameter support and docs/tests in lockstep when widening this route Tested: npm run lint Tested: npm run typecheck Tested: node --test packages/k-skill-proxy/test/server.test.js Tested: node --test scripts/skill-docs.test.js Tested: bash scripts/validate-skills.sh Tested: local proxy smoke (/health, household-waste invalid 400, household-waste valid 200) Not-tested: Live NEIS curl smoke test (KEDU_INFO_KEY unavailable locally)

vkehfdl1 · 2026-04-10T04:54:40Z

Used $code-review locally. I did not find a blocking issue in this round.

Real Result

npm run lint ✅
npm run typecheck ✅
node --test packages/k-skill-proxy/test/server.test.js ✅ (40/40 passing)
bash scripts/validate-skills.sh ✅ (skill layout looks valid)
node --test scripts/skill-docs.test.js ✅ (66/66 passing)
proxy smoke via local buildServer(...).inject(...) ✅
- /health → 200, body included "ok":true
- invalid /v1/household-waste/info?...pageNo=abc → 400, bad_request, pageNo must be an integer >= 1.
- valid /v1/household-waste/info?...pageNo=1&numOfRows=20 → 200, echoed sgg_nm=강남구, page_no=1

Review note

The household-waste pagination guard looks correct and is verified to reject malformed input before any upstream fetch.
The NEIS proxy paths, cache behavior, and KEDU_INFO_KEY server-only documentation all look internally consistent with the added tests.
Non-blocking follow-up: docs/setup.md now lists 학교 급식 식단 조회, but it still does not add a matching 생활쓰레기 배출정보 조회 row in the setup matrix. I'd patch that in a later docs sweep for consistency.

Issue #0's core proxy and NEIS changes were already present on feature/#0, but the setup guide still omitted the hosted/self-host household-waste entry that users need when checking which features require local secrets. Add a regression test first, then document the household-waste row and guide link so the setup matrix stays aligned with the shipped proxy routes. Constraint: Keep the follow-up diff narrow and avoid touching already-approved proxy behavior Rejected: Leave the setup matrix as-is | it would keep user-facing docs inconsistent with the shipped route list Confidence: high Scope-risk: narrow Reversibility: clean Directive: When proxy-backed skills move between hosted and self-host flows, update docs/setup.md alongside the feature docs Tested: node --test scripts/skill-docs.test.js Tested: npm run lint Tested: npm run typecheck Tested: node --test packages/k-skill-proxy/test/server.test.js Tested: bash scripts/validate-skills.sh Not-tested: Live hosted proxy deployment docs rendering on GitHub

vkehfdl1 · 2026-04-10T05:06:01Z

Implemented the approved follow-up on feature/#0 and pushed 42a6ee1.

added a regression test in scripts/skill-docs.test.js to keep the setup guide aligned with the shipped household-waste proxy flow
updated docs/setup.md so the setup matrix, intro text, and next-docs links now include 생활쓰레기 배출정보 조회 alongside the existing hosted/self-host proxy guidance
re-ran the requested verification: npm run lint, npm run typecheck, node --test packages/k-skill-proxy/test/server.test.js, bash scripts/validate-skills.sh, node --test scripts/skill-docs.test.js
re-smoked the proxy locally via buildServer(...).inject(...): /health -> 200, invalid household-waste pagination -> 400 (pageNo must be an integer >= 1.), valid household-waste request -> 200 (sgg_nm=강남구, page_no=1)

The setup guide already described proxy-hosted household-waste and NEIS flows elsewhere, but the opening summary still omitted school lunch from the no-user-key hosted-proxy list. This adds a regression test first, then aligns the intro sentence and test label so the doc stays consistent with the shipped proxy-backed feature set. Constraint: Keep the follow-up scoped to existing Issue #0/PR #103 documentation surfaces Rejected: Broader setup guide rewrite | unnecessary for the approved follow-up Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep hosted-proxy/no-user-key summaries aligned with the per-feature setup matrix when adding new proxy-backed skills Tested: Targeted red/green skill-docs regression; npm run lint; npm run typecheck; node --test packages/k-skill-proxy/test/server.test.js; bash scripts/validate-skills.sh; node --test scripts/skill-docs.test.js; buildServer runtime smoke for /health, household-waste validation, and NEIS missing-key behavior Not-tested: Live NEIS upstream call with a real KEDU_INFO_KEY Related: PR #102 Related: PR #103

vkehfdl1 · 2026-04-10T05:20:39Z

Used $code-review locally. I did not find a blocking issue in this round.

Real Result

npm run lint ✅
npm run typecheck ✅
node --test packages/k-skill-proxy/test/server.test.js ✅ (40/40 passing)
bash scripts/validate-skills.sh ✅ (skill layout looks valid)
node --test scripts/skill-docs.test.js ✅ (67/67 passing)
proxy smoke via local buildServer(...).inject(...) ✅
- /health → 200, body included "ok":true and "neisSchoolMealConfigured":true
- invalid /v1/household-waste/info?...pageNo=abc&numOfRows=20 → 400, bad_request, pageNo must be an integer >= 1.
- valid /v1/household-waste/info?...pageNo=1&numOfRows=20 → 200, query.sgg_nm=강남구, query.page_no=1, query.num_of_rows=20
- upstream smoke stub captured exactly one fetch to https://apis.data.go.kr/1741000/household_waste_info/info?...serviceKey=test-key&pageNo=1&numOfRows=20&returnType=json&cond[SGG_NM::LIKE]=강남구

Review note

The round-2 docs follow-up is in place: docs/setup.md now advertises hosted-proxy coverage for both 생활쓰레기 배출정보 and 학교 급식 식단, and the added regression test keeps the hosted-proxy summary plus the household-waste setup row/link under test.
I did not find a merge-blocking issue in the current PR state.
Non-blocking follow-up: docs/setup.md:36 still lists only 미세먼지/한강 수위/주유소 가격 in the “leave KSKILL_PROXY_BASE_URL unset” sentence, even though the same guide now treats household-waste and school-lunch as hosted-proxy-backed too. Also, scripts/skill-docs.test.js:235-246 says it covers both household-waste and school-lunch, but it only asserts the household-waste row/link today.

The approved PR #103 follow-up extends the setup-doc regression coverage so it checks the hosted KSKILL_PROXY_BASE_URL guidance and both household-waste + school-lunch setup entries, then updates the guide text to match the shipped hosted-proxy behavior. Constraint: Must stay within the approved PR #103 follow-up scope Rejected: Broader setup-doc wording sweep across unrelated features | unnecessary for this approved follow-up Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep docs/setup.md and scripts/skill-docs.test.js aligned whenever hosted proxy coverage changes Tested: node --test scripts/skill-docs.test.js Tested: npm run lint Tested: npm run typecheck Tested: node --test packages/k-skill-proxy/test/server.test.js Tested: bash scripts/validate-skills.sh Tested: local buildServer(...).inject(...) smoke for /health and household-waste pagination Not-tested: Live NEIS curl smoke with a real KEDU_INFO_KEY

vkehfdl1 · 2026-04-10T05:33:23Z

Implemented the approved docs follow-up on feature/#0 and pushed d18c8a5.

extended scripts/skill-docs.test.js first so the regression now checks the hosted KSKILL_PROXY_BASE_URL guidance plus the school-lunch setup row/link alongside household-waste coverage
updated docs/setup.md so the self-host/hosted-proxy paragraph now includes both 생활쓰레기 배출정보 and 학교 급식 식단 when KSKILL_PROXY_BASE_URL is left unset
re-ran verification: npm run lint, npm run typecheck, node --test packages/k-skill-proxy/test/server.test.js, bash scripts/validate-skills.sh, node --test scripts/skill-docs.test.js
re-smoked the proxy locally via buildServer(...).inject(...): /health -> 200 with neisSchoolMealConfigured: true, invalid household-waste pagination -> 400 without any upstream fetch, valid household-waste request -> 200 with sgg_nm=강남구, page_no=1, num_of_rows=20

The feature/#0 follow-up already had the proxy/runtime fixes in place, but the branch still needed the final regression locks and doc wording that keep the review fixes durable. This commit adds lower-bound household-waste pagination tests and aligns install/security docs with the proxy-only KEDU_INFO_KEY policy so the school lunch feature stays on the hosted-proxy / no-user-key path. Constraint: Follow-up had to stay on feature/#0 so PR #103 updates in place Constraint: User-facing secrets guidance must not imply KEDU_INFO_KEY belongs in the default client env file Rejected: Broaden the pass into more proxy/doc rewrites | unnecessary beyond the approved Issue #0 follow-up Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep school lunch docs and regression text aligned with the hosted proxy policy whenever setup/security wording changes Tested: npm run lint; npm run typecheck; node --test packages/k-skill-proxy/test/server.test.js; bash scripts/validate-skills.sh; node --test scripts/skill-docs.test.js; local proxy smoke for /health, invalid household-waste 400, valid household-waste 200, NEIS without KEDU_INFO_KEY 503 Not-tested: Live NEIS curl against a valid KEDU_INFO_KEY-enabled proxy Related: PR #102 Related: PR #103

vkehfdl1 · 2026-04-10T05:50:53Z

Used $code-review locally. I did not find a blocking issue in this round.

Real Result

npm run lint ✅
npm run typecheck ✅
node --test packages/k-skill-proxy/test/server.test.js ✅ (42/42 passing)
bash scripts/validate-skills.sh ✅ (skill layout looks valid)
node --test scripts/skill-docs.test.js ✅ (68/68 passing)
local proxy smoke via buildServer(...).inject(...) ✅
- /health → 200, body included "ok":true and "neisSchoolMealConfigured":true
- invalid /v1/household-waste/info?...pageNo=abc&numOfRows=20 → 400, bad_request, pageNo must be an integer >= 1., and the stub saw no upstream fetch before rejection
- valid /v1/household-waste/info?...pageNo=1&numOfRows=20 → 200, query.sgg_nm=강남구, query.page_no=1, query.num_of_rows=20
- upstream smoke stub captured exactly one fetch to https://apis.data.go.kr/1741000/household_waste_info/info?serviceKey=test-key&pageNo=1&numOfRows=20&returnType=json&cond[SGG_NM::LIKE]=강남구

Review note

The round-2 setup/docs follow-up is in place: docs/setup.md and scripts/skill-docs.test.js now lock the hosted-proxy / no-user-key guidance for both 생활쓰레기 배출정보 and 학교 급식 식단.
The household-waste pagination guard still looks correct after the last follow-up. I re-verified both lower-bound validation and the “reject before upstream call” behavior in tests plus the local smoke.
Non-blocking follow-up: k-skill-setup/SKILL.md:79-85 and k-skill-setup/SKILL.md:96-105 still omit 생활쓰레기 배출정보 / 학교 급식 식단 from the hosted-proxy summary and missing-secret checklist, so that setup skill can drift from docs/setup.md again unless that surface is aligned or added to the docs regression coverage.

The setup skill had drifted behind docs/setup.md after the household-waste and school-lunch proxy follow-ups. This updates the skill guidance and locks it with a regression so future doc edits keep the hosted-proxy and server-only-key story consistent across the user-facing setup surfaces. Constraint: Follow-up scope is limited to the approved docs/setup consistency gap from PR #103 review Rejected: Broad setup-doc sweep | unnecessary beyond the approved drift fix Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep k-skill-setup/SKILL.md aligned with docs/setup.md whenever hosted proxy coverage changes Tested: npm run lint; npm run typecheck; node --test packages/k-skill-proxy/test/server.test.js; bash scripts/validate-skills.sh; node --test scripts/skill-docs.test.js; local buildServer smoke for /health and household-waste pagination Not-tested: Live upstream NEIS/Data.go.kr network calls with real server credentials Related: PR #103

vkehfdl1 · 2026-04-10T05:59:34Z

Implemented the approved setup-skill follow-up on feature/#0 and pushed 08408ca.

added a TDD regression in scripts/skill-docs.test.js that keeps k-skill-setup/SKILL.md aligned with the hosted-proxy guidance for 생활쓰레기 배출정보 and 학교 급식 식단
updated k-skill-setup/SKILL.md so the hosted KSKILL_PROXY_BASE_URL summary, household-waste server-key note, and school-lunch server-key note now match docs/setup.md
re-ran verification: npm run lint, npm run typecheck, node --test packages/k-skill-proxy/test/server.test.js, bash scripts/validate-skills.sh, node --test scripts/skill-docs.test.js
re-smoked locally via buildServer(...): /health -> 200 with neisSchoolMealConfigured: true; invalid household-waste pagination -> 400 with pageNo must be an integer >= 1. and no upstream fetch; valid household-waste request -> 200 with sgg_nm=강남구, page_no=1, num_of_rows=20

vkehfdl1 · 2026-04-10T06:06:49Z

REJECT

Short reason: the current branch is not mergeable into dev. I did not find a new code-level blocker in the checked-out diff, but the PR currently has unresolved merge conflicts in core proxy/docs files.

Real Result

npm run lint ✅
npm run typecheck ✅
node --test packages/k-skill-proxy/test/server.test.js ✅ (42/42 passing)
bash scripts/validate-skills.sh ✅ (skill layout looks valid)
node --test scripts/skill-docs.test.js ✅ (69/69 passing)
local proxy smoke via buildServer(...).inject(...) ✅
- /health → 200, with ok: true and neisSchoolMealConfigured: true
- invalid /v1/household-waste/info?...pageNo=abc&numOfRows=20 → 400, bad_request, pageNo must be an integer >= 1., and 0 upstream fetches before rejection
- valid /v1/household-waste/info?...pageNo=1&numOfRows=20 → 200, query.sgg_nm=강남구, query.page_no=1, query.num_of_rows=20
- stubbed upstream was called exactly once with https://apis.data.go.kr/1741000/household_waste_info/info?serviceKey=test-key&pageNo=1&numOfRows=20&returnType=json&cond%5BSGG_NM%3A%3ALIKE%5D=%EA%B0%95%EB%82%A8%EA%B5%AC

Blocking evidence

gh pr view 103 --repo NomaDamas/k-skill --json mergeStateStatus returned "mergeStateStatus":"DIRTY".
A local dry-run merge of origin/feature/#0 into origin/dev failed with content conflicts in:
- README.md
- docs/features/household-waste-info.md
- docs/features/k-skill-proxy.md
- docs/install.md
- docs/security-and-secrets.md
- docs/setup.md
- k-skill-setup/SKILL.md
- packages/k-skill-proxy/README.md
- packages/k-skill-proxy/src/server.js
- packages/k-skill-proxy/test/server.test.js

Once this branch is rebased/merged cleanly on top of current dev, I’d expect a quick re-review.

Fastify parses repeated query parameters as arrays, so the household-waste normalizer now rejects repeated scalar fields before any upstream request. This adds a regression test for duplicated cond[SGG_NM::LIKE] values and keeps invalid requests on the documented 400 bad_request path. Constraint: Household-waste validation must fail before any upstream fetch Rejected: Join repeated query values into one filter | ambiguous request semantics Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep repeated scalar query params on the 400 path unless the public contract is expanded intentionally Tested: node --test packages/k-skill-proxy/test/server.test.js; npm test; npm run lint; npm run typecheck; local proxy smoke on duplicated cond[SGG_NM::LIKE] Not-tested: Live household-waste upstream success path with a real DATA_GO_KR_API_KEY

Review follow-up found that mixed alias pagination params could still bypass the narrowed public route contract even after duplicate scalar handling was added. This tightens the query normalizer so pageNo/page_no and numOfRows/num_of_rows must appear exactly once and match the fixed allowed values, expands regression coverage for those alias combinations, and aligns the public docs/examples with the exact pageNo=1 + numOfRows=100 contract. The household-waste test setup was also de-duplicated within the scoped suite to keep the follow-up readable without widening behavior. Constraint: The public household-waste proxy route must stay narrow and reject ambiguous scalar query shapes before any upstream fetch Rejected: Keep alias resolution as first-non-null | mixed pageNo/page_no and numOfRows/num_of_rows would still bypass the documented 400 path Rejected: Broaden the route to support alternate page sizes | outside the approved issue scope and contrary to the fixed-value contract Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep household-waste docs and regression tests aligned whenever the allowed query surface changes, including alias handling Tested: node --test packages/k-skill-proxy/test/server.test.js Tested: node --test scripts/skill-docs.test.js Tested: npm run lint Tested: npm run typecheck Tested: npm run test Tested: local proxy smoke on port 4121 for mixed pageNo/page_no 400, mixed numOfRows/num_of_rows 400, and returnType override reaching upstream with fixed pagination Not-tested: Live successful household-waste upstream response with a real DATA_GO_KR_API_KEY

hyeongr and others added 8 commits April 7, 2026 19:35

vkehfdl1 mentioned this pull request Apr 10, 2026

Feature/k-schoollunch-menu : 급식정보 조회기능 추가 #102

Closed

vkehfdl1 mentioned this pull request Apr 10, 2026

fix: #102, #103 수정사항 반영 #105

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/#0#103

Feature/#0#103
vkehfdl1 wants to merge 15 commits intodevfrom
feature/#0

vkehfdl1 commented Apr 10, 2026 •

edited

Loading

Uh oh!

vkehfdl1 commented Apr 10, 2026

Uh oh!

vkehfdl1 commented Apr 10, 2026

Uh oh!

vkehfdl1 commented Apr 10, 2026

Uh oh!

vkehfdl1 commented Apr 10, 2026

Uh oh!

vkehfdl1 commented Apr 10, 2026

Uh oh!

vkehfdl1 commented Apr 10, 2026

Uh oh!

vkehfdl1 commented Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

vkehfdl1 commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Test plan

Other

Uh oh!

vkehfdl1 commented Apr 10, 2026

Uh oh!

vkehfdl1 commented Apr 10, 2026

Uh oh!

vkehfdl1 commented Apr 10, 2026

Uh oh!

vkehfdl1 commented Apr 10, 2026

Uh oh!

vkehfdl1 commented Apr 10, 2026

Uh oh!

vkehfdl1 commented Apr 10, 2026

Uh oh!

vkehfdl1 commented Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vkehfdl1 commented Apr 10, 2026 •

edited

Loading