Skip to content

test(identity): prove same-subject relink drops ended capability - #236

Draft
cursor[bot] wants to merge 33 commits into
mainfrom
cursor/bc-add682ec-6f5d-45f4-9946-2cf6e666449b-9b92
Draft

cursor[bot] wants to merge 33 commits into
mainfrom
cursor/bc-add682ec-6f5d-45f4-9946-2cf6e666449b-9b92

Conversation

@cursor

@cursor cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Why

PR #222 binds account-linked capability to the current link_event_ref. The existing rebound test used a different subject, so a missing event check could still pass. A buyer who unlinks and later attaches the same Keyverse subject under a new event must not keep the pre-unlink grant.

TDD

RED same_subject_relink_with_a_new_event_rejects_the_ended_grant and persisted_same_subject_relink_rejects_the_ended_account_capability were missing. GREEN proves accept fails closed on the ended grant after same-subject relink, including against load_participant_identity_history after persist. Also covers unknown accept time and grant None for unlinked or foreign-tenant bindings.

Scope

Out of scope

  • HTTP account-link transport / OpenAPI
  • Live Keyverse token verification
  • Invalidating anonymous assessment sessions

Operator next action

Review the same-subject relink fail-closed proof. Do not merge #222 as a substitute if this unique-invariant head is the landing vehicle. Do not merge until exact-head checks and independent last-push approval are satisfied. Never self-approve.

Open in Web View Automation 

cursoragent and others added 25 commits August 16, 2026 15:25
A buyer who links an anonymous assessment to a Keyverse account must
still see that link after process restart. Persist assessment_participant
plus append-only link and link-end evidence, reload through the domain
lifecycle, and fail closed on conflicting replay or a subject already
bound to another participant.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Keep TRACEABILITY, ADR-0020, ERD, and as-built schema pointing at the
opened persist/reload vehicle instead of an unnamed Active PR.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
The Active PR #114 naming commit stored an empty ADR-0020. Restore the
accepted decision, including the #114 persistence status and APA 7
references, so identity-link governance is not silently deleted.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Apply each identity link and then its matching ends in one transaction so a complete in-memory unlink+relink aggregate survives restart. Cover one-shot persist, exact replay, and subject reuse after unlink.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Add a tenant-scoped current-subject lookup so a returning Keyverse login can find the same product-owned participant after the anonymous session token is gone. Ended or replaced subjects stay unfindable until they are current again.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Renumber the identity-link migration so it does not collide with #113 scoring-job health indexes on 0021. Name Active PR #124 as the merge candidate over #114.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Remove the accidentally committed build tree and ignore /target so later local verification cannot leak compiler outputs into the identity-link successor.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
A missing current_participant_identity_link row no longer hides a
returning Keyverse login or lets another participant bind the same
issuer-scoped subject. Lookup and uniqueness now read append-only
link rows that have no matching end.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Name the Active PR vehicle as the successor of #124 so TRACEABILITY,
ADR-0020, and the as-built schema do not treat projection-only lookup
as the landing contract.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Add a composite foreign key so a link-end or current projection cannot
point at another participant's identity-link row. Name Active PR #133
as the landing vehicle over #124 and #114.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Exact replay of the same identity-link history now reconciles the derived
current projection so operator repair cannot hide a returning login behind
a missing unique enforcer or leave a stale row after unlink. Name Active
PR #133 in TRACEABILITY instead of superseded #124.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Point TRACEABILITY, ADR-0020, ERD, and the as-built schema at this
successor so operators do not merge superseded #133, #124, or #114.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
A buyer who proves control of both the anonymous session and a Keyverse
account can persist that link and later recover the same product-owned
participant from a still-valid account proof. Expired proofs fail before
persist or lookup.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Point TRACEABILITY, ADR-0020, UML, and the as-built schema at the hosted
dual-proof persist/recover commands so operators do not treat persist-only
#147 as the last identity-link landing vehicle.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Point TRACEABILITY, ADR-0020, ERD, and the as-built schema at this
successor so operators do not merge superseded #147, #133, #124, or #114.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
#158 rebuilds current projections after restore. #160 adds dual-proof
write/recover commands. They share persist files, so merge them
sequentially after rebase instead of treating either as a replacement.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
A still-valid account proof must not receive a participant whose current
tenant, issuer, or subject no longer match that proof. Keep the loaded
record only when the current binding still matches, and cover expired,
other-tenant, and ended-subject recover paths.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Keep TRACEABILITY, ADR-0020, ERD, and the as-built schema pointing at
the opened recover current-binding successor instead of an unnamed
#160 follow-up.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Add hosted unlink so a still-valid Keyverse proof can end the matching
current binding. Reload stored history before authorization so a stale
in-memory record cannot unlink a rebound subject. Exact replay is
idempotent; expired proofs and unused accounts fail closed.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Record #206 as the write/recover/unlink landing over #176. Keep #202 as
the inspect-line unlink vehicle and #158 as restore reconcile.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Unlink already stops recover from returning the participant. A previously
recovered participant_ref could still be treated as an account grant.
Bind that grant to the current link_event_ref and re-check it so unlink
or rebound fails closed.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Record the post-recover grant/accept gate as Active PR #222 so
traceability and ADR-0020 keep persist unlink (#206) distinct from
link-event-bound account capability.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
A grant issued against a stored current binding must fail closed after
persist_authorized_account_unlink, matching recover returning None.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Subject match alone must not keep a pre-unlink grant. Relink the same
issuer-scoped subject under a new link_event_ref and accept the ended
grant against reloaded history so the unique event bind stays fail-closed.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Keep #222 as the grant/accept gate and name #236 as the same-subject
relink proof so operators land the unique link-event invariant.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
# Conflicts:
#	.gitignore
#	CHANGELOG.md
#	docs/TRACEABILITY.md
#	docs/architecture/ERD.md
#	src/lib.rs
The documentation contract requires the ERD to state that persist/reload of
assessment_participant remains Target while identity-link persistence is
described on its active PR lane.
@seonghobae
seonghobae marked this pull request as ready for review August 27, 2026 09:16

@devin-ai-integration devin-ai-integration Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 4 potential issues.

Devin Review

Comment thread docs/TRACEABILITY.md
Comment on lines +126 to +127
├── account_link.rs # dual-proof authorization before participant identity mutation
├── account_link_write.rs # Active PR dual-proof persist/recover/unlink plus link-event-bound account capability (not protected-main truth)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Traceability module map lists the same module twice

The protected-main module map already names account_link.rs, and this change adds a second account_link.rs line with a different description, so one module appears twice. The same insertion drops active-PR-only modules into a block whose header declares it the protected-main surface.

Prompt for agents
docs/TRACEABILITY.md section 4 'Source module map' is headed as the current protected-main Rust module surface. account_link.rs is already listed at line 106, but a second account_link.rs entry was added at line 126, duplicating the module. Additionally, account_link_write.rs and postgres_participant_identity_link.rs (both annotated 'not protected-main truth') were inserted into this protected-main list, which contradicts the section header. Remove the duplicate account_link.rs entry and move the active-PR module entries out of the protected-main surface listing (e.g. into the 'Active implementation work that is not protected-main truth' subsection) so the map stays accurate.
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +336 to +346
if inserted == 1 {
insert_current_projection(
transaction,
participant_ref,
identity_link_ref,
tenant_ref,
identity_issuer,
identity_subject_ref,
)?;
return Ok(true);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Projection insert and reconcile use different conflict semantics

A newly inserted link writes the current projection via a plain INSERT in insert_current_projection, while reconcile_current_projection later UPSERTs. Normal link/unlink/relink flows delete the participant projection row before the next link inserts, so no collision occurs. If a stale projection row survives operator repair and a new link is inserted in the same call, the plain INSERT hits the participant primary key and is reported as ConflictingReplay, though the trailing reconcile would have fixed it. Requires manual corruption; not a normal-path fault.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +588 to +606
fn reject_subject_bound_to_another_participant(
transaction: &mut Transaction<'_>,
participant_ref: &str,
tenant_ref: &str,
identity_issuer: &str,
identity_subject_ref: &str,
) -> Result<(), IdentityLinkPersistenceError> {
match current_subject_participant(
transaction,
tenant_ref,
identity_issuer,
identity_subject_ref,
)? {
Some(holder) if holder != participant_ref => {
Err(IdentityLinkPersistenceError::SubjectAlreadyBound)
}
Some(_) | None => Ok(()),
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Cross-participant subject uniqueness rests only on the projection constraint

reject_subject_bound_to_another_participant reads unterminated links with FOR SHARE under READ COMMITTED, and the history table has no unique constraint on (tenant, issuer, subject). Two concurrent links of the same subject to different participants both pass this check. Only the UNIQUE constraint on the current projection stops the second committer, failing it closed with SubjectAlreadyBound. The invariant holds, but entirely through the projection constraint, not the history check.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread src/account_link_write.rs

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 new potential issues.

Devin Review

Comment on lines +383 to +421
fn reconcile_current_projection(
transaction: &mut Transaction<'_>,
participant: &ParticipantRecord,
) -> Result<(), IdentityLinkPersistenceError> {
let participant_ref = required_reference(participant.participant_ref())?;
if let Some(event) = current_link_event(participant) {
let tenant_ref = required_reference(participant.tenant_ref())?;
let identity_link_ref = required_reference(event.link_event_ref())?;
let identity_issuer = required_reference(event.issuer_ref())?;
let identity_subject_ref = required_reference(event.subject_ref())?;
match transaction.execute(
"INSERT INTO current_participant_identity_link (\
participant_ref, identity_link_ref, tenant_ref, identity_issuer, \
identity_subject_ref\
) VALUES ($1, $2, $3, $4, $5) \
ON CONFLICT (participant_ref) DO UPDATE SET \
identity_link_ref = EXCLUDED.identity_link_ref, \
tenant_ref = EXCLUDED.tenant_ref, \
identity_issuer = EXCLUDED.identity_issuer, \
identity_subject_ref = EXCLUDED.identity_subject_ref",
&[
&participant_ref,
&identity_link_ref,
&tenant_ref,
&identity_issuer,
&identity_subject_ref,
],
) {
Ok(_) => Ok(()),
Err(error) => Err(classify_current_unique_violation(error)),
}
} else {
transaction.execute(
"DELETE FROM current_participant_identity_link WHERE participant_ref = $1",
&[&participant_ref],
)?;
Ok(())
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Projection upsert depends on same-row conflict

reconcile_current_projection upserts on the participant_ref arbiter while a second unique index on (tenant, issuer, subject) also exists. The same-row case (fresh persist, replay) satisfies both on one row, so no spurious error. A subject held by a different participant raises the second unique and is mapped to SubjectAlreadyBound fail-closed. Behavior is correct.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +187 to +219
let links = load_link_events(transaction, participant_ref)?;
let ends = load_link_end_events(transaction, participant_ref)?;
for link in &links {
record
.link_account(
link.link_event_ref(),
link.issuer_ref(),
link.subject_ref(),
link.anonymous_proof_ref(),
link.authenticated_proof_ref(),
link.linked_at_unix_ms(),
)
.map_err(|_| IdentityLinkPersistenceError::CorruptHistory)?;
for end in ends
.iter()
.filter(|event| event.linked_event_ref() == link.link_event_ref())
{
record
.record_link_end(
end.link_end_event_ref(),
end.evidence_ref(),
end.ended_at_unix_ms(),
)
.map_err(|_| IdentityLinkPersistenceError::CorruptHistory)?;
}
}
if ends.iter().any(|end| {
links
.iter()
.all(|link| link.link_event_ref() != end.linked_event_ref())
}) {
return Err(IdentityLinkPersistenceError::CorruptHistory);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Reload replay order matches domain invariants

load_participant_identity_history orders links by linked_at_unix_ms and applies each link's ends before the next link. This reconstructs the legal interleaving the domain enforced at write time; orphan ends and illegal ordering surface as CorruptHistory or a replay error. No ordering defect.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor

Admission-state correction for exact head 9b845c318d1032d7dc9b3ddbb70cd05a6a6051e6.

Finding: live main comparison is diverged (32 ahead / 4 behind), Runtime CI failed, and 5 review threads remain unresolved.

This PR remains Open and is moved to Draft/Proposed. Its commits, reviews, threads, and valid delta are preserved. Return it to Ready after causal repair/non-force reconciliation and fresh exact-head evidence. No bypass, synthetic status/approval, manual rerun, Force Push, review dismissal, or Close is used.

@seonghobae
seonghobae marked this pull request as draft September 19, 2026 22:02
@coderabbitai

coderabbitai Bot commented Sep 19, 2026

Copy link
Copy Markdown

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 2fbf2500-cb9d-4723-88aa-fa06532a1c9a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

seonghobae commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Exact-head CI RCA / minimal repair — 04f5f264881722bcd596e539b6cd6ddd712578b3.

  • Root cause: Runtime CI run 33060332306 / job 98477297843가 clippy::doc_markdown을 warnings-as-errors로 실행했고, tests/postgres_account_link_failed_persist_state.rs:1의 public test-module documentation에 PostgreSQL backticks가 없어 compile admission이 중단됐습니다. Coverage 두 job의 failure는 같은 source head에 대한 후속 cascade입니다.
  • Minimal GREEN: 문서 식별자만 `PostgreSQL`로 교정했습니다. Product behavior, SQL, identity lifecycle, dependency graph와 test logic은 변경하지 않았습니다.
  • Fresh local Rust 1.97.1 evidence: targeted clippy PASS; cargo clippy --all-targets -- -D warnings PASS; cargo fmt --all -- --check PASS; git diff --check PASS. Remote blob SHA b7cb51302c7998fc96d84556d81ebc29ce34c94b를 재확인했습니다.
  • Fresh hosted runs 35475440724, 35475440759, 35475440758, 35475440779, 35475440777, 35475440783은 모두 queued입니다. Non-terminal evidence를 GREEN으로 간주하지 않습니다.

PR은 Draft/open으로 유지합니다. Force Push, destructive rebase, gate 완화, self-approval 또는 manual rerun은 사용하지 않았습니다.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants