fix: resolve spec-drift audit findings (code + specs)#148
Merged
Conversation
From the spec-drift audit: - Remove the legacy DELETE /api/people/:slug route + softDelete service method — an undocumented admin-only soft-delete running parallel to the canonical self|staff POST /deactivate (diverging authz). Superseded by deactivate/reactivate/purge. - commit-meta: emit the Response-Message trailer (HTTP reason phrase) that specs/behaviors/storage.md#commit-message-shape requires; only Response-Code was being written. - PATCH /api/people/:slug: enumerate editable fields + additionalProperties: false so privileged fields (accountLevel) are rejected (422) instead of silently ignored — hardens the dedicated-endpoint rule. type-check + lint clean; people + write-api suites 52/52. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- storage.md: resolve the public/private contradiction. Ground truth: codeforphilly-data is a PRIVATE GitHub repo today holding public-by-design content. Corrected the table claim that it "contains emails, real names, IPs" (which contradicted the redaction section — PII lives in the private store). Fixed "publicly cloneable" framing, the scrub-data.ts path, and clarified commit bodies carry only the caller summary (request-body redaction is moot). - people.md: document slackHandle + deletedAt in the Person response shape (deletedAt gated to self/staff); clarify ?accountLevel= returns empty 200 to non-staff; point the PATCH note at the dedicated account-level endpoint. - legacy-id-mapping.md: tighten the byLegacyId claim — runtime indices exist only for people/projects/blog-posts; the other sheets carry legacyId for import idempotence only. - conventions.md: mark Idempotency-Key as built-but-not-yet-wired (status). plans/spec-drift-fixes.md records real findings, fixes, and the verified false positives (swapPublic, perPage 400-vs-422, the account-level trailer "race") that were checked rather than blindly "fixed". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves the real findings from a spec-drift audit (two scoped
spec-drift-auditorpasses), and records the false positives that were verified away rather than blindly 'fixed'.Code (apps/api)
DELETE /api/people/:slugroute +softDelete— an undocumented admin-only soft-delete parallel to the canonical self|staffPOST /deactivate(diverging authz). Superseded by deactivate/reactivate/purge.commit-meta: emit theResponse-Messagetrailer thatstorage.mdrequires (onlyResponse-Codewas written).PATCH /api/people/:slug:additionalProperties: false+ enumerated fields — privileged fields (accountLevel) are now rejected (422), not silently ignored.Specs/docs
storage.mdpublic/private contradiction — ground-truthed:codeforphilly-datais private on GitHub today, holding public-by-design content. Corrected the table claim it "contains emails, real names, IPs" (contradicted the redaction section). Fixed "publicly cloneable", thescrub-data.tspath, and the moot request-body redaction note.people.md— documentedslackHandle+deletedAtin the Person response shape (deletedAt gated to self/staff); clarified?accountLevel=→ empty 200 for non-staff; PATCH note points at the dedicated endpoint.legacy-id-mapping.md— tightened thebyLegacyIdclaim (runtime indices only for people/projects/blog-posts).conventions.md— Idempotency-Key marked built-but-not-yet-wired.Verified false positives (NOT changed)
reloadInMemoryStatevsswapPublic— the hot-reload path does callswapPublic(reload.ts:76); spec was right.perPageout-of-range "returns 400" — this app remaps validation → 422 (errors.ts:179).Previous-Account-Level: unknown"race" — 404 aborts the commit before any trailer persists.Validation
type-check + lint clean; people + write-api suites 52/52.
🤖 Generated with Claude Code