feat(observability): health IM boundary + agent-turn latency/error metrics (OCT-17)

[lml2468]

Summary

Observability for the end-to-end agent turn on the server↔channel (WuKongIM) boundary (OCT-17, sub-task of OCT-10).

• Health (/v1/health): already probed db + redis; now also probes WuKongIM reachability. Any HTTP response (even non-2xx) counts as reachable — the probe asserts the IM API answers on the network, not that a specific health route exists, so readiness isn't coupled to a WuKongIM version/path. Only a transport failure marks im=down; unconfigured APIURL reports im=unknown without failing overall readiness. 2s timeout bounds a hung IM control plane. HTTP stays 200 (aggregate status in body) — additive, non-breaking for existing consumers.
• Agent-turn metrics (modules/bot_api/metrics.go): instrument the bot reply delivery leg (sendMessage → dispatchMsgSendReq):
  • dmwork_agent_turn_delivery_duration_seconds (HistogramVec) — latency of bot reply delivery to the channel.
  • dmwork_agent_turn_delivery_total{result} (CounterVec) — result=error is the failed agent-turn deliveries counter.
  • Registered on the default registry via promauto (consistent with modules/oidc/metrics.go); exposed by the existing /metrics scrape. Labels kept low-cardinality: channel_type{person|group|topic|other} × result{ok|error}, pre-warmed to zero series.

Acceptance (from OCT-10)

• Health check present (extended to the channel/IM boundary).
• At least one latency + one error metric for the agent turn.

Test plan

• go build ./...
• go vet ./modules/bot_api/ ./modules/common/ (only linter gated in CI)
• go test ./modules/bot_api/ -run 'TestAgentTurn|TestObserveAgentTurn|TestBotAPINoLegacyResponseError'
• go test ./modules/common/ -run 'TestIMReachable'
• CI green (full suite needs MySQL/Redis/WuKongIM)

No REST/WS contract break (health gains an additive im field; metrics are new). No auth/secrets/permissions changes.

[mochashanyao]

[Octo-Q · automated review]

Verdict: Approve — no blocking findings; notes below (data-flow traced).

---

PR #407 Review Report — OCT-17 Observability + Bot Provisioning + Auth Extensions

Reviewer: Octo-Q (automated review)
Head SHA: dfea4c5613e9fdb3dd82c4014538c77d8f9cf9bb
Repo: Mininglamp-OSS/octo-server
Scope: security_sensitive (new auth endpoints, credential handling, authorization gates)

---

1. Verification Summary

Area	Status	Evidence
Agent-turn metrics (metrics.go)	✅	Low-cardinality labels, init pre-warm, correct data flow from req.ChannelType uint8 (send.go:27) → agentTurnChannelTypeLabel() switch with default "other"
Health IM probe (common/api.go)	✅	Config-sourced URL (not user input → no SSRF), 2s timeout, empty→"unknown", transport-fail→"down"
Bot mint (bot_provision/bot_api.go)	✅	Session auth → assertSpaceMember (3-way JOIN: space.status=1 + user.status=1 + sm.status=1) → MintBotOBO
Bot token (bot_provision/bot_api.go)	✅	api_key Bearer → resolveAPIKey (status=1 + client_id='botfather') → assertSpaceMember → SQL cross-space filter + creator check
Auth verify-token (user/api.go:4011)	✅	OwnedBots UNCONDITIONAL (v3.3.5 fix), ?include=context fail-secure (empty lists, not 500)
Auth verify-api-key (user/api.go:4333)	✅	3-step: key lookup → 3-way JOIN membership → opt-in ownedBotsBySpace
Runtime onboarding (botfather/api_runtime_onboarding.go)	✅	Session auth → CheckMembership → user.status=1 gate → getOrCreateAPIKey → URL sanity check
SQL injection safety	✅	All new queries use parameterized ? placeholders via dbr
Anti-enumeration (bot_provision)	✅	All credential failures collapse to ErrBotProvisionAuthFailed 401
Rate limiting (botToken)	✅	1000 req/min/IP "verify" bucket, appropriate for credential-touching endpoint
i18n error codes	✅	5 new bot_provision codes + ErrUserAPIKeyInvalid registered with zh-CN/en-US translations

2. Findings

F1 — P2: Hardcoded MinIO credentials committed to tracked config

File: configs/tsdd.yaml (new block added by this PR)
Lines: PR diff +135–144

fileService: "minio"
minio:
  url: "http://127.0.0.1:9000"
  accessKeyID: "admin"
  secretAccessKey: "12345678"

Diff-scope: NEW — this PR adds the block. The file is tracked in git (not in .gitignore). The inline comment acknowledges "Production should set proper AccessKey/SecretKey via env vars, not commit creds" but the creds are committed.

Risk: admin/12345678 are MinIO's factory defaults. They only reach 127.0.0.1:9000 so no remote production exposure, but:

• Anyone deploying from this config without overriding gets a publicly-known storage credential
• Sets a precedent that weak creds in config are acceptable

Recommendation: Move the dev block to a gitignored configs/tsdd.dev.yaml (add to .gitignore), or replace with env-var references (${MINIO_ACCESS_KEY}). Keep the commented-out template as documentation.

F2 — P2: New http.Client per health probe call (no connection reuse)

File: modules/common/api.go, imReachable() function

func imReachable(apiURL string) error {
    client := &http.Client{Timeout: imHealthTimeout}  // new client each call
    ...
}

Diff-scope: NEW — introduced by this PR.

Impact: Each health probe creates a new http.Client with default Transport (no connection pooling). For a health endpoint probed every 10–30s by orchestrators, this means a fresh TCP+TLS handshake per probe. Not a correctness issue but wasteful.

Recommendation: Use a package-level var imHealthClient = &http.Client{Timeout: imHealthTimeout} or configure a custom Transport with MaxIdleConnsPerHost: 1.

3. Suggestions

1. F1 fix: Move dev MinIO creds to gitignored config or env-var interpolation. Low effort, high hygiene value.
2. F2 fix: Hoist the http.Client to package level. One-line change.
3. Nit: The health endpoint's statusMap["error"] = lastError.Error() (pre-existing pattern, not this PR) can leak internal hostnames/connection strings to unauthenticated callers. Consider a future PR to redact the error detail from the public response while keeping it in the log.

4. Additional Observations

• MintBotOBO friend-add is best-effort (mint_obo.go:59-65): AddFriend failures are logged-and-continue. This is intentional (bot is usable without friend link) but worth noting that a minted bot may lack the bidirectional friend relationship in edge cases.
• botToken SQL no-rows → 500 not 404 (bot_api.go:155): When SelectBySql().Load(&r) finds no matching robot, dbr returns an error → 500. The r.BotToken == "" 404 path (line 160) is unreachable for the no-rows case. Functionally correct (caller gets an error either way) but the status code is imprecise. Pre-existing dbr pattern, not a blocker.
• deriveServiceURL (command.go:1137): Simple port-swap helper, correctly handles URLs with and without trailing port. No edge-case issues found.

5. Data-Flow Tracing

Consumer	Upstream Source	Verified Flow
observeAgentTurnDelivery(req.ChannelType, ...)	req struct field ChannelType uint8 (send.go:27), JSON-bound from HTTP body	✅ → agentTurnChannelTypeLabel() switch maps to {person,group,topic,other}, default catches all unknowns
imReachable(imURL)	cn.ctx.GetConfig().WuKongIM.APIURL (config, not user input)	✅ → TrimSpace → empty check → GET with 2s timeout
assertSpaceMember(uid, spaceID)	mintBot: c.GetLoginUID() + req.SpaceID; resolveAPIKey: user_api_key.uid + user_api_key.space_id	✅ → 3-way JOIN filters space.status=1, user.status=1, sm.status=1
botToken SQL callerSpace	resolveAPIKey() → user_api_key.space_id (DB-sourced, not user input)	✅ → parameterized query, cross-space filter enforced
authVerifyToken OwnedBots	robot table JOIN user on robot_id=uid, filtered by creator_uid=resp.UID + status=1	✅ → UNCONDITIONAL load (v3.3.5 fix), DB error → empty list (fail-secure)
authVerifyAPIKey Step 2 membership	keyInfo.UID + keyInfo.SpaceID from Step 1 DB lookup	✅ → same 3-way JOIN pattern as assertSpaceMember
runtimeOnboarding spaceID	c.Query("space_id") or c.GetHeader("X-Space-Id") (user input)	✅ → validated by CheckMembership + user.status=1 before any side-effect

6. Blindspot Checklist (R5 — security_sensitive)

C1 — Dual-path parity: ✅ CLEAR

• add/remove: No symmetric pair operations in this PR
• create-gate ↔ execute-path: mintBot checks space membership before MintBotOBO; botToken checks api_key + space + creator; runtimeOnboarding checks membership + user.status before lazy-create of api_key. All authorization gates are symmetric between creation and execution paths.
• Parallel implementations: assertSpaceMember (bot_provision/resolve.go) and the inline 3-way JOIN in authVerifyAPIKey (user/api.go:4385) use identical SQL patterns (space.status=1 + user.status=1 + sm.status=1). No drift.

C2 — Control-flow ordering / nesting reuse: ✅ CLEAR

• observeAgentTurnDelivery called exactly once per sendMessage — no double-counting risk
• deriveServiceURL called twice (fleet + matter) with different port suffixes — no nesting
• imReachable called once per health check — no re-entry
• No security controls (escape/sanitize/regex) introduced that need non-canonical-form testing

C3 — Authorization boundary ≠ capability boundary: ✅ CLEAR

• mintBot: session auth (web) → space membership gate → MintBotOBO. Only authenticated web users who are space members can mint.
• botToken: api_key Bearer → resolveAPIKey (status=1 + client_id='botfather') → space membership + creator check. Only the bot's creator with a valid daemon key can pull the token.
• runtimeOnboarding: session auth → space membership + user.status=1 → lazy-create api_key. Banned users blocked.
• authVerifyAPIKey: internal endpoint, api_key validation → 3-way JOIN. No unauthorized path.

C4 — Authorization lifecycle / container-member state cascade: ✅ CLEAR

• assertSpaceMember checks all three levels: space.status=1 (container), sm.status=1 (membership), user.status=1 (account ban). A disabled space cascades to deny all member operations. A banned user cascades to deny all api_key/mint operations.
• authVerifyAPIKey Step 2 uses the same 3-way JOIN — symmetric coverage.
• queryUserSpaceContext spaces query JOINs space.status=1 — disabled spaces excluded from context.
• No gap found where a parent container's disabled state fails to cascade.

7. Cross-round Blocker Recheck (R6)

N/A — first review of this PR.

---

[Octo-Q] verdict: APPROVED

No P0/P1 findings. Two P2 items (hardcoded dev credentials in tracked config, per-call http.Client allocation) are hygiene/efficiency improvements that should be addressed but do not block landing. All authorization chains verified complete across the new bot provisioning, auth verification, and runtime onboarding surfaces. C1–C4 blindspot checks all clear.

[yujiawei]

Code Review — PR #407 (octo-server)

Reviewed at head dfea4c5613e9fdb3dd82c4014538c77d8f9cf9bb. Scope: 5 files / +268 lines (modules/common/api.go, modules/bot_api/metrics.go, modules/bot_api/send.go, plus tests). This PR is security-sensitive (it touches an unauthenticated endpoint and adds an outbound network probe), so I looked at it carefully.

Verdict: Approve. No correctness, data-loss, or build-breaking issues. go build, go vet, and all new tests pass locally. The two new metrics are correctly wired through the single delivery chokepoint, label cardinality is bounded, and the health probe is timeout-bounded and fails safe. The items below are non-blocking — one info-disclosure point I'd like a maintainer to consciously accept, plus three minor robustness/test nits.

What's good

• Metrics design is sound. delivery_duration_seconds / delivery_total are labeled channel_type{person|group|topic|other} × result{ok|error} — fully bounded, no robotID/channelID to explode Prometheus memory. The default -> "other" collapse is fuzz-safe and unit-tested. init() pre-warms the 8-series matrix so dashboards can distinguish "zero deliveries" from "metric absent". Namespace/subsystem/buckets are consistent with the existing dmwork_http_* and modules/oidc/metrics.go patterns.
• Instrumentation point is correct. observeAgentTurnDelivery(req.ChannelType, elapsed, err) is recorded immediately after dispatchMsgSendReq (the single caller), before the if err != nil branch — intentional and right. The error classification captures both transport failures and non-2xx IM responses as result=error.
• Health probe fails safe. Unconfigured WuKongIM.APIURL → im=unknown without failing overall readiness; only a transport-level failure marks im=down. The 2s http.Client{Timeout} bounds a hung IM control plane so it can't stall the handler (or a liveness probe behind it). Treating any HTTP response (incl. 4xx/5xx) as reachable correctly decouples readiness from a specific WuKongIM route/version.

Non-blocking findings

[P2 — please have a human accept] Unauthenticated /v1/health discloses the internal IM address on probe failure

modules/common/api.go — /v1/health is registered via raw r.GET(...) (no auth middleware), and on a transport failure the handler returns statusMap["error"] = lastError.Error() in the JSON body. The imReachable transport error embeds the configured WuKongIM.APIURL (e.g. Get "http://10.x.x.x:5001": dial tcp ...: connection refused), so an unauthenticated caller can learn internal control-plane host:port / DNS topology.

Important context: this is not a new vulnerability class — the parent commit already returned lastError.Error() for db.Ping/redis.Ping failures, which leak the DB/Redis host:port the same way. This PR adds one more internal endpoint (IM) to an already-leaky surface. I also confirmed no secret leaks: WuKongIM.ManagerToken is a separate config field sent via an HTTP header at the real call sites and is never part of APIURL.

Because it's incremental rather than novel, I'm not blocking on it — but since this PR is security-sensitive and it does widen the disclosure surface by one endpoint, a maintainer should consciously accept it. The clean fix (which also closes the pre-existing db/redis case): keep the per-component up/down/unknown fields in the public body but drop the raw lastError.Error() string, logging it server-side only (the handler already does cn.Error(...)).

[P2] imReachable follows redirects, partially contradicting "any HTTP response = reachable"

modules/common/api.go — imReachable uses the default http.Client (no CheckRedirect), which follows up to 10 redirects. The doc comment says any HTTP response counts as reachable, but a 3xx from the IM root is followed rather than counted; if the redirect target then fails transport, the probe reports im=down even though WuKongIM answered. The tests cover 200/404/500 but not a redirect. Low likelihood (the IM root is a flat API server) and it fails closed, so this is a robustness nit — consider client.CheckRedirect = func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse } to make the "first response wins" intent explicit.

[P2] Health probe doesn't propagate the request context

modules/common/api.go — the probe uses http.NewRequest rather than http.NewRequestWithContext(c.Request.Context(), ...). If the health caller disconnects, the outbound IM probe still runs to the 2s timeout. Bounded and minor (and a global rate limiter already guards the endpoint), but threading the request context would let it cancel early under health-check floods.

[P2 — test quality] TestAgentTurnDeliveryTotal_AllLabelsPrewarmed doesn't assert what its name claims

modules/bot_api/metrics_test.go — the test calls GetMetricWithLabelValues, which lazily creates the series if absent, so it would pass even if init() stopped pre-warming the 8 combinations. It validates label well-formedness, not prewarming. To actually assert prewarming, check testutil.CollectAndCount(metricAgentTurnDeliveryTotal) == 8 (or assert each series is present) before any observe* call runs.

Coverage notes / blind spots

• I verified build, vet, the new tests, the WuKongIM.APIURL config field, the ChannelType* constants, and that dispatchMsgSendReq has a single caller. I did not exercise the probe against a live WuKongIM, nor confirm how octo-deployment k8s manifests consume /v1/health (whether the JSON body is parsed or only the 200 status is read) — that drives the real-world impact of the info-disclosure point above.
• Latency semantics: the histogram measures the outbound dispatch leg only, not the end-to-end human→bot→human turn. That matches the stated OCT-17 scope and the PR description's rationale (no reliable server-side correlation key across the two async HTTP legs), so it's a deliberate, documented choice — just worth keeping in mind when naming dashboards.

[yujiawei]

Code Review — PR #407 (octo-server)

Reviewed at head dfea4c5613e9fdb3dd82c4014538c77d8f9cf9bb. This is a full-scope review of all 36 changed files (≈3.1k additions), correcting an earlier review that only covered the OCT-17 observability slice (5 files) and missed the bot-provisioning / cross-service auth surface that makes up the bulk of this PR.

Verdict: Request changes. One P1 (a ban-revocation gap on the user-token verification path) and one credential-hygiene item I'd want a human to sign off on, plus several non-blocking nits. The auth surface is, on the whole, carefully built — the items below are specific and each has a small fix.

What's solid

• Cross-space / IDOR gates on the daemon paths are correct. GET /v1/bot/:uid/token only returns a token for a bot that is both a member of the caller api_key's bound space (INNER JOIN space_member sm ON sm.uid=r.robot_id AND sm.space_id=? AND sm.status=1) and created by the caller (r.creator_uid == callerUID); a guessed botUID yields 403/404 with no existence leak.
• Anti-enumeration is sound. Credential failures on the daemon path collapse to single codes (ErrBotProvisionAuthFailed / BotForbidden / BotNotFound). I confirmed the dbr semantics: botToken uses .Load() (multi-row), which returns (0, nil) on zero rows — so the no-rows case correctly falls through to the r.BotToken == "" → 404 branch (it does not 500). The 404 path is reachable and correct.
• Caller-supplied bot_token cannot hijack an existing bot. robot.bot_token carries a functional UNIQUE index on NULLIF(bot_token,''), so a colliding INSERT fails; createBotCoreWithRetry holds the token constant across retries, so a collision errors out rather than overwriting a victim's row.
• Parameterized SQL throughout; the only string concatenation is static identifier quoting (`user`), never user input.
• owned_bots_by_space whitelist holds. The token path filters bot rows through validSpaces built from the caller's own active spaces, and the api_key path scopes via SQL AND sm.space_id=? — neither leaks a foreign space_id.

Blocking

[P1] POST /v1/auth/verify does not gate user.status, so a banned user keeps gateway access until token TTL

modules/user/api.go — authVerifyToken resolves the session token purely from the Redis cache (Cache().Get(TokenCachePrefix + token)) and builds the full response — identity, OwnedBots, and (with ?include=context) spaces + owned_bots_by_space + context_included — with no user.status=1 check.

The ban path does not evict that cache. Admin ban (liftBanUser, modules/user/api_manager.go) sets user.status=0, bans the IM channel, and calls QuitUserDevice(uid, -1). I traced QuitUserDevice in the pinned dependency: it issues a single POST to the IM control plane's /user/device_quit and touches nothing else. The Redis key authVerifyToken reads (token:{token}) is deleted only by revokeAllDeviceTokens, which is called only from the account-deletion path (deleteAdminUsers) — never from ban. So after a ban, the user's session token remains valid in the cache (default TTL is long), and this endpoint keeps returning a full, populated authorization context for a banned account.

Why this is in-scope for this PR rather than pre-existing noise: the PR's two new sibling credential endpoints — authVerifyAPIKey and runtime-onboarding — both explicitly add a user.status=1 gate, with comments stating they are closing exactly this account-ban bypass. Their comments also assert that the session-token path is already covered because "QuitUserDevice clears the redis token cache." That premise is false (per the trace above), and the token path is the one path left ungated. The PR builds its new cross-service authz primitives (owned_bots_by_space, context_included) on this ungated path, so it extends the gap rather than merely inheriting it.

Fix: mirror the sibling pattern — gate the resolved info.UID on user.status=1 in authVerifyToken (and/or add the INNER JOIN user u ... u.status=1 to queryUserSpaceContext's spaces query) so a banned user fails closed here as it already does on the api_key path. If the downstream gateway consumers in fact re-check ban state and this is intentional, please say so explicitly and remove the now-incorrect "clears redis token cache" comments — but as written, this is a fail-open revocation gap on a credential-verification endpoint in a security-sensitive PR.

Human verification requested (this PR carries needs-human-review): please confirm the intended ban-revocation contract for the token gateway path before merge.

Non-blocking (should fix, not gating)

[P2 — please have a human security owner confirm] Committed MinIO credentials in the default config

configs/tsdd.yaml uncomments and activates fileService: "minio" with accessKeyID: "admin" / secretAccessKey: "12345678", and this file is the default -config path (main.go). Mitigating factors: the endpoint is loopback (http://127.0.0.1:9000), the block is labeled dev-only, and the values are env-overridable (TS_-prefixed AutomaticEnv is wired, so TS_MINIO_SECRETACCESSKEY overrides). Still, committing live credentials to a public repo is a hygiene line worth not crossing, and the in-file comment ("Production should set proper creds via env vars, not commit creds") argues against the change itself. Recommend re-commenting the block or switching to env-var references before merge. The same hunk flips external.ip from auto-detected to hardcoded localhost, which would feed http://localhost:8090 to onboarding output unless External.BaseURL is set — worth a second look.

[P2] POST /v1/bot/mint has no per-UID rate limit

modules/bot_provision/bot_api.go mounts mint behind AuthMiddleware only, while the sibling bot-create route uses SharedUIDRateLimiter and even this PR's own /v1/bot/:uid/token got a StrictIPRateLimit. Each mint amplifies into ~5 DB writes + IM app creation, so an authenticated space member can script bot/space_member churn (bounded only by the global per-IP floor). Add SharedUIDRateLimiter to match the convention.

[P2] Caller-supplied bot_token has no format/entropy floor

mintBot accepts req.BotToken verbatim when non-empty (no bf_ prefix / length / entropy check). It can't hijack another bot (UNIQUE index), but it lets a caller register a low-entropy bearer value in the global bot-token namespace that authVerifyBot trusts unscoped. Prefer server-only generation, or enforce the bf_ prefix + entropy.

[P2] client_id='botfather' / status=1 duplicated as literals across 3+ sites

resolve.go, authVerifyAPIKey, and the issuance default all hard-code the daemon-gate literals (forced by a real import cycle). Not exploitable today, but a drift risk on the gate that keeps integration-client keys off the daemon path — a single exported constant would remove the class.

[P2] runtime-onboarding uses raw error responses and isn't in the legacy-response guard list

modules/botfather/api_runtime_onboarding.go uses c.ResponseErrorWithStatus(errors.New(...)) at ~8 sites (the repo's envelope/i18n contract prefers httperr.ResponseErrorL + registered errcode), and the new file isn't added to TestBotfatherNoLegacyResponseError's file list, so future edits here evade the guard. Strings are generic (no info leak today); this is a contract/consistency fix.

Build / test / coverage notes

• go build and go vet pass for all changed packages. The new observability tests pass. The bot_provision / user auth tests require a live MySQL/Redis (they panic in testutil.NewTestServer on DB/migration setup here) — consistent with the PR's stated "CI needs MySQL/Redis/WuKongIM"; I could not execute them in this environment.
• Out-of-repo blind spots that bound the P1's real-world blast radius: the fleet/matter consumers of owned_bots_by_space / context_included live in other repos, and the nginx internal-IP ACL the code cites as the primary control for the verify-* / token endpoints is in the deployment repo. The server-side gate is what I can assess, and it is missing on the token path.

[Jerry-Xin]

This PR is relevant to octo-server, but it contains blocking regressions outside the stated observability scope.

🔴 Blocking

• 🔴 Critical: configs/tsdd.yaml enables local-only deployment settings in a committed config: external.ip: "localhost" and MinIO as the active fileService with admin / 12345678 credentials at lines 139-145. This changes runtime behavior for anyone using this config and commits local PoC credentials. Keep this as commented documentation or move it to a local/example override file.

• 🔴 Critical: modules/botfather/api_runtime_onboarding.go introduces a new user-facing endpoint that returns legacy raw errors via c.ResponseErrorWithStatus at lines 66, 79, 93, 97, 107, 111, 124, and 134. This violates the repository’s localized error-envelope contract for new endpoints. The existing botfather source guard also does not include this new file: modules/botfather/api_i18n_test.go. Add registered errcodes/helpers and include the new handler file in the guard.

💬 Non-blocking

• 🟡 Warning: authenticated new routes are missing the shared UID limiter. /v1/runtime-onboarding is mounted with only AuthMiddleware in modules/botfather/api.go, and /v1/bot/mint is mounted similarly in modules/bot_provision/bot_api.go. Project guidance says authenticated routes should mount SharedUIDRateLimiter after auth.

• 🟡 Warning: gofmt -l reports unformatted Go files: modules/botfather/api_runtime_onboarding.go, modules/bot_provision/bot_api.go, modules/user/api.go, and modules/user/api_verify_apikey_test.go.

✅ Highlights

• The agent-turn metrics use bounded labels and prewarm low-cardinality series.
• The IM health probe is additive and has a timeout, which keeps the health endpoint from hanging indefinitely.

I also tried go test ./modules/bot_api ./modules/common; it failed during test server setup with an existing migration-state error (unknown migration in database), so I could not validate those packages end to end in this checkout.