feat(people): add purpose-bound Employment history read - #149
Draft
seonghobae wants to merge 22 commits into
Draft
feat(people): add purpose-bound Employment history read#149seonghobae wants to merge 22 commits into
seonghobae wants to merge 22 commits into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
Scheduled review-feedback autofix for this PR head.
|
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
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.
Buyer-visible HRIS gap
Protected
develop@9e3e4847510e1e612b48474ba42b177b8ed824dfhas authoritative Employment identity/version truth, but the employee-profile backend still needs a purpose-bound API contract for bitemporal Employment history. This Orgmetra-only slice owns that read boundary. It does not mutate Employment, infer attendance/fitness/compensation/performance, authorize an employment decision, query another service's application tables, or mutate a dedicated-writer dependency.Test-first integrity evidence
The original contract-first head was
23c3417edd7024ecc4c1c64f2d7017b573ab9eaf; its hosted run did not reach terminal RED before the branch advanced, so no RED is claimed for that predecessor.Subsequent reviews found and repaired three narrower alias/capture issues. The latest review found the root weakness that remained: the accepted persistence row was a frozen dataclass, and
object.__setattr__could still rewrite its fields in place, leaving correctness dependent on mutation detection after the fact.Exact test-only head
c07ce7baf738679e1ef5cbef1d98760fefe670e3addedtest_persistence_record_is_structurally_immutable_against_object_setattrbefore the root repair. People API Quality run33257244737, exact checkout job99113016031, produced a genuine terminal RED: 1 failed / 159 passed because the pre-repair record allowedobject.__setattr__; owned production coverage still remained 1,524/1,524 statements and 508/508 branches.Root repair
The root repair begins at
6ef636cdf803ef3195f80db089f1ee432e0d7646.EmploymentHistoryRecordnow uses tuple-backed immutable storage rather than a frozen dataclass. A persistence adapter retaining an accepted row therefore cannot rewrite its fields in place through ordinary assignment orobject.__setattr__. The People service still reconstructs every exact-type persistence row through the public validating constructor before tenant/Person scope checks, system-time visibility, version uniqueness, business-time overlap checks, deterministic sorting, or purpose-bound field emission. That second boundary is required because low-leveltuple.__new__can bypass the public constructor; deliberately forged exact-type tuple rows are covered and fail closed at runtime integrity validation.This is an in-process object-integrity boundary only. It does not claim to replace a transactional database snapshot, MVCC, locks, or the persistence adapter's obligation to return one coherent bitemporal view. ADR 0149 and executable traceability record the same boundary.
Exact-current-head evidence
Current exact head:
44c83128701f1985f8566b39cbf837c7b20f0111Fresh live base:
develop@9e3e4847510e1e612b48474ba42b177b8ed824dfPeople API Quality run
33257526770/ job99113735308is terminal SUCCESS on this exact head: 160 tests passed, owned production 1,521 statements / 506 branches, exact 100.00% statement and branch coverage, compile and clean-checkout GREEN. Recovery Rehearsal Quality33257526810, SAST Semgrep33257526769, and Foundation CI33257526857are also terminal SUCCESS on the exact head.Required OpenCode current-head check
100046874858is now terminal SUCCESS after central control-plane repair and revalidation on 2026-09-02. The buyer-facing source head did not change; the earlier failed OpenCode verdict remains predecessor execution evidence only and is not transferred.Required Noema current-head check
100046855876is now terminal SUCCESS after central review-sidecar repair and revalidation on 2026-09-02. The exact Orgmetra head is unchanged, so this is fresh review evidence on the same product revision rather than evidence inherited from another commit.Security Scan run
33257526786remains terminal FAILURE solely at the diff-scoped Dependency Review boundary. Dependency-review jobs99113735568(attempt 1) and99114237208(attempt 2) both verify the exact head, then the public-repository dependency-graph compare for exact base/head returns HTTP 403 with curl exit 0;actions/dependency-review-actionis therefore skipped and the workflow correctly fails closed. Attempt 2 OSV, Trivy, and Scorecard are GREEN but do not substitute for missing diff-scoped dependency-review evidence. Orgmetra has no branch-owned.github/workflows/security-scan.ymlat this head, so this central owner boundary remains routed through existing.github#810.Required Strix current-head check
100046582747is terminal CANCELLED after the latest central revalidation attempt. That is not a clean Strix verdict and is not upgraded to success. The earlier exact-head Strix failure established that target/base materialization, trusted-workflow smoke, sidecar/healthz, provider-route preflight, gatewaychat/completionspreflight, and pinnedstrix-agent==1.5.3installation succeeded before the trusted gate rejected the local gateway URL contract. The central owner lane remains.github#1327; this leaf branch does not add a workaround or fabricate a vulnerability finding/no-finding report.Merge governance
GitHub reports this PR open and Draft. Current exact-head OpenCode and Noema are now GREEN, while Strix still lacks a terminal clean verdict and Security remains fail-closed on the Dependency Review 403. The inherited organization ruleset also still carries its live approval-count/bypass drift. Keep Draft and do not merge until the unchanged exact head satisfies every applicable local/central check and the ordinary protected path is reconciled. Do not self-approve, use administrator bypass, transfer evidence from another head/PR, manufacture status evidence, or mutate a dedicated-writer dependency.