# Remove `IsHitTCIDMNewArchByCluster` Compatibility Code — Plan

## Background

All dedicated clusters now have `cluster.GetAttributes().GetUseNewDedicatedArch() == true`.
The `IsHitTCIDMNewArchByCluster` function in `internal/common/utils.go` determines whether
to use the **3-layer regional architecture** (new) or **cloud-native global architecture** (legacy).
Since all dedicated clusters are on the new arch, `IsHitTCIDMNewArchByCluster` should always
return `true`, and all `false`-branch (legacy) code paths can be removed.

### Function Under Cleanup

```go
// internal/common/utils.go:108-131
func IsHitTCIDMNewArchByCluster(ctx context.Context, cluster *core.Cluster) bool {
    component := ComponentFromContext(ctx)
    if len(component) > 0 {
        switch component[0] {
        case ComponentImport:
            return cluster.GetAttributes().GetUseNewDedicatedArch() &&
                config.IsHitImportNewArch(...)
        case ComponentDM: // TODO(ideascf): [DM] always use new arch
            return true
        }
    }
    return cluster.GetAttributes().GetUseNewDedicatedArch() &&
        config.IsHitDMNewArch(...)
}
```

**Cleanup approach:** Make this function always return `true` (removing feature-gate config
checks), then remove all `false`-branch dead code at each call site, then remove the function
itself and cascade-dead symbols.

> **Scope boundary:** This PR removes only TCIDM import/DM compatibility code.
> `UseNewDedicatedArch` checks in `app/` (backup, restore, CDC) are **out of scope**
> — they guard different codepaths for different features.

---

## Impact Analysis

### Direct Call Sites of `IsHitTCIDMNewArchByCluster` (5 locations)

| # | File | Line | Caller Function | True Branch (KEPT) | False Branch (REMOVED) |
|---|------|------|-----------------|--------------------|------------------------|
| 1 | `internal/common/utils.go` | 137 | `GetNetworkByCluster` | `getRegionalNetworkByAPI(ctx, cluster)` | `pkggrpcli.InfraClient().GetNetwork(ctx, networkReq)` |
| 2 | `internal/common/utils.go` | 184 | `GetAgentClientByCluster` | passes `newArch=true` → `NewRegionalAgentClient()` | passes `newArch=false` → HTTP agent `AgentClientImpl` |
| 3 | `internal/service/import/service.go` | 359 | `importService.Get` | `pimport.NewRegionalResourceAPI(...)` | `infraapi.GetInfraAPI()` as resourceAPI |
| 4 | `internal/model/import/import.go` | 825 | `Model.BgContext` | wraps API with `NewRegionalResourceAPI` | keeps original `m.api` unwrapped |
| 5 | `internal/model/import/import.go` | 1286 | `BgContext.syncImportCR` | `imp.ArchType = entity.ThreeLayerArch` | `imp.ArchType = entity.CloudNativeArch` |

### Indirect Impact via `GetAgentClient` (agent.go)

`GetAgentClientByCluster` calls `agent.GetAgentClient(ctx, newArch, cluster, network)`.
When `newArch` is always `true`, the entire `else` branch of `GetAgentClient` becomes dead:

```go
// internal/handler/agent/agent.go:51-66 — DEAD when newArch=true always
if network == nil || network.DataflowAgentUrl == "" {
    return nil, errors.Errorf(...)
}
var client HttpRequester
if infraProviderType == infraapi.ProviderTypeEKS || ... {
    client = NewAgentHttpRequester()
} else {
    client, err = NewAgentHttpRequesterForGardenerAndServerless()
}
return &AgentClientImpl{BaseURL: network.DataflowAgentUrl, Client: client, ProviderType: infraProviderType}, nil
```

**However**, `AgentClientImpl` is referenced in:
- `internal/handler/agent/agent_test.go` — tests for HTTP-based agent methods
- `internal/handler/agent/lightning_test.go` — lightning client tests

These tests test the `AgentClientImpl` struct's methods directly. Removing the struct
would require removing or rewriting those tests. **Decision: defer `AgentClientImpl` and
`AgentHttpRequester` struct removal to a separate PR** that also audits the test coverage.
In this PR, we only remove the false-branch in `GetAgentClient` that *instantiates* them.

### Cascade Dead Code

| Symbol | File | Why Dead |
|--------|------|----------|
| `IsHitTCIDMNewArchByCluster` | `internal/common/utils.go:108` | Always returns true → remove function, inline true |
| `config.IsHitImportNewArch` | `internal/config/config.go:1273` | Only caller was `IsHitTCIDMNewArchByCluster` |
| `config.IsHitDMNewArch` | `internal/config/config.go:1285` | Only caller was `IsHitTCIDMNewArchByCluster` |
| `config.IsHitTCIDMNewArch` | `internal/config/config.go:1297` | Only callers were the two above |
| `config.isMatchTCIDMCondition` | `internal/config/config.go:1313` | Only caller was `IsHitTCIDMNewArch` |
| `ComponentDM` constant | `internal/common/contextval.go:10` | No code sets it (setter `ContextWithDMWorkerVersion` was deleted in earlier commit) |
| `ComponentFromContext` | `internal/common/contextval.go:14` | Only caller was `IsHitTCIDMNewArchByCluster` |
| `setComponent` | `internal/common/contextval.go:19` | Only callers: `ContextWithImport` (kept) — **check if still needed** |
| `ContextWithImport` | `internal/common/contextval.go:24` | Used at 5+ call sites — **KEEP** even if `IsHitTCIDMNewArchByCluster` is removed (may serve other future purposes or logging) |
| `entity.CloudNativeArch` | `internal/db/entity/constant.go:239` | Still referenced in `app/models/restore/impl/restore_base.go` — **KEEP** |
| `config.ImportConfig.MatchArchType` | `internal/config/config.go:354` | Only used by removed `IsHitImportNewArch` |
| `config.ImportConfig.MatchConditions` | `internal/config/config.go:355` | Only used by removed `IsHitImportNewArch` |
| `config.DMConfig.MatchArchType` | `internal/config/config.go:546` | Only used by removed `IsHitDMNewArch` |
| `config.DMConfig.MatchConditions` | `internal/config/config.go:547` | Only used by removed `IsHitDMNewArch` |
| `config.MatchArchType` type | `internal/config/config.go:593` | Only used in removed code |
| `config.MatchArchTypeOld` | `internal/config/config.go:596` | Only used in removed code |
| `config.MatchArchType3Layer` | `internal/config/config.go:597` | Only used in removed code |
| `config.MatchCondition` struct | `internal/config/config.go:600` | Only used in removed code |
| `GetEngineClient` | `internal/common/utils.go:188` | Zero callers in entire codebase — dead code even before this cleanup |

### Symbols to KEEP (used outside of dead paths)

| Symbol | File | Reason |
|--------|------|--------|
| `ContextWithImport` | `internal/common/contextval.go` | Used at ~5 call sites in import model/service |
| `ComponentImport` | `internal/common/contextval.go` | Still used by `ContextWithImport` |
| `entity.ThreeLayerArch` | `internal/db/entity/constant.go` | Used in `import_into.go`, import model |
| `entity.CloudNativeArch` | `internal/db/entity/constant.go` | Used in `restore_base.go` |
| `AgentClientImpl` struct | `internal/handler/agent/agent.go` | Used in agent unit tests — defer removal |
| `AgentHttpRequester` | `internal/handler/agent/http.go` | Used by `AgentClientImpl` in tests |
| `NewRegionalAgentClient` | `internal/handler/agent/regional-agent.go` | Remains the sole agent creation path |

---

## Execution Steps

### Pre-condition: verify build is clean

```bash
make build-dataflow && make app-unit-test-local
```

Expected: both pass with zero errors.

---

### Step 1 — Simplify `GetNetworkByCluster` in `internal/common/utils.go`

**Change:** Remove the `IsHitTCIDMNewArchByCluster` check and always use regional API.

```go
// BEFORE (lines 133-152)
func GetNetworkByCluster(ctx context.Context, cluster *core.Cluster) (*pkggrpcli.RegionalNetwork, error) {
    logger := log.WithContextL(ctx)
    if IsHitTCIDMNewArchByCluster(ctx, cluster) {
        logger.Info("use 3-layer network", ...)
        return getRegionalNetworkByAPI(ctx, cluster)
    }
    // ... legacy infra API path ...
}

// AFTER
func GetNetworkByCluster(ctx context.Context, cluster *core.Cluster) (*pkggrpcli.RegionalNetwork, error) {
    return getRegionalNetworkByAPI(ctx, cluster)
}
```

**Verify:** `make build-dataflow` compiles.

---

### Step 2 — Simplify `GetAgentClientByCluster` in `internal/common/utils.go`

**Change:** Remove `IsHitTCIDMNewArchByCluster` call, always pass `true` to `GetAgentClient`.

```go
// BEFORE (lines 183-186)
func GetAgentClientByCluster(ctx context.Context, cluster *core.Cluster, network *pkggrpcli.RegionalNetwork) (agent.AgentClient, error) {
    newArch := IsHitTCIDMNewArchByCluster(ctx, cluster)
    return agent.GetAgentClient(ctx, newArch, cluster, network)
}

// AFTER
func GetAgentClientByCluster(ctx context.Context, cluster *core.Cluster, network *pkggrpcli.RegionalNetwork) (agent.AgentClient, error) {
    return agent.GetAgentClient(ctx, true, cluster, network)
}
```

**Verify:** `make build-dataflow`.

---

### Step 3 — Simplify `GetAgentClient` in `internal/handler/agent/agent.go`

**Change:** Remove the `newArch` parameter and always use `NewRegionalAgentClient`.

```go
// BEFORE (lines 30-66)
func GetAgentClient(ctx context.Context, newArch bool, cluster *core.Cluster, network *pkggrpcli.RegionalNetwork) (AgentClient, error) {
    ...
    if newArch {
        ...
        return NewRegionalAgentClient(ctx, clusterID, infraProviderType, region)
    }
    // legacy HTTP agent path ...
}

// AFTER
func GetAgentClient(ctx context.Context, cluster *core.Cluster) (AgentClient, error) {
    ...
    return NewRegionalAgentClient(ctx, cluster.Id, cluster.InfraProviderType, cluster.RegionName)
}
```

Also update all callers:
- `GetAgentClientByCluster` in `utils.go` → `agent.GetAgentClient(ctx, cluster)`
- `GetAgentClientByNetwork` in `utils.go` → `agent.GetAgentClient(ctx, cluster)`

Since `network` parameter is no longer needed by `GetAgentClient`, propagate signature changes:
- `GetAgentClientByCluster` no longer needs `network` param → but it's an exported function
  used by `GetNetworkAgentByCluster` and `GetAgentClientByNetwork`. Simplify conservatively:
  keep signature for now, just ignore `network` inside.

**Verify:** `make build-dataflow`.

---

### Step 4 — Simplify Import service `Get()` in `internal/service/import/service.go`

**Change:** Always use `NewRegionalResourceAPI`, remove the else branch.

```go
// BEFORE (lines 359-373)
if common.IsHitTCIDMNewArchByCluster(ctx, imp.Cluster) {
    resourceAPI, err = pimport.NewRegionalResourceAPI(...)
    ...
} else {
    resourceAPI = infraapi.GetInfraAPI()
}

// AFTER
resourceAPI, err = pimport.NewRegionalResourceAPI(
    ctx,
    infraapi.GetInfraAPI(),
    imp.Cluster.GetInfraProviderType(),
    imp.Cluster.GetRegionName(),
    imp.Cluster.GetTenantId())
if err != nil {
    logger.Error("failed to get regional resource api", zap.Error(err))
    return nil, wrapNonErrorCodeAsInternalError(err, "")
}
```

**Verify:** `make build-dataflow`.

---

### Step 5 — Simplify `BgContext()` in `internal/model/import/import.go`

**Change:** Always wrap API with `NewRegionalResourceAPI`, remove the conditional.

```go
// BEFORE (lines 825-831)
if infraAPI, ok := m.api.(*infraapi.API); ok &&
    common.IsHitTCIDMNewArchByCluster(ctx, importTask.Cluster) {
    resourceAPI, err := NewRegionalResourceAPI(ctx, infraAPI, providerType, region, tenantID)
    ...
    newModel.api = resourceAPI
}

// AFTER
if infraAPI, ok := m.api.(*infraapi.API); ok {
    resourceAPI, err := NewRegionalResourceAPI(ctx, infraAPI, providerType, region, tenantID)
    if err != nil {
        return BgContext{}, err
    }
    newModel.api = resourceAPI
}
```

**Verify:** `make build-dataflow`.

---

### Step 6 — Simplify `syncImportCR()` in `internal/model/import/import.go`

**Change:** Always set `ThreeLayerArch`, remove the else branch.

```go
// BEFORE (lines 1286-1290)
if common.IsHitTCIDMNewArchByCluster(ctx, imp.Cluster) {
    imp.ArchType = entity.ThreeLayerArch
} else {
    imp.ArchType = entity.CloudNativeArch
}

// AFTER
imp.ArchType = entity.ThreeLayerArch
```

**Verify:** `make build-dataflow`.

---

### Step 7 — Remove `IsHitTCIDMNewArchByCluster` and related dead code from `internal/common/utils.go`

**Changes:**
1. Delete `IsHitTCIDMNewArchByCluster` function (lines 108-131)
2. Delete `GetEngineClient` function (lines 188-200) — zero callers, dead code
3. Remove now-unused imports: `config`, `entity` (verify which are still needed)

**Verify:** `make build-dataflow`.

---

### Step 8 — Clean up `internal/common/contextval.go`

**Changes:**
1. Delete `ComponentDM` constant (line 10) — no setter, unreachable
2. Keep `ComponentImport`, `ContextWithImport`, `setComponent`, `ComponentFromContext`
   — `ContextWithImport` is still used in 5+ places

**Verify:** `make build-dataflow`.

---

### Step 9 — Remove config feature gate functions from `internal/config/config.go`

**Changes:**
1. Delete `IsHitImportNewArch` function (~line 1273)
2. Delete `IsHitDMNewArch` function (~line 1285)
3. Delete `IsHitTCIDMNewArch` function (~line 1297)
4. Delete `isMatchTCIDMCondition` function (~line 1313)
5. Delete `MatchArchType` type and constants (`MatchArchTypeOld`, `MatchArchType3Layer`)
6. Delete `MatchCondition` struct and its `IsValid()` method
7. Remove `MatchArchType` and `MatchConditions` fields from `ImportConfig` struct
8. Remove `MatchArchType` and `MatchConditions` fields from `DMConfig` struct
9. Clean up now-unused imports (e.g., `utils` if only used by `isMatchTCIDMCondition`)

**Verify:** `make build-dataflow`.

---

### Step 10 — Remove false branch from `GetAgentClient` in `internal/handler/agent/agent.go`

**Changes:**
1. Remove `newArch bool` parameter from `GetAgentClient`
2. Remove the `if newArch { ... }` conditional — always call `NewRegionalAgentClient`
3. Remove `network *pkggrpcli.RegionalNetwork` parameter (no longer used)
4. Remove the entire false branch (lines 51-66)
5. Update all callers: `GetAgentClientByCluster`, `GetAgentClientByNetwork`

**Verify:** `make build-dataflow`.

---

### Step 11 — Remove unused imports across all changed files

After all changes, compiler errors from unused imports will surface.
Remove all imports that are now unused in:
- `internal/common/utils.go` — `config`, `entity`, possibly `agent`
- `internal/service/import/service.go` — `common` if no longer used (check other usages)
- `internal/model/import/import.go` — `common` if no longer used (check other usages)
- `internal/handler/agent/agent.go` — `pkggrpcli`, `infraapi` if no longer used

**Verify:** `make build-dataflow`.

---

### Step 12 — Final verification

```bash
make build-dataflow
make app-unit-test-local
```

Both must pass with zero errors.

---

## Risk Assessment

| Risk | Mitigation |
|------|------------|
| `connprofile` and `cluster/data_ingestion` use `GetNetworkByCluster` without setting import context | These callers previously fell through to the default branch which also checked `GetUseNewDedicatedArch()`. Since all dedicated clusters have this attribute true, and we're simplifying `GetNetworkByCluster` to always use regional API, this is safe. |
| `AgentClientImpl` used in unit tests | We keep the struct and its methods — only remove the instantiation path in `GetAgentClient`. Tests that construct `AgentClientImpl` directly still compile. Defer full struct removal. |
| Config fields removed from `ImportConfig`/`DMConfig` | These are `mapstructure` fields read from config files. Removing them means YAML/TOML config files with these keys will have unknown fields — `mapstructure` silently ignores them by default. No runtime error. |
| `entity.CloudNativeArch` still used in restore code | We keep the constant. Only the `syncImportCR` assignment is changed; restore code is unaffected. |
| `GetEngineClient` removal | Function has zero callers — safe to remove. Uses old infra API `GetTiFlowJob` which is part of the engine/tiflow architecture, not the 3-layer arch decision. |

## Out of Scope

- `UseNewDedicatedArch` checks in `app/` (backup, restore, CDC workflows)
- `AgentClientImpl` struct and its method implementations (used in tests)
- `AgentHttpRequester` struct (used by `AgentClientImpl`)
- `engine.go` / `EngineClient` struct (part of `AgentClient` interface, used by regional impl)
- `internal/service/import/convert.go:44` — direct `GetUseNewDedicatedArch()` check for GCS node role, not via `IsHitTCIDMNewArchByCluster`
- `internal/model/import/agent.go:136` — direct `GetUseNewDedicatedArch()` check for GCS node role

---

## Execution Log

### Additional dead code discovered and removed during execution

| Item | File | Reason |
|------|------|--------|
| `getDMMatchConditions` | `internal/model/dm/model.go` | Callers (`getDMNewArchConditions`, `getDMOldArchConditions`) were removed in earlier commits on this branch. Function references removed `config.MatchCondition` type. |
| `getDMNotMatchConditions` | `internal/model/dm/model.go` | Same — callers removed in earlier commits. |
| `Test_getDMMatchConditions` | `internal/model/dm/model_test.go` | Tests for removed functions. |
| `Test_getDMNotMatchConditions` | `internal/model/dm/model_test.go` | Tests for removed functions. |

### Final verification

- `make build-dataflow` — **PASS**
- `make app-unit-test-local` — **PASS** (36 packages ok, 0 failures)

### Summary of all files changed

| File | Changes |
|------|---------|
| `internal/common/utils.go` | Removed `IsHitTCIDMNewArchByCluster`, `GetEngineClient`; simplified `GetNetworkByCluster`, `GetAgentClientByCluster`; cleaned imports |
| `internal/common/contextval.go` | Removed `ComponentDM` constant |
| `internal/handler/agent/agent.go` | Simplified `GetAgentClient` (removed `newArch`, `network` params, false branch); cleaned imports |
| `internal/service/import/service.go` | Simplified `Get()` — always use regional API |
| `internal/model/import/import.go` | Simplified `BgContext()` and `syncImportCR()` — removed conditional branches |
| `internal/model/dm/model.go` | Removed dead `getDMMatchConditions`, `getDMNotMatchConditions` |
| `internal/model/dm/model_test.go` | Removed dead test functions; cleaned imports |
| `internal/config/config.go` | Removed `ArchType`, `MatchCondition`, config functions (`IsHitImportNewArch`, `IsHitDMNewArch`, `IsHitTCIDMNewArch`, `isMatchTCIDMCondition`); removed struct fields from `ImportConfig` and `DMConfig` |