Skip to content

1.9.79: company linking stamps the customer's updated_at - #311

Merged
tracking202 merged 4 commits into
1.9.79from
1.9.79-ltv-link-updated-at
Oct 28, 2026
Merged

1.9.79: company linking stamps the customer's updated_at#311
tracking202 merged 4 commits into
1.9.79from
1.9.79-ltv-link-updated-at

Conversation

@tracking202

@tracking202 tracking202 commented Oct 28, 2026

Copy link
Copy Markdown
Owner

Backlog: S6C6

Stacked on #304 (1.9.79-ltv-company-linking-cursor, backlog 4J3V). The base is 1.9.79, so until #304 merges this diff also shows #304's commit (27e092a); the change here is the one commit on top of it. Merge after #304.

What changed and why

MysqlCompanyRepository::linkUnlinkedCustomers() (the LTV maintenance job's company-linking sweep) set company_id and company on a customer and left updated_at as it was. Every other write of a customer's company sets it. So a customer the job attached kept the updated_at of its last save on GET /ltv/customers/{id} (p202 ltv customers <id>), and read as unchanged to anything asking what changed since.

  • The link now sets updated_at = time(). That is the clock the rest of the company sweep uses: clearBlankCompanies() (1.9.79: company linking walks past names it cannot link #304) and reconcileAttachedStrings() bind time(). Every other writer binds a time() its caller took, the cron's {$now} included. Nothing writes the column with UNIX_TIMESTAMP() or an injected clock.
  • reconcileAttachedStrings() already set updated_at with time(), so it is unchanged.
  • Machine check. tests/Ltv/CustomerWritesStampUpdatedAtTest walks every served SQL string as PHP assembles it (AssembledStrings):
    • Each UPDATE of 202_customers must assign updated_at for each alias of the table it writes (self-joins, derived tables and joined-only use are read).
    • Each INSERT/REPLACE must name it, and assign it in any ON DUPLICATE KEY UPDATE.
    • It finds 21 writes. One statement builds its SET at run time (applyCrmFields(), which appends updated_at = ? to every SET); it is listed with that reason. Statements whose table is built at run time are listed per file with what names their tables; the CRUD writers' tables are read by reflection (RuntimeWriteTables).
    • An unreadable table name (an interpolation, sprintf's %s) is listed, never read as another table. An unqualified assignment in a multi-table UPDATE is refused.
    • 38 data-provider shapes cover the forms it claims to read.
  • Integration test. CompanyLinkingIntegrationTest::testWhatTheSweepWritesReadsAsChangedSinceBeforeIt runs the sweep on customers last written at 1, then asks which have updated_at >= <time before the sweep>:
    • The linked customer and 1.9.79: company linking walks past names it cannot link #304's cleared blank-name row are listed. The test asks twice: with the predicate a list's updated_since applies (Controller::list()), and through MysqlCustomerCrmRepository::get(), the call that serves GET /ltv/customers/{id}.
    • The pristine NULL row, the detached '' row and the other account's row keep 1.
  • Changelog lines in changelogs.txt and 202-config/changelogs.json. One sentence in documentation/api/20-ltv.md (1.9.79: company linking walks past names it cannot link #304's paragraph). A short paragraph in CLAUDE.md Upgrade to PHP 8 #5 saying the floor covers 202_customers only.

Plants: each was put in, the named test failed, and the file was restored and compared with cmp

Plant Fails
The link's old UPDATE (no updated_at) the integration test (linked customer missing from the changed-since list), CustomerWritesStampUpdatedAtTest (MysqlCompanyRepository.php:738 without updated_at), CompanyTest::testLinkSweepStampsTheEntityAndStoresBlankNamesAsNone
#304's clear without updated_at the integration test (blank row missing), CustomerWritesStampUpdatedAtTest (:773)
The cron's rollup reconcile without c.updated_at = {$now} CustomerWritesStampUpdatedAtTest (ltv_maintenance.php:174)
insertCustomer() naming modified_at for updated_at CustomerWritesStampUpdatedAtTest (MysqlCustomerRepository.php:748)

Every UPDATE 202_customers (and its one INSERT)

Site Writes Sets updated_at
MysqlCompanyRepository::attachCustomer(), by name (:227) company_id, company yes, caller's $now (time())
attachCustomer(), by email domain (:255) company_id, company yes, caller's $now
update() rename (:471) company yes, time()
merge() (:542) company_id, company yes, time()
linkUnlinkedCustomers() link (:738) company_id, company no, now yes, time()
clearBlankCompanies() (#304, :773) company = '' yes, time()
reconcileAttachedStrings() (:814) company yes, time()
MysqlCustomerCrmRepository::merge() target (:339) first_click_id, first_seen_time, last_activity_time yes
merge() source (:365) merged pointer, status, company, company_id, rollups yes
merge() earlier merges re-pointed (:378) merged_into_customer_id yes
erase() (:416) PII, company, status yes
reconcileCustomer() (:532) rollups yes
applyCrmFields() (:657, SET built at run time) CRM columns yes, appended to every SET it runs
syncCompanyLink() detach (:699) company, company_id yes
MysqlCustomerRepository::adjustRollups() (:462) rollups, last_activity_time yes
MysqlCustomerRepository::insertCustomer() INSERT (:748) all yes
MysqlEngagementRepository engagement touch (:77) last_activity_time yes
MysqlSubscriptionRepository::refreshCustomerSubscriptionRollups() (:514) mrr, active_subscription_count yes
static-endpoint-helpers.php engagement touch (:328) last_activity_time yes
ltv_maintenance.php subscription sweep refresh (:140) mrr, active_subscription_count yes, cron's $now
ltv_maintenance.php rollup reconcile (:174) rollups yes, cron's $now, and on rows that did not change (finding below)

The two readers the item named

  • GET /ltv/customers?updated_since=. This filter does not exist. LtvController::customers() reads limit, offset, sort, dir, segment, q and the LTV window, and ignores everything else. Measured live: ?updated_since=<now> answered 200 with all 3 customers. Reported as a finding, not added here.
  • Sync. grep -rn "ltv" 202-config/Sync go-cli/cmd/sync* finds nothing: 202-config/Sync does not exist and go-cli/cmd/sync.go names no LTV entity. SyncEngine::ENTITY_ENDPOINTS carries only these: aff-networks, ppc-networks, ppc-accounts, campaigns, landing-pages, text-ads, rotators, trackers. No sync profile runs against LTV customers, so there is nothing to show picking one up.
  • The reader that does exist is updated_at on GET /ltv/customers/{id} (p202 ltv customers <id>). The integration test and the live pass read that one, plus the updated_at >= ? predicate a future filter would apply.

Live end-to-end (port 8165, database p202_s6c6_live)

Without the licence. The licence stub is down and install.php refuses the install at its licence check ("We couldn't confirm that license key"). So I ran install.php's own steps without that check, from a scratch script: INSTALL::install_databases(), InstallAccount::create(), the 202_api_keys insert and install_database_partitions(). php -S ran with every proxy variable pointed at 127.0.0.1:9. The v3 API answered without the licence.

Steps:

  1. Created three customers with POST /api/v3/ltv/customers (company "Acme Corp").
  2. With SQL, made two of them rows stored before their company existed: company_id = NULL with company = 'Initech Inc', and company_id = NULL with company = "\t". All three got updated_at = last_activity_time = 1700000000, outside the reconcile's 48h window.
  3. Ran php 202-cronjobs/ltv_maintenance.php, then read each customer with GET /api/v3/ltv/customers/{id}.

Results:

  • With the old link planted: {"customer_id":1,"company":"Initech Inc","company_id":2,"updated_at":1700000000}. The customer was attached and the stamp did not move. The blank-name row did move (1.9.79: company linking walks past names it cannot link #304's clear).
  • With this commit: {"customer_id":1,"company":"Initech Inc","company_id":2,"updated_at":1793183727}, with the run starting at 1793183726. The blank-name row moved too, and customer 3 (untouched) stayed at 1700000000.

Verification

.claude/skills/p202-verify/scripts/verify.sh --since origin/1.9.79 --changed (after the commit; PHP 8.3.6, full vendor/; unit: 6147 tests, 0 failures, 4 skipped, the documented DB-singleton skips):

Scope report
------------
  patterns   PASS
  phpcs      PASS
  phpstan    PASS
  syntax     PASS
  unit       PASS

Not covered by this script:
  live      stand up an instance, drive the path a user would take
  agent     add and run a case under tests/fixtures/agent-eval/cases/
  ci        a workflow still in_progress has not passed; read the run

Integration suites run by hand against a scratch database (p202_s6c6_test, MariaDB 10.11):

  • --group integration tests/Ltv/: OK (56 tests, 4707 assertions). This includes CompanyLinkingIntegrationTest (4 tests) and LtvDatabaseIntegrationTest, which calls the sweep and reconcileAttachedStrings().

Could not run

  • The Go CLI (p202 ltv customers <id>): not run. The shared brief says it gets 402 from an instance installed without the licence; I did not try it. The endpoint it calls was exercised directly.
  • The schema tier: no schema path changed, and --since origin/1.9.79 --changed did not select it.
  • Agent eval: not agent-facing beyond the value of an existing field.
  • CI: not observed.

For a reviewer

  • Clock. The link binds time() at each UPDATE, as clearBlankCompanies() does. It does not use one time taken at the start of the call. Each statement commits on its own (the cron runs the sweep outside a transaction), so the time at the statement is the closest to when the change becomes visible.
  • Scope of the check. It holds 202_customers only. Running the same reading over the 21 other tables that keep updated_at found writers that change an API-visible column without it (findings below). They are internal bookkeeping or another concern, so they are reported here, not widened into this PR.

Findings outside this item

  • GET /ltv/customers ignores updated_since: it answers 200 with every customer (measured). Reading the handler, it reads only its own parameters, so any other unknown parameter is ignored the same way.
  • Adding or deleting a customer alias (POST /ltv/customers/{id}/aliases, DELETE …/aliases/{aliasId}), and a save carrying only aliases or custom_fields, leave the customer's updated_at as it was. Measured for an alias add: it answered 201, and the detail showed the new alias with updated_at unchanged. The delete and the aliases/custom_fields-only save were read from the code, not measured.
  • The rollup reconcile in ltv_maintenance.php stamps updated_at = {$now} on every customer active in the last 48h whether or not a rollup changed. It reports each one as "corrected": measured "1 customer rows corrected" on two runs in a row for a customer with nothing to correct.
  • customer.updated webhooks fire for API upserts, patches, merges and erasures. They do not fire for a company rename or merge that rewrites attached customers' company, or for the maintenance job's link or blank-name clear.
  • ModelRepository::clearDefault() clears is_default on other attribution models, and AppIcons::store() writes an app registration's app_icon. Neither sets those rows' updated_at.
  • GET /ltv/customers and GET /ltv/customers/{id} answer total_revenue, refunded_amount and mrr as decimal strings ("0.00000"), where the LTV summary answers numbers since 17305ee.

Generated by Claude Code

claude added 2 commits October 28, 2026 09:29
@claude
1.9.79: company linking walks past names it cannot link
27e092a 
MysqlCompanyRepository::linkUnlinkedCustomers() took up to $limit of an
account's customers with a company string and no company_id, in no order,
and skipped the ones whose string canonicalName() reduces to '' (a tab, a
no-break or ideographic space). A skip wrote nothing, so the next run took
the same rows first: the cron asks for 2,000 per account, and an account
with more than 2,000 such customers never had anyone else linked (each run
reported 0 attached).

- The sweep walks the account's customers in customer_id order with a
  cursor, until $limit are linked or none are left, so a return below
  $limit means the account is done (the contract the cron's loop in #285
  reads).
- A string that names nothing is stored as '' in one guarded UPDATE per
  batch (still unlinked, still holding the string read). '' is what
  clearing an unlinked string writes, and keeps the one reader that tells
  NULL from '' (domain auto-attach) answering as it did for the blank
  string. Doing it in SQL instead would need a second reading of
  canonicalName() that MariaDB, MySQL and PHP disagree on.
- The select names its index: left alone, MariaDB 10.11 served the cursor
  from merged_into (every account's customers in id order) and read
  102,000 rows to return 2,000; FORCE INDEX (user_company) reads 2,000.
- A company id of 0 from findByName() or resolveOrCreate() was skipped the
  same way in the sweep (taken again every run) and in attachCustomer()
  (a CRM save answered done with the company unattached). Both now throw a
  QueryException naming the company and account.

The cron's per-account probe asks the select's predicate without the
cursor; once the blank strings are stored as '' it stops finding them, so
the two agree and the cron no longer calls the sweep for nothing.

Tests: CompanyLinkingIntegrationTest seeds 2,100 unlinkable names ahead of
a linkable one and runs the cron's call (2,000); it fails on the old code
(0 linked). CompanyTest covers the cleared statement, the cursor binds and
the refusals. CLAUDE.md #37 records the shape.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
@claude
1.9.79: company linking stamps the customer's updated_at
3cb3be3 
MysqlCompanyRepository::linkUnlinkedCustomers() set company_id and company
on a customer whose company was stored before the company existed and left
updated_at as it was. Every other write of a customer's company sets it
(attachCustomer(), a rename, a merge, the CRM detach, #304's clear of a
blank name, reconcileAttachedStrings()), so a customer the LTV maintenance
job attached answered the time of its last save as its updated_at on
GET /ltv/customers/{id}, and read as unchanged to anything asking what
changed since. Measured on a live instance: before, the linked customer
kept updated_at 1700000000 after the cron attached it; after, it is the
time of the run.

- The link sets updated_at = time(), the clock the rest of the sweep uses
  (clearBlankCompanies() and reconcileAttachedStrings() bind time(); every
  other writer binds a time() its caller took; nothing writes the column
  with UNIX_TIMESTAMP()).
- CustomerWritesStampUpdatedAtTest walks every served SQL string and holds
  each UPDATE of 202_customers to assigning updated_at for each alias of the
  table it writes, and each INSERT or REPLACE to naming it (and assigning it
  in an ON DUPLICATE KEY UPDATE). It finds 21 writes; the one SET built at
  run time (applyCrmFields()) and the UPDATEs whose table is built at run
  time are listed with what decides them, and the CRUD writers' tables are
  read by reflection. It fails on the link as it was, and on the cron's
  rollup reconcile and the customer INSERT with the column removed.
- CompanyLinkingIntegrationTest runs the sweep on customers last written at
  1 and asks which have updated_at at or after a time taken before it: the
  linked customer and the cleared blank name, through the predicate a
  list's updated_since applies and through the repository call that serves
  GET /ltv/customers/{id}; the untouched ones keep 1. It fails with the old
  link, and with #304's clear missing its stamp.
- CLAUDE.md #5 records that a row's change stamp is a sweep over its
  writers, and that only 202_customers is held to it.

Neither reader the item named exists: GET /ltv/customers takes no
updated_since (it answers every customer, measured), and no sync carries
LTV customers. Both are noted in the PR as findings.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
Copilot AI balanced review requested due to automatic review settings October 28, 2026 10:51
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 28, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-28T10:51:38.986059Z
We couldn’t complete this review. Comment @codex review to try again.		 3cb3be3 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 28, 2026

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Copilot AI reviewed Oct 28, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@github-actions

github-actions Bot commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Code review: CLEAN

For 3cb3be3: 0 issue(s). The review is encrypted for the maintainers in the review-findings artifact of this run.

@github-actions

github-actions Bot commented Oct 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Adversarial review: NON-BLOCKING

For e8050da: 5 finding(s). The report is encrypted for the maintainers in the review-findings artifact of this run.

Rewritten on every push; the check fails on BLOCKING.

claude added 2 commits October 28, 2026 12:16
@claude
Merge 1.9.79 into 1.9.79-ltv-company-linking-cursor
e1307f0 
Brings in 1.9.79's new commits. The conflicts were in 202-config/changelogs.json, changelogs.txt, where both sides added lines: the incoming lines are kept as they are and this branch's follow.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
@claude
Merge ltv-company-linking-cursor into 1.9.79-ltv-link-updated-at
e8050da 
Brings in ltv-company-linking-cursor (this branch's stacked base), and through it 1.9.79's new commits. The conflicts were in 202-config/changelogs.json, where both sides added lines: the incoming lines are kept as they are and this branch's follow.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
@tracking202
tracking202 merged commit 966b96d into 1.9.79 Oct 28, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tracking202 @claude