RDKEMW-21739: Restore crashupload success marker logline for upload success path [8.6]

README.md

@@ -231,7 +231,7 @@ cd test/functional-tests
 | Single successful upload (TC-081) | `upload.feature` | ✅ Complete |
 | Upload retry (TC-082/083) | `upload_retry.feature` | ✅ Complete |
 
-**Coverage**: 66 / 66 applicable TCs implemented (19 not applicable — see `L2_TESTS.md`)
+**Coverage**: 66 / 66 applicable TCs implemented (19 not applicable — see `test/L2_TESTS.md`)
 
 ## Rate Limiting & Recovery Mode
 

c_sourcecode/src/upload/upload.c

@@ -330,7 +330,7 @@ int upload_file(const char *filepath, const char *url, const char *dump_name, co
             }
             else
             {
-                CRASHUPLOAD_INFO("S3 %s Upload is successful\n", filepath);
+                CRASHUPLOAD_INFO("Upload is successful with TLS1.2 for %s\n", filepath);
                 if (t2_enabled)
                 {
                     t2CountNotify("SYS_INFO_S3CoreUploaded", 1);

openspec/config.yaml

@@ -0,0 +1,43 @@
+schema: spec-driven
+
+context: |
+  Repo: crashupload, an RDK crash dump discovery, packaging, and upload system with two active implementation paths: legacy shell scripts and a newer C binary.
+
+  Use this context strategy to keep SDLC artifact generation accurate and token-efficient:
+  - Read OpenSpec docs first: openspec/project.md, openspec/runtime/**, openspec/pipeline/**, openspec/subsystems/**, openspec/integrations/**, openspec/diagrams/**, openspec/gaps/**.
+  - Read documentation second: README.md, c_sourcecode/README.md, unittest/README.md, docs/migration/**, and other repo README files relevant to the touched area.
+  - Read implementation third, but only the narrow slice needed for the task: uploadDumps.sh, runDumpUpload.sh, uploadDumpsUtils.sh, c_sourcecode/src/**, c_sourcecode/include/**, c_sourcecode/common/**, src/inotify-minidump-watcher.c, and related service files.
+  - Read verification and delivery context only when relevant: run_ut.sh, run_l2.sh, cov_build.sh, rdk_build.sh, test/L2_TESTS.md, unittest/**, test/functional-tests/**, and .github/workflows/*.yml.
+
+  Repository reality to preserve in artifacts:
+  - Runtime is event/timer/script driven, not a single long-running daemon.
+  - uploadDumps.sh is the top-level dispatcher.
+  - mediaclient is binary-first when /usr/bin/crashupload is present; broadband and extender still rely on the legacy shell path.
+  - C implementation lives under c_sourcecode/ and legacy shell behavior remains authoritative for some device types.
+  - L1 means unit-test coverage centered on unittest/ and ./run_ut.sh.
+  - L2 means functional coverage centered on test/functional-tests/, test/L2_TESTS.md, and ./run_l2.sh after ./cov_build.sh --l2-test.
+  - CI context comes from .github/workflows/*.yml plus build/test entry scripts at repo root.
+
+  Context minimization rules:
+  - Prefer the existing OpenSpec artifacts as the summary layer instead of re-reading large code trees.
+  - Pull only the docs and code slices directly related to the subsystem being changed.
+  - Do not load all docs or all source files by default; start with the nearest README or OpenSpec doc, then drill into exact code paths.
+  - Separate shell-path behavior, C-path behavior, test impact, and CI impact instead of merging them into a generic summary.
+
+rules:
+  proposal:
+    - Start from openspec/project.md and only add the specific OpenSpec/doc/code files needed for the affected subsystem.
+    - State whether the change touches shell runtime, C runtime, test harness, CI, documentation, or a combination.
+    - Keep repository background concise; prefer explicit file anchors over broad repo summaries.
+  design:
+    - Prefer openspec/runtime, openspec/pipeline, openspec/subsystems, and openspec/diagrams before raw implementation files.
+    - Pull docs/migration/** and relevant README files when they add architecture, deployment, or migration detail.
+    - Call out device-type routing differences and any shell-versus-C divergence.
+  specs:
+    - Use code-truth for behavioral claims: uploadDumps.sh, runDumpUpload.sh, c_sourcecode/src/**, service units, and workflows when behavior depends on them.
+    - Separate L1 unit-test expectations from L2 functional-test expectations.
+    - Mention CI behavior only if workflows, build scripts, or test scripts are affected.
+  tasks:
+    - Generate file-scoped, validation-oriented tasks.
+    - Reference the smallest sufficient context set for each task.
+    - Include concrete verification commands when applicable, choosing from ./cov_build.sh, ./run_ut.sh, ./cov_build.sh --l2-test, and ./run_l2.sh.

openspec/integrations/client-integration-model.md

@@ -0,0 +1,76 @@
+# Client and Integration Model
+
+## External Interaction Surfaces
+
+## 1) CLI / command interfaces
+### uploadDumps.sh
+- Primary external entry for services and helper watchers.
+- Accepts positional arguments passed through to selected runtime path.
+
+### crashupload binary
+- Fact: current main expects positional flags where argv[2] determines dump mode (0 minidump, 1 coredump) and optional argv[3]=secure, argv[4]=wait_for_lock.
+- Inference: practical CLI contract is script-oriented rather than end-user friendly options.
+
+### inotify-minidump-watcher
+- Usage includes directory, command, args, and file patterns.
+- Executes configured command when matching file is created.
+
+## 2) System integration points
+- systemd units and timer/path triggers.
+- Filesystem directories as crash intake source and local spool.
+- Marker/flag files in /tmp and /opt governing policy and control flow.
+
+## 3) Library/API integrations
+- Property/config: getDevicePropertyData, getIncludePropertyData, GetPartnerId, GetModelNum, etc.
+- Policy/config APIs: RFC parameter read/write, RBUS privacy parameter reads.
+- Telemetry: T2 events for counters and split values.
+- Upload/security: uploadutil and TLS/curl stack.
+
+## Request Lifecycle: Trigger to Completion
+1. Trigger source invokes uploadDumps.sh.
+2. Dispatcher selects C or legacy path.
+3. Runtime initializes config/platform and lock.
+4. Runtime verifies eligibility (dumps present, deferral/privacy/reboot/rate-limit rules).
+5. Runtime discovers and normalizes artifacts.
+6. Runtime archives payload with metadata/log context.
+7. Runtime performs upload workflow.
+8. Runtime records outcome markers and cleanup.
+9. Runtime exits with status consumed by invoker policy.
+
+## Intended Caller Workflows
+### Systemd boot workflow
+- Timer fires after boot window, invoking minidump processing path via uploadDumps.sh.
+
+### Filesystem event workflow
+- Path/inotify detects dump creation, invokes upload script.
+
+### Manual operator workflow
+- invoke script or binary for diagnostics/retry scenarios.
+
+## Configuration and Environment Dependencies
+- Files:
+  - /etc/device.properties
+  - /etc/include.properties
+  - /etc/breakpad-logmapper.conf
+- Runtime dirs:
+  - /opt/minidumps, /minidumps, /var/lib/systemd/coredump, /opt/secure/*
+- Runtime flags/markers:
+  - /tmp/.uploadMinidumps, /tmp/.uploadCoredumps
+  - /tmp/.minidump_upload_timestamps, /tmp/.deny_dump_uploads_till
+  - /tmp/set_crash_reboot_flag
+  - /opt/.upload_on_startup
+
+## Rate-Limit State Semantics
+- /tmp/.deny_dump_uploads_till: global cooldown gate; if active, upload processing is blocked regardless of dump type.
+- /tmp/.minidump_upload_timestamps: minidump-only rolling counter used to trigger cooldown when upload volume exceeds threshold.
+
+## Device-Side vs External Integration Boundaries
+- Device-side only:
+  - file scanning, archive composition, local policy gates, cleanup.
+- Externally integrated:
+  - RFC/RBUS data sources, telemetry backplane, crash portal endpoint/signed URL service, S3 upload endpoint.
+
+## Integration Risks for OpenSpec Planning
+- Fact: behavior differs strongly by device type and runtime branch.
+- Fact: some integration paths are stubs or unsupported in specific branches (for example broadband in upload.c).
+- Inference: future changes should specify per-device/path applicability and rollback strategy.

openspec/runtime/execution-model.md

@@ -0,0 +1,94 @@
+# Runtime Execution Model
+
+## Execution Modes
+### Mode 1: Mediaclient preferred C path
+- Fact: uploadDumps.sh selects crashupload binary if present.
+- Fact: binary may fall back to legacy via script if exit code indicates failure and dispatcher chooses fallback.
+
+### Mode 2: Broadband/extender legacy path
+- Fact: uploadDumps.sh directly invokes runDumpUpload.sh for broadband/extender.
+- Fact: upload.c currently marks broadband branch unsupported for C upload_process.
+
+### Mode 3: Manual/diagnostic invocation
+- Inference: crashupload can be executed directly with argument contract expected by main (argv[2] dump type flag; optional secure/wait_for_lock flags).
+- Unknown: direct CLI contract stability across releases.
+
+## Detailed C Binary Flow (Observed)
+1. Preconditions and argument check
+- argc must be >= 3.
+- argv[2] determines dump type branch: 0 minidump, 1 coredump.
+
+2. Locking and signal behavior
+- lock file path set by dump type.
+- SIGTERM handler removes lock file based on active mode.
+- lock mode optional wait_for_lock causes blocking lock behavior.
+
+3. Initialization
+- t2 init, config load, platform init, core log creation.
+
+4. Prerequisite gate
+- dump presence check in expected directory.
+- mediaclient deferral to 480 seconds uptime threshold.
+- optional coredump completion wait logic tied to mutex flag file.
+
+5. Privacy gate
+- mediaclient reads RBUS privacy mode.
+- DO_NOT_SHARE skips upload, but cleanup path still executes.
+
+6. Startup cleanup and scan
+- cleanup_batch runs before and after processing.
+- scanner finds candidate files and ensures size stability.
+
+7. Per-dump processing
+- sanitize/rename/telemetry extraction.
+- metadata naming with mac/date/box/model/process context.
+- archive creation with selected sidecar files.
+
+8. Rate limit and reboot checks
+- skip upload if reboot flag present.
+- global deny-window (`/tmp/.deny_dump_uploads_till`) is evaluated before dump-type specific checks and can block both minidump and coredump uploads while active.
+- minidump upload-counter check (`/tmp/.minidump_upload_timestamps`) is only evaluated for minidump runs.
+- when blocked, pending dumps in the active working directory can be removed.
+
+9. Upload loop
+- sequential upload per archive.
+- break on first upload error.
+
+10. Cleanup and termination
+- cleanup batch rerun.
+- release lock, uninit telemetry, exit with ret code.
+
+## Concurrency and Process Model
+- Fact: single-process, mostly sequential processing.
+- Fact: no explicit multithreading in processing path.
+- Fact: inter-process concurrency controlled by flock lock files.
+- Inference: throughput scales by trigger frequency and batch size rather than parallel upload workers.
+
+## State and File Semantics
+### Locking
+- /tmp/.uploadMinidumps
+- /tmp/.uploadCoredumps
+
+### Rate limit and deny windows
+- /tmp/.minidump_upload_timestamps
+- /tmp/.deny_dump_uploads_till
+- policy note: deny-window is global cooldown; minidump timestamp file is minidump-only counter state.
+
+### Startup/cleanup markers
+- /tmp/.on_startup_dumps_cleaned_up_<flag>
+- /opt/.upload_on_startup
+
+### Reboot/flow flags
+- /tmp/set_crash_reboot_flag
+- /tmp/coredump_mutex_release (checked in prerequisites path)
+
+## Failure Handling Semantics
+- Fact: lock contention in non-wait mode exits quickly.
+- Fact: scan failures/no dumps route to cleanup and often zero exit.
+- Fact: upload failure breaks remaining upload loop.
+- Fact: cleanup is best-effort and often non-fatal.
+- Unknown: upstream service restart policies and retry cadence on non-zero exits.
+
+## Execution Truth vs Expectations
+- Fact: several comments indicate optimized 7-step conceptual flow, but implementation includes TODOs and mixed maturity.
+- Inference: OpenSpec work should baseline on observed behavior rather than intended design commentary.

openspec/subsystems/subsystem-analysis.md

@@ -0,0 +1,83 @@
+# Subsystem Responsibility Analysis
+
+## Subsystem Map
+1. Trigger and orchestration subsystem
+  - systemd units, timer/path, optional inotify watcher, uploadDumps.sh dispatcher.
+
+2. Initialization and config subsystem
+  - system_init, config_manager, platform.
+
+3. Intake and qualification subsystem
+  - prerequisites, lock_manager, scanner.
+
+4. Packaging subsystem
+  - archive, file utilities, log mapping in scanner.
+
+5. Transport subsystem
+  - upload module with endpoint discovery, metadata POST, S3 PUT.
+
+6. Policy subsystem
+  - ratelimit, privacy mode check, reboot flag behavior.
+
+7. Operations and observability subsystem
+  - logger and telemetry interface wrappers.
+
+## Subsystem A: Trigger and orchestration
+- Facts:
+  - systemd timer and path units invoke uploadDumps.sh.
+  - uploadDumps.sh chooses C vs legacy runtime branch and fallback.
+- Inferences:
+  - This layer is the primary operational policy switch for migration strategy.
+- Unknowns:
+  - exact service installation/enabling matrix per platform SKU.
+
+## Subsystem B: Initialization and config
+- Facts:
+  - config_manager derives mode from argv and device properties.
+  - platform subsystem builds identifier set (mac/model/sha1) with fallbacks.
+- Risks:
+  - mixed defaults and TODOs can hide platform-specific behavior gaps.
+
+## Subsystem C: Intake and qualification
+- Facts:
+  - lock files ensure single active worker per dump type.
+  - prerequisites include dump-presence check and optional deferred upload.
+  - scanner enforces max file count and file-size stability gate.
+- Inference:
+  - stability gate mitigates racing with producer write completion.
+
+## Subsystem D: Packaging
+- Facts:
+  - archive_create_smart creates tar.gz and can include mapped process logs.
+  - filename normalization logic handles long names and container delimiter cleanup.
+- Unknown:
+  - full fidelity vs legacy script for all edge-case filename conventions.
+
+## Subsystem E: Transport
+- Facts:
+  - endpoint resolution path includes RFC and device property fallback.
+  - upload is two-step (metadata then object upload) with telemetry markers.
+- Inference:
+  - control plane and data plane are split, enabling signed URL workflows.
+
+## Subsystem F: Policy (rate limit/privacy/reboot)
+- Facts:
+  - deny-window and upload timestamp files in /tmp govern upload throttling.
+  - deny-window (`/tmp/.deny_dump_uploads_till`) is a global cooldown gate and can block both minidump and coredump uploads while active.
+  - upload timestamp counter (`/tmp/.minidump_upload_timestamps`) is only used for minidump rate-threshold evaluation.
+  - reboot flag can skip upload to avoid conflict during reboot transitions.
+  - privacy DO_NOT_SHARE bypasses upload for mediaclient path.
+- Assumption:
+  - product-level privacy compliance also depends on paths outside this component.
+
+## Subsystem G: Observability
+- Facts:
+  - component emits logs and T2 counters/values.
+  - telemetry calls are feature-flag/build-flag dependent wrappers.
+- Unknown:
+  - production log routing and marker governance ownership.
+
+## Cross-Cutting Concerns
+- Device type branching significantly changes behavior.
+- Build flags (RFC_API_ENABLED, RBUS_API_ENABLED, T2_EVENT_ENABLED, L2_TEST) alter effective runtime semantics.
+- Legacy and C implementations coexist and must be jointly reasoned during change planning.

L2_TESTS.md → test/L2_TESTS.md

@@ -80,8 +80,8 @@ sh cov_build.sh --l2-test
 | TC-047 | Upload-on-startup mode (minidump-on-bootup) | Cleanup | ⚠️ Partial | Upload-on-startup flow exists in `main.c` via `minidump-on-bootup-upload.service`; exact behaviour may differ from shell on-startup path | ✅ Implemented | `test_cleanup_batch.py` :: `test_upload_on_startup_flag_removed_for_coredump_mode` |
 | TC-048 | Upload count ≤ 10 → ALLOW_UPLOAD | Rate Limiting | ✅ Yes | `is_upload_limit_reached()` in `ratelimit.c` counts timestamp file lines; ≤ 10 entries → returns `ALLOW_UPLOAD` | ✅ Implemented | `test_ratelimit_allow.py` :: `test_upload_allowed_when_count_at_or_below_limit` |
 | TC-049 | Upload count > 10 within window → STOP_UPLOAD | Rate Limiting | ✅ Yes | > 10 lines within `RECOVERY_DELAY_SECONDS` window → `is_upload_limit_reached()` returns `STOP_UPLOAD` | ✅ Implemented | `test_ratelimit.py` :: `test_upload_blocked_when_count_exceeds_10` |
-| TC-050 | Rate limiting applied to minidump path only | Rate Limiting | ✅ Yes | `ratelimit_check_unified()` called in `main.c` only for `DUMP_TYPE_MINIDUMP` branch; coredump path skips rate limiting | ✅ Implemented | `test_ratelimit_allow.py` :: `test_coredump_not_rate_limited_by_minidump_counter` |
-| TC-051 | Recovery time not yet reached → uploads still blocked | Rate Limiting | ✅ Yes | `is_recovery_time_reached()` reads deny-till timestamp from `/tmp/.deny_dump_uploads_till`; still inside window → blocked | ✅ Implemented | `test_ratelimit.py` :: `test_upload_blocked_when_deny_file_active` |
+| TC-050 | Minidump counter check is minidump-only | Rate Limiting | ✅ Yes | `is_upload_limit_reached()` reads `/tmp/.minidump_upload_timestamps` only for `DUMP_TYPE_MINIDUMP`; coredump path bypasses this counter check when deny-window is not active | ✅ Implemented | `test_ratelimit_allow.py` :: `test_coredump_not_rate_limited_by_minidump_counter` |
+| TC-051 | Recovery time not yet reached → global cooldown blocks uploads | Rate Limiting | ✅ Yes | `is_recovery_time_reached()` reads deny-till timestamp from `/tmp/.deny_dump_uploads_till`; if still inside window, both minidump and coredump runs are blocked | ✅ Implemented | `test_ratelimit.py` :: `test_upload_blocked_when_deny_file_active`, `ratelimit_gtest.cpp` :: `RatelimitCheckUnified_CoredumpType_RecoveryWindow_BlockUpload` |
 | TC-052 | Recovery time reached → uploads unblocked | Rate Limiting | ✅ Yes | `is_recovery_time_reached()` returns `true` after window expires → uploads resume normally | ✅ Implemented | `test_ratelimit_allow.py` :: `test_recovery_time_expired_unblocks_upload` |
 | TC-053 | Timestamp written to rate limit log after upload | Rate Limiting | ✅ Yes | `set_time()` in `ratelimit.c` appends current timestamp entry to rate limit log file | ✅ Implemented | `test_ratelimit_allow.py` :: `test_no_deny_file_allows_upload_to_proceed` |
 | TC-054 | Rate limit counter resets after recovery period | Rate Limiting | ✅ Yes | Timestamps older than `RECOVERY_DELAY_SECONDS` are not counted; counter effectively resets after recovery period | ✅ Implemented | `test_ratelimit_allow.py` :: `test_rate_limit_resets_after_recovery_period` |

L2_TODO.md → test/L2_TODO.md

@@ -48,7 +48,7 @@ Tracks which applicable TCs still need L2 functional tests.
 | TC-047 | Upload-on-startup mode — `/opt/.upload_on_startup` removed on coredump run |
 | TC-048 | Upload count ≤ 10 → ALLOW_UPLOAD |
 | TC-049 | Upload count > 10 within window → STOP_UPLOAD |
-| TC-050 | Rate limiting applied to minidump path only |
+| TC-050 | Minidump counter check is minidump-only |
 | TC-051 | Recovery time not yet reached → uploads still blocked |
 | TC-052 | Recovery time expired → uploads unblocked |
 | TC-054 | Rate-limit counter reset after recovery period |

test/functional-tests/features/ratelimit.feature

@@ -23,7 +23,7 @@ Feature: Rate limiting — allow and block upload paths
   ratelimit_check_unified() enforces two independent limits:
     1. Deny-window check (is_recovery_time_reached): reads /tmp/.deny_dump_uploads_till.
     2. Per-minidump upload-count check (is_upload_limit_reached): reads
-       /tmp/.minidump_upload_timestamps (minidump path only).
+       /tmp/.minidump_upload_timestamps (counter check only for minidump mode).
   When either check returns BLOCK, remove_pending_dumps() cleans the working dir.
 
   Background:
@@ -53,8 +53,9 @@ Feature: Rate limiting — allow and block upload paths
     And the binary exits with return code 0
 
   # TC-050
-  Scenario: Rate-limit upload-count check is skipped for coredump mode
+  Scenario: Rate-limit upload-count check is skipped for coredump mode when deny-window is inactive
     Given /tmp/.minidump_upload_timestamps has 11 lines (would block a minidump run)
+    And /tmp/.deny_dump_uploads_till does not exist
     When the binary runs in coredump mode
     Then the upload-count else-branch returns ALLOW_UPLOAD immediately
     And /tmp/.deny_dump_uploads_till is NOT created by the rate limiter