Skip to content

Feature/#207 - #209

Merged
vkehfdl1 merged 4 commits into
devfrom
feature/#207
May 6, 2026
Merged

Feature/#207#209
vkehfdl1 merged 4 commits into
devfrom
feature/#207

Conversation

@vkehfdl1

@vkehfdl1 vkehfdl1 commented May 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

  • Handle Daiso Mall pickup-stock Unauthorized responses as explicit blocked pickup-stock results: pickupStock.status = "unavailable", retrievalStatus = "blocked", inventoryStatus = "unknown", and reason = "unauthorized".
  • Keep online-stock lookup available only as a separate onlineStock.referenceOnly = true fallback/reference signal, not store inventory.
  • Clarify the pickup-stock public contract so pickupStock.status is a retrieval/result category and actual inventory availability is read from inStock / inventoryStatus.
  • Update package docs, skill guidance, source notes, and the patch Changeset.

Validation

  • TDD: new Unauthorized, retrieval-state, reference-only online-stock, and zero-stock inventoryStatus assertions failed before implementation and pass after the fix.
  • npm test --workspace daiso-product-search — PASS, 14/14 tests
  • npm run lint --workspace daiso-product-search — PASS
  • Node smoke with mocked 403 Unauthorized verifies blocked pickup stock plus separate online stock quantity/reference signal.
  • Architect verification: CLEAR / APPROVED.
  • Post-deslop: git diff --check && npm run ci — PASS.

Assumption

  • The implementation intentionally does not bypass Daiso Mall session/security policy; it reports blocked pickup-stock lookups conservatively.

@vkehfdl1
Handle Daiso pickup-stock authorization blocks
402c99c 
Treat Daiso Mall pickup-stock 401/403 Unauthorized as an explicit unavailable store-stock result so product lookup remains usable without attempting to bypass session checks. Preserve online-stock lookup as a clearly separate fallback signal and document the limitation.

Constraint: Issue #207 reports Daiso Mall now blocks /api/pd/pdh/selStrPkupStck for normal API calls.

Rejected: Browser/session bypass | would work around Daiso security policy and require credentialed/non-public state.

Confidence: high

Scope-risk: narrow

Directive: Keep online stock distinct from store pickup stock; never present online quantity as exact store inventory.

Tested: Added failing TDD tests first; npm test --workspace daiso-product-search; npm run lint --workspace daiso-product-search; node smoke with mocked 403 Unauthorized; git diff --check && npm run ci after deslop.

Not-tested: Live Daiso Mall request after policy change, because the fix intentionally avoids bypassing session-gated Unauthorized responses.
@vkehfdl1

vkehfdl1 commented May 5, 2026

Copy link
Copy Markdown
Member Author

CODE REVIEW REPORT

Files reviewed: 8
Total issues: 0 blocking, 2 LOW follow-up suggestions
Architectural status: WATCH
Recommendation: COMMENT / approve as-is with follow-up watchlist

Summary

I reviewed PR #209 using the $code-review workflow, including separate code-quality and architecture/devil's-advocate lanes, plus local verification. I did not find any CRITICAL, HIGH, or MEDIUM issues that should block merge.

The implementation keeps the important boundary intact: Daiso Mall pickup-stock Unauthorized responses are normalized as pickupStock.status = "unavailable" / reason = "unauthorized", while online stock remains a separate reference signal and is not presented as store inventory.

Real Result

Actual verification run locally on branch feature/#207:

  • npm test --workspace daiso-product-search — PASS, 13/13 tests
  • npm run lint --workspace daiso-product-search — PASS (node --check for source and tests)
  • git diff --check origin/dev...HEAD — PASS
  • npm run ci — PASS, including lint, typecheck, tests, skill validation, and pack dry-run
  • Manual Node smoke with mocked 403 Unauthorized from /api/pd/pdh/selStrPkupStck — PASS:
    • pickupStock.status returned "unavailable"
    • pickupStock.reason returned "unauthorized"
    • pickupStock.quantity / inStock returned null
    • onlineStock.quantity still returned 13047 as a separate reference value

Evidence

  • packages/daiso-product-search/src/index.js:116-139 scopes Unauthorized conversion to getStorePickupStock() only; generic non-OK requests still throw DaisoRequestError.
  • packages/daiso-product-search/src/parse.js:334-345 normalizes Daiso { success: false, message: "Unauthorized" } into the unavailable pickup-stock result shape.
  • packages/daiso-product-search/src/index.js:142-153 keeps online stock lookup separate from store pickup stock; it does not use strCd and is returned as onlineStock separately.
  • packages/daiso-product-search/test/index.test.js:285-340 covers both direct 401 conversion and end-to-end 403 pickup Unauthorized with preserved online stock.
  • Docs and skill guidance explicitly warn not to treat online stock as exact store inventory and not to bypass Daiso session/security policy.
  • .changeset/issue-207-daiso-unauthorized.md correctly adds a patch Changeset; no test asserts Changeset file presence.

LOW / WATCH follow-ups

  1. packages/daiso-product-search/src/index.js:183-206 returns pickupStock and onlineStock as siblings. The docs are clear, but a future public-contract hardening could make reference-only semantics even harder to misuse, e.g. onlineStock.referenceOnly: true or an aggregate availabilityStatus.
  2. Unauthorized classification is currently split between packages/daiso-product-search/src/index.js:69-75 and packages/daiso-product-search/src/parse.js:330-345. This is fine for the current patch, but if more Daiso failure modes appear, consider centralizing the stock-result/reason contract.

Architectural status

WATCH, not BLOCK. The PR solves the reported Unauthorized behavior conservatively and avoids conflating online stock with store pickup inventory. The main follow-up is to formalize the pickup/online stock result union before additional unavailable/error reasons accumulate.

RECOMMENDATION: COMMENT

No merge-blocking issues found.

@vkehfdl1
Clarify online stock as reference-only
4cf7967 
Make the Daiso online-stock fallback self-describing so pickup inventory cannot be confused with online availability after Unauthorized pickup-stock blocks.\n\nConstraint: Issue #207 requires preserving online stock as a separate fallback/reference signal, not store inventory.\nRejected: Adding an aggregate availabilityStatus | unnecessary contract expansion for this patch.\nConfidence: high\nScope-risk: narrow\nDirective: Do not treat onlineStock.quantity as store pickup or shelf inventory.\nTested: npm test --workspace daiso-product-search; npm run lint --workspace daiso-product-search; mocked 403 Unauthorized Node smoke; git diff --check; npm run ci\nNot-tested: Live Daiso Mall session-gated pickup-stock access.
@vkehfdl1

vkehfdl1 commented May 5, 2026

Copy link
Copy Markdown
Member Author

Implementation update

  • Added the approved follow-up hardening so online-stock results now include referenceOnly: true, making the fallback/reference semantics explicit and harder to confuse with store pickup inventory.
  • Added TDD assertions for direct online stock and the mocked 403 Unauthorized pickup-stock lookup path; those assertions failed before the implementation and pass after it.
  • Updated package README, skill guidance, feature docs, and the patch Changeset wording to document the reference-only online stock contract.
  • Preserved the conservative boundary: pickup-stock Unauthorized still returns pickupStock.status = "unavailable" / reason = "unauthorized"; no Daiso session/security bypass was added.

Verification

  • npm test --workspace daiso-product-search — PASS, 13/13 tests
  • npm run lint --workspace daiso-product-search — PASS
  • Mocked Node smoke for 403 Unauthorized pickup-stock — PASS (pickupStock unavailable/unauthorized, onlineStock.quantity = 13047, onlineStock.referenceOnly = true)
  • Architect verification — APPROVED, no blockers
  • Deslop scan on changed files — completed, no masking fallback slop found
  • git diff --check && npm run ci — PASS

@vkehfdl1

vkehfdl1 commented May 5, 2026

Copy link
Copy Markdown
Member Author

CODE REVIEW REPORT

Files reviewed: 8
Total issues: 0 blocking, 2 LOW/WATCH follow-up suggestions
Architectural status: WATCH
Recommendation: COMMENT / approve as-is with follow-up watchlist

Summary

I reviewed the round-2 update with the $code-review workflow, including separate code/spec/security and architecture/devil's-advocate lanes plus fresh local verification. I did not find any CRITICAL, HIGH, or MEDIUM issues that should block merge.

The round-2 hardening addresses the prior watch item by making online stock explicitly referenceOnly: true, while preserving the conservative boundary: Daiso Mall pickup-stock Unauthorized responses become pickupStock.status = "unavailable" / reason = "unauthorized", and online stock is kept as a separate reference signal rather than store inventory.

Real Result

Actual verification run locally on origin/feature/#207 at commit 4cf7967:

  • npm test --workspace daiso-product-search — PASS, 13/13 tests
  • npm run lint --workspace daiso-product-search — PASS (node --check for source and tests)
  • git diff --check origin/dev...HEAD — PASS
  • npm run ci — PASS, including lint, typecheck, tests, skill validation, and npm pack dry-run
  • Manual Node smoke with mocked 403 Unauthorized from /api/pd/pdh/selStrPkupStck plus mocked online stock — PASS:
    • pickupStock.status returned "unavailable"
    • pickupStock.reason returned "unauthorized"
    • pickupStock.quantity / pickupStock.inStock returned null
    • onlineStock.quantity returned 13047
    • onlineStock.referenceOnly returned true

Evidence

  • packages/daiso-product-search/src/index.js:55-63 preserves non-OK response metadata in DaisoRequestError instead of losing status/payload context.
  • packages/daiso-product-search/src/index.js:69-74 narrowly classifies only pickup-stock 401/403 Unauthorized failures for conversion.
  • packages/daiso-product-search/src/index.js:116-139 scopes Unauthorized conversion to getStorePickupStock(); unrelated request failures still throw.
  • packages/daiso-product-search/src/parse.js:334-345 normalizes Daiso { success: false, message: "Unauthorized" } into an explicit unavailable pickup-stock result shape.
  • packages/daiso-product-search/src/parse.js:374-381 marks online stock with referenceOnly: true.
  • packages/daiso-product-search/src/index.js:183-206 returns pickupStock and onlineStock as siblings without using online stock as store pickup inventory.
  • packages/daiso-product-search/test/index.test.js:286-342 covers direct 401 pickup Unauthorized conversion and end-to-end 403 pickup Unauthorized with preserved reference-only online stock.
  • packages/daiso-product-search/README.md:23-24 and related skill/docs updates document the conservative Unauthorized behavior and the reference-only online stock contract.
  • .changeset/issue-207-daiso-unauthorized.md adds the required patch Changeset; no test asserts Changeset file presence.

LOW / WATCH follow-ups

  1. packages/daiso-product-search/src/parse.js:334-363 now uses pickupStock.status for both successful retrieval ("available") and blocked retrieval ("unavailable"). This is acceptable for this patch because reason = "unauthorized" and inStock = null are explicit, but if more failure modes appear, consider a stronger retrieval-vs-inventory discriminator such as retrievalStatus or availabilityStatus.
  2. packages/daiso-product-search/src/parse.js:374-381 and packages/daiso-product-search/src/index.js:197-206 keep online stock as a sibling object with quantity/inStock plus referenceOnly: true. The new flag and docs are clear enough for merge, but a future public-contract hardening could move it under something like referenceSignals.onlineStock or add a top-level storeInventoryResolved: false when pickup stock is blocked.

Synthesis

  • code-reviewer recommendation: APPROVE
  • architect status: WATCH
  • final recommendation: COMMENT

RECOMMENDATION: COMMENT

No merge-blocking issues found. The remaining WATCH items are API-contract hardening ideas for future expansion, not blockers for this conservative Unauthorized handling fix.

@vkehfdl1
Clarify pickup stock retrieval state
8715505 
Add explicit retrieval-state metadata so blocked Daiso pickup-stock lookups can be distinguished from resolved inventory results without reinterpreting online stock as store inventory.

Constraint: Daiso Mall Unauthorized responses must be reported conservatively without session or security-policy bypass.

Rejected: Restructuring onlineStock under a new referenceSignals object | broader API churn is unnecessary for this follow-up.

Confidence: high

Scope-risk: narrow

Directive: Preserve onlineStock.referenceOnly semantics; do not treat online stock as store pickup inventory.

Tested: npm test --workspace daiso-product-search; npm run lint --workspace daiso-product-search; mocked Node 403 Unauthorized smoke; git diff --check; npm run ci

Not-tested: live Daiso Mall pickup-stock endpoint because current behavior intentionally handles blocked Unauthorized responses with mocks.
@vkehfdl1

vkehfdl1 commented May 5, 2026

Copy link
Copy Markdown
Member Author

Implementation follow-up

  • Added the approved API-contract hardening for pickup-stock results: successful pickup-stock lookups now include retrievalStatus: "resolved", while Daiso Mall Unauthorized pickup-stock responses include retrievalStatus: "blocked" alongside the existing status: "unavailable" / reason: "unauthorized" shape.
  • Kept onlineStock.referenceOnly: true unchanged so online stock remains an explicit reference signal and is still not treated as store pickup inventory.
  • Updated package README, skill guidance, feature docs, and the patch Changeset to document the retrieval-state contract.
  • Preserved the conservative boundary: no Daiso session/security bypass was added.

Verification

  • TDD: new retrievalStatus assertions failed before implementation and pass after the fix.
  • npm test --workspace daiso-product-search — PASS, 13/13 tests
  • npm run lint --workspace daiso-product-search — PASS
  • Mocked Node smoke for 403 Unauthorized pickup-stock — PASS (pickupStock.status = "unavailable", pickupStock.retrievalStatus = "blocked", pickupStock.reason = "unauthorized", onlineStock.quantity = 13047, onlineStock.referenceOnly = true)
  • Deslop scan on Ralph-owned changed files — PASS, no masking fallback/slop markers found
  • Architect verification — APPROVED, no blockers
  • git diff --check — PASS
  • npm run ci — PASS

@vkehfdl1

vkehfdl1 commented May 5, 2026

Copy link
Copy Markdown
Member Author

CODE REVIEW REPORT

Files reviewed: 8
Total issues: 0 blocking, 1 LOW/WATCH follow-up suggestion
Architectural status: WATCH
Recommendation: COMMENT / approve as-is with follow-up watchlist

Summary

I reviewed the round-3 update with the $code-review workflow, including separate code/spec/security and architecture/devil's-advocate lanes plus fresh local verification. I did not find any CRITICAL, HIGH, or MEDIUM issues that should block merge.

The round-3 hardening addresses the prior retrieval-vs-inventory watch item by adding pickupStock.retrievalStatus: successful pickup-stock lookups now return retrievalStatus = "resolved", while Daiso Mall pickup-stock Unauthorized responses return retrievalStatus = "blocked" with status = "unavailable" and reason = "unauthorized". Online stock remains a separate referenceOnly: true signal and is still not treated as store pickup inventory.

Real Result

Actual verification run locally on origin/feature/#207 at commit 8715505:

  • npm test --workspace daiso-product-search — PASS, 13/13 tests
  • npm run lint --workspace daiso-product-search — PASS (node --check for source and tests)
  • git diff --check origin/dev...HEAD — PASS
  • npm run ci — PASS, including lint, typecheck, tests, skill validation, and npm pack dry-run
  • Manual Node smoke with mocked 403 Unauthorized from /api/pd/pdh/selStrPkupStck plus mocked online stock — PASS:
    • pickupStock.status returned "unavailable"
    • pickupStock.retrievalStatus returned "blocked"
    • pickupStock.reason returned "unauthorized"
    • pickupStock.quantity / pickupStock.inStock returned null
    • onlineStock.quantity returned 13047
    • onlineStock.referenceOnly returned true

Evidence

  • packages/daiso-product-search/src/index.js:69-75 narrowly classifies only pickup-stock 401/403 Unauthorized failures for conversion.
  • packages/daiso-product-search/src/index.js:116-139 scopes Unauthorized conversion to getStorePickupStock(); unrelated request failures still throw.
  • packages/daiso-product-search/src/parse.js:334-346 normalizes Daiso { success: false, message: "Unauthorized" } into status: "unavailable", retrievalStatus: "blocked", reason: "unauthorized", and null stock fields.
  • packages/daiso-product-search/src/parse.js:356-363 marks successful pickup-stock retrievals with status: "available" and retrievalStatus: "resolved".
  • packages/daiso-product-search/src/parse.js:376-383 keeps online stock explicitly referenceOnly: true.
  • packages/daiso-product-search/src/index.js:183-206 returns pickupStock and onlineStock as siblings without using online stock as store pickup inventory.
  • packages/daiso-product-search/test/index.test.js:288-350 covers direct pickup-stock 401 conversion and end-to-end 403 Unauthorized lookup with preserved reference-only online stock.
  • packages/daiso-product-search/README.md:23-24, daiso-product-search/SKILL.md:145-161, and docs/features/daiso-product-search.md:75-87 document the conservative Unauthorized behavior and reference-only online stock contract.
  • .changeset/issue-207-daiso-unauthorized.md adds the required patch Changeset; no test asserts Changeset file presence.

LOW / WATCH follow-up

  1. packages/daiso-product-search/src/parse.js:356-363 still uses pickupStock.status = "available" for successful retrieval, even when quantity = 0 and inStock = false. This is acceptable for this patch because retrievalStatus now carries the retrieval-state contract and inStock carries inventory state, but future public docs/API examples should keep emphasizing that status is not the same as product availability.

Synthesis

  • code-reviewer recommendation: APPROVE
  • architect status: WATCH
  • final recommendation: COMMENT

RECOMMENDATION: COMMENT

No merge-blocking issues found. The remaining WATCH item is a naming/API-doc clarity concern, not a blocker for this conservative Unauthorized handling fix.

@vkehfdl1
Clarify pickup stock inventory state
ddfc717 
Add an explicit pickup-stock inventory state so resolved lookup status cannot be mistaken for product availability while preserving the Unauthorized and online reference contracts.

Constraint: Issue #207 follow-up requires TDD, docs alignment, patch Changeset, and no Daiso session/security bypass.

Rejected: Renaming or removing pickupStock.status | would be a breaking public API change for a patch follow-up.

Confidence: high

Scope-risk: narrow

Directive: Treat pickupStock.status as retrieval/result category; use inStock or inventoryStatus for product inventory availability.

Tested: TDD failure observed before implementation; npm test --workspace daiso-product-search; npm run lint --workspace daiso-product-search; mocked Node 403 Unauthorized smoke; architect verification CLEAR; post-deslop git diff --check && npm run ci.

Not-tested: Live Daiso Mall calls, to avoid relying on or bypassing session/security policy.
@vkehfdl1

vkehfdl1 commented May 5, 2026

Copy link
Copy Markdown
Member Author

Implementation follow-up

  • Added the approved API-contract hardening for pickup-stock inventory semantics: successful lookup results now include inventoryStatus: "in_stock" or "out_of_stock", while Daiso Mall Unauthorized pickup-stock responses include inventoryStatus: "unknown" with the existing blocked shape.
  • Preserved the retrieval-state contract: pickupStock.status remains a retrieval/result category, retrievalStatus remains "resolved" or "blocked", and actual product availability is read from inStock / inventoryStatus.
  • Kept onlineStock.referenceOnly: true unchanged so online stock remains a reference signal and is still not treated as store pickup inventory.
  • Updated package README, skill guidance, feature docs, and the patch Changeset to document the clarified inventory-state contract.
  • Preserved the conservative boundary: no Daiso session/security bypass was added.

Verification

  • TDD: new inventoryStatus assertions for in-stock, zero-stock, and Unauthorized paths failed before implementation and pass after the fix.
  • npm test --workspace daiso-product-search — PASS, 14/14 tests
  • npm run lint --workspace daiso-product-search — PASS
  • Mocked Node smoke for 403 Unauthorized pickup-stock — PASS (pickupStock.status = "unavailable", retrievalStatus = "blocked", inventoryStatus = "unknown", reason = "unauthorized", onlineStock.quantity = 13047, onlineStock.referenceOnly = true)
  • Architect verification — CLEAR / APPROVED, no blockers
  • Deslop scan on changed files — PASS, no masking fallback/slop edits needed
  • git diff --check — PASS
  • npm run ci — PASS

@vkehfdl1

vkehfdl1 commented May 5, 2026

Copy link
Copy Markdown
Member Author

Final verdict: APPROVE

Short reason: PR #209 now implements the conservative Daiso Mall pickup-stock Unauthorized contract without bypassing session/security controls, keeps online stock as an explicit reference-only signal, and documents/tests the retrieval-vs-inventory semantics.

Real Result from actual verification

Verified locally on PR head ddfc717b94a0d7c30f583a00f50f864986d25ee4 against origin/dev:

  • git diff --check origin/dev...HEAD — PASS
  • npm test --workspace daiso-product-search — PASS, 14/14 tests
  • npm run lint --workspace daiso-product-search — PASS
  • npm run ci — PASS, including root lint, typecheck, tests, skill validation, and package dry-run

Evidence checked

  • packages/daiso-product-search/src/index.js:69-139 narrowly converts only pickup-stock 401/403 Unauthorized failures into normalized pickup-stock results; other request failures still throw.
  • packages/daiso-product-search/src/parse.js:334-347 returns blocked Unauthorized pickup stock as status: "unavailable", retrievalStatus: "blocked", inventoryStatus: "unknown", reason: "unauthorized", with quantity/inStock set to null.
  • packages/daiso-product-search/src/parse.js:357-365 returns resolved pickup-stock lookups with retrievalStatus: "resolved" and separates actual inventory via inventoryStatus: "in_stock" | "out_of_stock".
  • packages/daiso-product-search/src/parse.js:370-385 marks online stock as referenceOnly: true.
  • packages/daiso-product-search/test/index.test.js:225-254 covers zero-stock and Unauthorized normalization; packages/daiso-product-search/test/index.test.js:316-376 covers direct 401 pickup-stock conversion and end-to-end 403 Unauthorized with preserved reference-only online stock.
  • packages/daiso-product-search/README.md:23-25, docs/features/daiso-product-search.md:75-88, and daiso-product-search/SKILL.md:146-162 document that pickupStock.status is a retrieval/result category and actual availability comes from inStock / inventoryStatus.
  • .changeset/issue-207-daiso-unauthorized.md adds the required patch Changeset.

No merge-blocking issues found.

@vkehfdl1
vkehfdl1 merged commit e873308 into dev May 6, 2026
1 check passed
@vkehfdl1 vkehfdl1 mentioned this pull request May 8, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@vkehfdl1